On Fri, Nov 22, 2019 at 10:55 PM Eric Blake <[email protected]> wrote: > > On 11/22/19 1:53 PM, Richard W.M. Jones wrote: > > To avoid breaking existing plugins, Python plugins wishing to use > > version 2 of the API must opt in by declaring: > > > > def api_version(): > > return 2 > > > > (Plugins which do not do this are assumed to want API version 1). > > Could we also permit the python code to declare a global variable > instead of a function? But a function is just fine (and easier to > integrate the way we do all our other callbacks). > > > > +++ b/plugins/python/example.py > > @@ -34,6 +34,13 @@ import errno > > disk = bytearray(1024 * 1024) > > > > > > +# There are several variants of the API. nbdkit will call this > > +# function first to determine which one you want to use. This is the > > +# latest version at the time this example was written. > > +def api_version(): > > + return 2 > > Matches the C counterpart of #define NBDKIT_API_VERSION 2 at the top of > a plugin.
This is the same thing I was thinking about. This makes it more clear that the api version is constant, and not something the plugin should change while it is being used. Same for all can_xxx functions, NBD does not support changing any of these anyway after negotiation. v2 is an opportunity to do this change if we want to. ... > > -def pread(h, count, offset): > > +def pread(h, count, offset, flags): > > global disk > > return disk[offset:offset+count] > > Do we really want to be passing 'flags' as an integer that the python > script must decode? Could we instead pass the flags as named kwargs? > For pread, there are no defined flags, so that would mean we stick with I was going to comment about the same thing. > > def pread(h, count, offset): > > > > > > > -def pwrite(h, buf, offset): > > +def pwrite(h, buf, offset, flags): > > but for pwrite, it would look like: > > def pwrite(h, buf, offset, fua=False): > > > > > -def zero(h, count, offset, may_trim): > > +def zero(h, count, offset, flags): > > and for zero (once fast zero is supported later in the series), it could > look like: > > def zero(h, count, offset, may_trim=False, fua=False, fast=False): This is nicer - but not extensible. > The user could also write: > > def zero(h, count, offset, **flags) This is less nice, but easy to extend without adding v3. But I would call it **options instead. > > and manually extract key/value pairs out of the flags, to be robust to > unknown flags (although our API somewhat promises that we never pass > flags to the data-handling callbacks unless the can_FOO callbacks > already indicated that the plugin was willing to support the flag) ... Nir _______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
