[Libguestfs] [nbdkit PATCH 0/7] Initial implementation of FUA flag passthrough

[Eric Blake]

On 01/16/2018 02:25 AM, Richard W.M. Jones wrote:
> I had an idea about how we might do this and still keep a single
> .pwrite method for plugins.  Maybe it's too complicated but here goes:
> 

> 
> This is binary-compatible with old plugins, but not source compatible, so:
> 
> 
> (2) Plugsin may optionally define NBDKIT_PLUGIN_LEVEL before including
> <nbdkit-plugin.h>.  Code wishing to use the flags variant of pwrite
> can do:
> 
> #define NBDKIT_PLUGIN_LEVEL 2
> #include <nbdkit-plugin.h>
> 
> which would modify the definition of the struct (again), something
> like:
> 
> #ifndef NBDKIT_PLUGIN_LEVEL
> #define NBDKIT_PLUGIN_LEVEL 1
> #endif

So the difference between your proposal and my patch 4 is that in your
proposal, a new plugin has to explicitly opt-in to the new ABI by
defining NBDKIT_PLUGIN_LEVEL 2, while in mine, a new plugin opts-in by
assigning to .pwrite_fua instead of .pwrite.  Yours is slightly cleaner
in making it obvious that there is only one (and not two) pwrite
callback per plugin, and either way, the plugin chooses which variant it
wants.

> (3) Core nbdkit code always defines NBDKIT_PLUGIN_LEVEL == 2 so that
> it always sees the new function, but it may need to call the old
> function:
> 
> int
> plugin_pwrite (...)
> {
>   if (plugin.pwrite != NULL)
>     plugin.pwrite (handle, buf, count, offset, flags);
>   else if (plugin.pwrite_old1 != NULL)
>     plugin.pwrite_old1 (handle, buf, count, offset);
> }

This part happens whether by your proposal or by mine (see patch 5) - a
new nbdkit is always able to call the correct one out of two callbacks
for both old and new plugins.

Both proposals are still able to install a shim for the old name when
the plugin uses only the new semantics; if I use your proposal, my patch
7 would be rewritten to only create the shims when a plugin requests
NBDKIT_PLUGIN_LEVEL 2 (whereas by my proposal, I had to define the shims
always).

Documentation may be a bit tricky under your proposal, but I'm not
seeing it as much different than mine.  Then there's the decision of
whether to go with a single flags parameter (closer to the wire
protocol, but the plugin has to decode bits; and hopefully we do proper
feature advertisement so that a client never passes a flag that the
plugin won't understand) or with multiple bool parameters (what my
proposal did for .pwrite_fua).

> 
> 
> (4) Internal plugins which don't need the FUA flag can continue to use
> the level 1 API, which means that we keep checking that the old API
> still works.

My proposal did that as well - most plugins still used the older
spelling without the fua flag.

> 
> 
> What do you think?

I don't see any fundamental differences in end behavior, so much as
which style we find nicer to document and expose to plugin writers.  I'm
happy to go with whichever of the two styles you prefer.

The other thing to consider is how we translate all of this to other
language bindings.  For languages that allow function overloading, I
suppose we want the declaration of 'pwrite' to take either function
signature, then the glue code that converts back to C knows based on the
function signature whether the plugin is written for old- or new-style
API; for languages without function overloading, they will have to
mirror some way of declaring which of the two APIs they are interested in.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20180116/f0e8430f/attachment.sig>