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 <[email protected]> >> >> --- >> >> 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
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
