On 5/13/19 2:56 AM, Richard W.M. Jones wrote:
On Thu, May 09, 2019 at 10:03:34PM -0500, Eric Blake wrote:
> Until the next few patches expose and implement new callbacks for
> filters and plugins, our initial implementation for NBD_CMD_CACHE is
> to just blindly advertise the feature, and call into .pread with an
> ignored buffer.
>
> Note that for bisection reasons, this patch treats any use of a filter
> as a forced .can_cache of false to bypass the filter's caching; once
> all affected filters are patched to handle cache requests correctly, a
> later patch will then switch the filter default to passthrough for the
> sake of remaining filters.
>
> Signed-off-by: Eric Blake <eblake(a)redhat.com>
>
> ---
>
> Note: we could also choose to always advertise caching, but make the
> nbdkit default version be a no-op rather than call out to .pread. For
> that matter, maybe a plugin's .can_cache should allow the plugin to
> choose between a tri-state of no-op, fallback to .pread, or call
> .cache; I'm not sure which is the saner default for a random plugin
> compiled against older nbdkit. At least it's fairly easy to write a
> filter that changes the default from no-op to .pread or from .pread to
> no-op. Be thinking about that as you review the rest of the series.
I'm worried that we end up in the same situation that we do with
.zero: because we emulate it, we end up with a slow zero support,
whereas clients [well, Nir S. mainly] actually want
NBD_FLAG_SEND_WRITE_ZEROES == *fast* zeroing is available.
Yes, I still need to revisit my NBD protocol proposal for an additional
flag during WRITE_ZEROES for a client to easily get fast failure if it
would otherwise be emulated. It may make .can_zero turn into tri-state :)
In particular in the NBD_CMD_CACHE case: for some plugins emulating
this behind the scenes with pread makes some sense. eg. It works for
nbdkit-file-plugin because it will prefetch the data into Linux's page
cache. However if a plugin makes a network access,
eg. nbdkit-vddk-plugin, that can be both very expensive and has no
prefetching benefit.
Therefore I do think we should default to ignoring NBD_CMD_CACHE
commands and *not* advertising the feature, except when the plugin
says that it is able to handle it.
For tri-stating .can_cache: yes that makes sense, because there is
moderate difficulty for a plugin to emulate .cache using .pread. The
plugin has to at least allocate a dummy buffer and throw it away, and
we could probably do something more efficient than that if we
implement an emulate mode in the server.
Okay, I'll rework the series along those lines, where .can_cache has to
be set for a plugin/filter to opt-in to advertising the feature to
clients (rather than always blindly advertising it).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization:
qemu.org |
libvirt.org