On Fri, Jan 19, 2018 at 10:22:27AM -0600, Eric Blake wrote: > > +Suggestions for filters > > +----------------------- > > > > * adding artificial delays (see wdelay/rdelay options in the file > > plugin) > > How about a filter that intentionally disables WRITE_ZEROES and/or FUA > support, if for no other reason than for testing sane fallbacks and > comparing speed differences?
If you have ideas for the TODO file just go ahead and commit them, no need for review. > > +=head1 SYNOPSIS > > > +To see example filters, take a look at the source of nbdkit, in the > > +C<filters> directory. > > + > > +Filters must be written in C and must be fully thread safe. > > Should we mention that the rules on filter semantics are stricter than > for plugins, and that unlike plugins, we don't guarantee stable > cross-version API? Yes, we'll need to tighten up the language here. I noted your other patch which modified this text. > > +=head1 CALLBACKS > > > +=head2 C<.get_size> > > + > > + int64_t (*get_size) (struct nbdkit_next_ops *next_ops, void *nxdata, > > + void *handle); > > + > > +This intercepts the plugin C<.get_size> method and can be used to read > > +or modify the apparent size of the block device that the NBD client > > +will see. > > + > > +The returned size must be E<ge> 0. If there is an error, C<.get_size> > > +should call C<nbdkit_error> with an error message and return C<-1>. > > The rule on non-zero size matches plugins, but I'm not sure if there is > a technical reason why we have that restriction. Down the road, when I > get the upstream NBD resize proposal finalized, I can see the ability to > create an image with size 0, then use resize to update it, as something > that would be nice to support. OK that's interesting. BTW qemu has historically had lots of bugs in the block layer related to zero (or even < 4096 byte) devices. We added a workaround in libguestfs: https://github.com/libguestfs/libguestfs/blob/a30b51747fc6605f50a9d5d8095fd2b6d1473b1c/lib/drives.c#L395 > > + > > +=head2 C<.pread> > > + > > + int (*pread) (struct nbdkit_next_ops *next_ops, void *nxdata, > > + void *handle, void *buf, uint32_t count, uint64_t offset); > > Wrong signature, missing the uint32_t flags that the backend interface > has, and that I'm adding for plugins API_VERSION 2 for FUA support. Right, but this patch is against the version 1 API. We will need to add flags at some point. > > +=head1 THREADS > > + > > +Because filters can be mixed and used with any plugin and thus any > > +threading model supported by L<nbdkit-plugin(3)>, filters must be > > +thread safe. They must be able to handle concurrent requests even on > > +the same handle. > > + > > +Filters may have to use pthread primitives like mutexes to achieve > > +this. > > Would it ever make sense to allow a filter to intentionally request a > lower concurrency level than the underlying plugin? At connection time, > you'd compute the threading level as the lowest level requested by any > element of the chain; it might make it easier to write plugins that > aren't fully parallel (such filters may penalize the speed that can > otherwise be achieved by using the plugin in isolation - but again, it > may be useful for testing). That would imply adding a > backend.thread_model() callback, which needs to be exposed to filters > but not to plugins (plugins continue to use the #define at compile > time); if the filter does not define the .thread_model callback, it > defaults to fully-parallel; but if it does define the callback, it can > result in something less concurrent. Yes, this is a good idea actually. > > +=item B<--filter> FILTER > > + > > +Add a filter before the plugin. This option may be given one or more > > +times to stack filters in front of the plugin. They are processed in > > +the order they appear on the command line. See L</FILTERS> and > > +L<nbdkit-filter(3)>. > > + > > Does it ever make sense to provide the same filter more than once on the > command line? Or are we stating that a given filter can only be used > once in the chain? I couldn't think of a case where we'd need the same filter multiple times. It could be made to work but we'd have to ban global variables and modify the load() function. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/ _______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
