Re: [Libguestfs] [PATCH nbdkit v2 03/10] python: Implement nbdkit API version 2.
On Fri, Nov 22, 2019 at 10:55 PM Eric Blake <eblake(a)redhat.com> 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