On Sat, Nov 23, 2019 at 3:10 PM Richard W.M. Jones <rjones@redhat.com> wrote:
On Sat, Nov 23, 2019 at 01:42:15AM +0200, Nir Soffer wrote:
On Fri, Nov 22, 2019 at 9:55 PM Richard W.M. Jones <rjones@redhat.com> wrote:
+def pread(h, count, offset, flags): + assert flags == 0 + return h['disk'][offset:offset+count]
Very nice and simple test plugin!
But this returns always a bytearray, which is also what nbdkit python plugin expects. But real code using HTTPConnection return bytes:
c = http.client.HTTPSConnection("www.python.org") c.request("GET", "/") r = c.getresponse() r.read()[:10] b'<!doctype '
I think the plugin should support both bytearray, memoryview, or bytes. Supporting objects implementing the buffer protocol would be best.
I thought bytearray & bytes were the same thing :-/
Would adding this break existing nbdkit Python scripts? Should this be considered for v2 of the API? Are there performance implications / improvements from doing this?
There is performance implication for v1 plugins like rhv-upload-plugin that need to copy the bytes from imageio server on python side: r = http.getresponse() ... return bytearray(r.read()) This is sad because on the C side we mempcpy the data again. So with this patch we avoid one copy of the two. To avoid all unneeded copies, we need to change pread() to: def pread(h, buf, offset): So the python side we can do: f.readinto(buf) Or: sock.recv_info(buf) It does not work for HTTPResponse, so in this case we have to do: buf[:] = r.read() Since we work on v2 now, I think we should consider this change. An uglier alternative is: def preadinto(h, buf, offset): Matching python read() and readinto() interface.
test-python-plugin.py
This text can use python docstring:
""" This tests ... """
Good point, I'll fix this. We could probably also have nbdkit do something with the docstring, such as printing it in --help output, although that's something for another patch series.
+ h = nbd.NBD () + cfg = codecs.encode (pickle.dumps (cfg), "base64").decode()
base64.b64encode() is better, avoiding unwanted newlines.
Ah OK, I originally added strip(), but this is better.
+ cmd = ["nbdkit", "-v", "-s", "--exit-with-parent", + "python", srcdir + "/test-python-plugin.py", + "cfg=" + cfg] + h.connect_command (cmd) + return h + +# Test we can send an empty pickled test configuration and do nothing +# else. This is just to ensure the machinery of the test works. +h = test ({})
So we have now running nbdkit that will exit the python collects when h is implicitly closed when creating a new handle?
This is fragile, but can be solved with the help of a testing framework. [...] pytest test-python.py
I'll probably use unittest though because it's built into Python and because it's what we use in libguestfs, hivex etc but yes good idea.
Thanks,
Rich.
-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW