[Libguestfs] Extending the nbdkit python plugin

[Daniel P. Berrangé]

On Thu, Nov 21, 2019 at 01:57:28PM +0000, Richard W.M. Jones wrote:
> We have an nbdkit plugin that lets you write NBD servers in Python.
> An example of an existing Python plugin is here:
> 
>   https://github.com/libguestfs/nbdkit/blob/master/plugins/python/example.py#L1
> 
> This morning I tried to modify the plugin to use the newer nbdkit API
> (version 2).  One of the things that would change would be passing
> flags parameters to some functions, eg:
> 
>   def pwrite (h, buf, offset):
> 
> might become one of these possibilities (where flags is a bitmask):
> 
>   def pwrite (h, buf, offset, flags):
>   def pwrite (h, buf, offset, flags=0):
> 
> The problem is if we did this it would break all existing Python
> plugins.  While we don't guarantee the nbdkit API for non-C languages,
> we do nevertheless have Python plugins that we care about such as the
> rhv-upload-plugin used by virt-v2v.  Having a flag day which breaks
> all existing plugins is very awkward.
> 
> I tried to simply pass the extra arguments from the C code to the
> Python code, and existing plugins break with:
> 
> nbdkit: python[1]: error: ./test.py: pwrite: error: pwrite() takes 3 positional arguments but 4 were given
> 
> One possibility is that we could introspect the plugin to find out how
> many parameters it takes.  This is possible, but very difficult from
> C.  (See https://stackoverflow.com/a/41188411 for how to do it from
> Python).

You can do the introspection in python instead of C.

Instead of having the C code directly call the python plugin code,
write a shim plugin impl that uses the new API contract. This shim
python plugin then introspects the real python plugin and calls it
with the appropriate API contract (if possible).

> Another possibility is we could encourage existing Python plugins to
> add **kwargs to all functions where we might plausibly add extra
> parameters in future, ie. the above would become:
> 
>   def pwrite (h, buf, offset, **kwargs):
> 
> This still requires all Python plugins to change, but at least they
> would remain backwards compatible with old and new nbdkit.  However I
> couldn't actually work out how to make this work because:
> 
>   >>> def test(a, **kwargs):
>   ...   pass
>   ... 
>   >>> test(1)
>   >>> test(1,2)
>   Traceback (most recent call last):
>     File "<stdin>", line 1, in <module>
>   TypeError: test() takes 1 positional argument but 2 were given
> 
> Yet another possibility is the Python plugin itself should declare
> which version of the API it wants, and the C code can then pass the
> correct parameters.  (This is in fact how nbdkit C plugins work).
> This pushes a bunch of work into the C code, but I guess we can deal
> with that.

I would define a new 'api_version()' method for your python
plugins.

Existing plugins don't implement that of course so when your
C code invokes that it'll get an exception, and can assume
the classic API contract for pwrite().

If the plugin implements  'api_version()' returning '1' (or
whatever), then this should inform the C code that the plugin
has the 4 arg variant of pwrite().

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|