On Wed, Sep 04, 2019 at 01:28:08PM -0500, Eric Blake wrote: > We want to default to requesting structured replies, whether or not > that request will be honored (it's essential for efficient sparse file > reads and the DF flag for structured pread, as well as for meta > context support even if we do not request a default meta context). > However, for integration testing, it can be nice to easily request a > client that does not request structured replies. > > We can test this by reusing our eflags test. Note that nbdkit does > not provide a 'can_df' callback in the sh script (so our key=value > override is silently ignored), rather, we control whether nbdkit > advertises df based on whether we request structured replies. > --- > > I'm open to renaming the API to the shorter 'nbd_set_request_sr' if > the existing name choice seems too long.
There's not really a limit on the length of API names, and in this case the longer name explains what the option does more clearly. Anyway this looks fine to me. ACK I have one comment unrelated to the patch: > + "set_request_structured_replies", { > + default_call with > + args = [Bool "request"]; ret = RErr; > + permitted_states = [ Created ]; > + first_version = (1, 2); I just know that we're going to end up adding new APIs and forgetting to set the first_version field. There are various things we could do to prevent this: (1) In ‘default_call’ set first_version = (0, 0). Update all existing calls with first_version = (1, 0). Then add a check that first_version <> (0, 0). There's still a danger of copy and paste from an existing API, but we should be able to catch that in review. (2) Store first_version in a separate table. Add checks to ensure the new table exhaustively covers all APIs. It should be obvious when submitting a new API that the first_version table must be updated and what to add here: let first_version = [ "set_debug", (1, 0); ... "set_request_structured_replies", (1, 2); "get_request_structured_replies", (1, 2); ] Not sure which is better. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs