7:57 a.m.
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.p...
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).
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.
So really this is a question directed at Python experts. What's the
right way to go about this?
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
8:13 a.m.
On 11/21/19 7:57 AM, 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.p...
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.
At one point, I tried using Python introspection to determine which
signature the user's function has, to determine if the user wrote
against the v1 protocol. It may also be possible to use python's keyword
argument parsing to pass all optional arguments like flags by key=value
pairs (allowing future addition of additional flags). I'll have to
revive my patches.
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).
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.
Yes, that's along the lines of the work I had attempted previously. But
I attempted it back when we were using Python2, so I'd have to rework it
for Python3.
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.
So really this is a question directed at Python experts. What's the
right way to go about this?
I'll try and revive my patches; I may be able to save you some time
based on what I already have in my tree.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
8:17 a.m.
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.p...
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 :|
8:51 a.m.
On Thu, Nov 21, 2019 at 02:17:43PM +0000, Daniel P. Berrangé wrote:
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().
I suspect this has to be the way to go. One reason is that
the V1 zero API is defined as:
def zero(h, count, offset, may_trim=False):
but in V2 we'd like to replace the fourth parameter with a flags
bitmask. I believe pure introspection can never detect this.
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
9:29 a.m.
On Thu, Nov 21, 2019 at 4:52 PM Richard W.M. Jones <rjones(a)redhat.com> wrote:
On Thu, Nov 21, 2019 at 02:17:43PM +0000, Daniel P. Berrangé wrote:
> 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().
I suspect this has to be the way to go. One reason is that
the V1 zero API is defined as:
def zero(h, count, offset, may_trim=False):
but in V2 we'd like to replace the fourth parameter with a flags
bitmask. I believe pure introspection can never detect this.
You can with:
def using_api_v2(module):
sig = inspect.signature(module.zero)
return "flags" in sig.parameters
See https://www.redhat.com/archives/libguestfs/2019-November/msg00130.html
using it to discover if ImageTransfer supports "format" argument.
But I would add:
API_VERSION = 2
And query it from the python plugin when importing the module.
Nir
9:49 a.m.
This may be confusing at first but is not that difficult once you wrap
your head around it.
There are two types of arguments in Python: positional(*) and
keyword(**). There is a rule that all positional arguments are defined
before the keyword arguments. Also once an argument has a default value
it is considered keyword (not positional). There are some nuances for
which is best to reach to some documentation.
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):
Here you defined single positional argument 'a'.
... pass
...
>>> test(1)
>>> test(1,2)
You are passing two positional arguments. A keyword argument has to have
a name. E.g. this will work: test(1, foo=2)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: test() takes 1 positional argument but 2 were given
Check the attached script for some examples. Hopefully that will help
you get a better grasp.
Tomas
--
Tomáš Golembiovský <tgolembi(a)redhat.com>
9:54 a.m.
On Thu, Nov 21, 2019 at 04:49:00PM +0100, Tomáš Golembiovský wrote:
This may be confusing at first but is not that difficult once you
wrap
your head around it.
There are two types of arguments in Python: positional(*) and
keyword(**). There is a rule that all positional arguments are defined
before the keyword arguments. Also once an argument has a default value
it is considered keyword (not positional). There are some nuances for
which is best to reach to some documentation.
Thanks for the explanation. I think the api_version idea is the way
to go, I have a patch for this coming up.
Rich.
> 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):
Here you defined single positional argument 'a'.
> ... pass
> ...
> >>> test(1)
> >>> test(1,2)
You are passing two positional arguments. A keyword argument has to have
a name. E.g. this will work: test(1, foo=2)
> Traceback (most recent call last):
> File "<stdin>", line 1, in <module>
> TypeError: test() takes 1 positional argument but 2 were given
>
Check the attached script for some examples. Hopefully that will help
you get a better grasp.
Tomas
--
Tomáš Golembiovský <tgolembi(a)redhat.com>
#!/usr/bin/env python3
def abc(*args, **kwargs):
print(args)
print(kwargs)
def with_positional(a, b, c, *args, **kwargs):
print(a, b, c)
print(args)
print(kwargs)
# Wrong: cannot define keyword arguments before positional
#def with_keyword(a=None, b=None, c=None, *args, **kwargs):
# pass
def with_keyword(*args, a=None, b=None, c=None, **kwargs):
print(a, b, c)
print(args)
print(kwargs)
# This is also possible
def with_keyword2(*args, a, b, c, **kwargs):
pass
print(a, b, c)
print(args)
print(kwargs)
# (1, 'abc')
# {'foo': 2, 'bar': 3}
abc(1, 'abc', foo=2, bar=3)
# Wrong: cannot pass positional arguments after keyword arguments
#abc(1, 'abc', foo=2, bar=3, 4, 5)
print('---')
# 1 2 3
# (4, 5)
# {'foo': 10, 'bar': 11}
with_positional(1, 2, 3, 4, 5, foo=10, bar=11)
# Wrong: a,b,c are alrady bound to 4,5,6
#with_positional(4, 5, 6, foo=10, bar=11, a=1, b=2, c=3)
# This works as one would expect
with_positional(a=1, b=2, c=3, foo=10, bar=11)
# These two are wrong
#with_positional(4, a=1, b=2, c=3, foo=10, bar=11)
#with_positional(a=1, b=2, c=3, 4, foo=10, bar=11)
print('---')
with_keyword(4, 5, foo=10, bar=11)
with_keyword(4, 5, foo=10, bar=11, a=1, b=2, c=3)
# These two are equivalent
with_keyword2(foo=10, bar=11, a=1, b=2, c=3)
with_keyword2(a=1, b=2, c=3, foo=10, bar=11)
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://libguestfs.org
1863
days inactive
1863
days old
6 comments
5 participants
participants (5)
-
Daniel P. Berrangé -
Eric Blake -
Nir Soffer -
Richard W.M. Jones -
Tomáš Golembiovský