On 4/23/19 8:34 AM, Richard W.M. Jones wrote:
I agree we should only be zeroing this buffer on NBD_CMD_READ, so
the
patch is wrong as it stands.
Having an "I promise not to be bad!" flag I think just adds more
complexity to plugins. It would be nice to do the best thing
automatically.
If we have a per-thread buffer then we're still (potentially) leaking
data between clients, even if that data only consists of previously
read data from another part of the disk. However this does seem like
the least bad approach since (a) we're not leaking random heap data
like secret keys and (b) we don't need to make the plugin API any more
complicated. I'll see how easy this is to implement ...
Right now, we're firing up separate threads for separate clients - a
given thread is only reused by a single client. (Observe a trace of the
file module with two simultaneous clients - our default of spawning 16
threads per connection means 32 helper threads are active at once when
there are two clients).
Of course, if we DO switch to a smarter thread-pooling approach (where
we put a ceiling of at most 16 outstanding requests from a given client,
but only add threads as we actually get enough parallel requests), then
we may indeed start having to worry about reusing a thread to service
requests from more than one client.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization:
qemu.org |
libvirt.org