[PATCH nbdkit] python: Pass memoryview to pwrite()
by Nir Soffer
Passing a memoryview we avoid unneeded copy of the original buffer. On
the python side memoryview object can be used for slicing, writing to
file, or sending to socket.
This may break plugins assuming that the they get a bytearray, but
good python code should not care about the type of the buffer, only
about the behaviour.
Testing with a plugin writing to /dev/null shows 2.7x speedup. Real
plugin will probably show much smaller improvement.
Without patch:
$ time qemu-img convert -p -f raw -O raw -n /var/tmp/disk.img nbd://localhost/
(100.00/100%)
real 0m1.284s
user 0m0.091s
sys 0m0.576s
With patch:
$ time qemu-img convert -p -f raw -O raw -n /var/tmp/disk.img nbd://localhost/
(100.00/100%)
real 0m0.477s
user 0m0.078s
sys 0m0.653s
---
More info on how I tested this:
# Creating test image
$ dd if=/dev/zero bs=1M count=1024 | tr "\0" "U" > /var/tmp/disk.img
$ cat zero.py
import builtins
def open(readonly):
return builtins.open("/dev/zero", "r+b")
def get_size(h):
return 1024**3
def pwrite(h, buf, offset):
h.write(buf)
def pread(h, count, offset):
raise NotImplementedError
def close(h):
h.close()
# Running nbdkit
$ ./nbdkit -f -v python zero.py
plugins/python/python.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/plugins/python/python.c b/plugins/python/python.c
index 214fffb..2b4361a 100644
--- a/plugins/python/python.c
+++ b/plugins/python/python.c
@@ -496,8 +496,8 @@ py_pwrite (void *handle, const void *buf,
PyErr_Clear ();
r = PyObject_CallFunction (fn, "ONL", obj,
- PyByteArray_FromStringAndSize (buf, count),
- offset, NULL);
+ PyMemoryView_FromMemory ((char *)buf, count, PyBUF_READ),
+ offset, NULL);
Py_DECREF (fn);
if (check_python_failure ("pwrite") == -1)
return -1;
--
2.21.0
4 years, 5 months
[PATCH nbdkit v3 0/7] Implement nbdkit API v2 for Python plugins.
by Richard W.M. Jones
v2 was here:
https://www.redhat.com/archives/libguestfs/2019-November/msg00163.html
I pushed patch 1 (with spelling fix), patch 4 and patch 5 since those
were previously ACKed on the list.
Differences in v3:
- Add error checking to PyModule_AddIntConstant.
- Use API_VERSION constant instead of function.
- Add max API version supported to --dump-plugin output.
- Print API_VERSION selected by the module in debug output.
- Allow .cache to be used from v1 API. Since it's a newly added
function we just use the same API as v2.
Differences in tests patch:
- converted the test suite to use unittest
- use base64 instead of codecs module
(also this means we strip whitespace around pickled+base64 string)
- formatting to avoid long lines
- whitespace consistency
- remove "python" mention in test-lang-plugins.c since it's no longer used
- remove #! from test-python-plugin.py as it is not needed
- add tests of pread + various buffer types
Rich.
4 years, 6 months
[PATCH nbdkit] python: Print readable tracebacks
by Nir Soffer
We used traceback.format_exception, returning traceback lines. Join the
lines to make the tracebacks more readable.
Here is an example traceback:
$ ./nbdkit -f -v python tests/python-exception.py
...
nbdkit: debug: python: config_complete
nbdkit: error: tests/python-exception.py: config_complete: error: Traceback (most recent call last):
File "tests/python-exception.py", line 51, in config_complete
raise RuntimeError("this is the test string")
RuntimeError: this is the test string
---
plugins/python/python.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/plugins/python/python.c b/plugins/python/python.c
index 148097f..71fa2e3 100644
--- a/plugins/python/python.c
+++ b/plugins/python/python.c
@@ -167,7 +167,7 @@ print_python_traceback (const char *callback,
type, error, traceback, NULL);
if (rv == NULL)
return -1;
- traceback_str = PyObject_Str (rv);
+ traceback_str = PyUnicode_Join (NULL, rv);
Py_DECREF (rv);
traceback_cstr = python_to_string (traceback_str);
if (traceback_cstr == NULL) {
--
2.21.0
4 years, 6 months
[PATCH] python: Support buffer protocol for pread() result
by Nir Soffer
pread() can return now any object supporting the buffer protocol[1] such
as bytearray, bytes, or memoryview.
[1] https://docs.python.org/3/c-api/buffer.html
---
When the new python tests will be in master we can add some automated tests.
For now I tested this manually using the example plugin as is and with
these changes:
def pread(h, count, offset):
global disk
return bytes(disk[offset:offset+count])
def pread(h, count, offset):
global disk
return memoryview(disk)[offset:offset+count]
plugins/python/python.c | 31 +++++++++++++++++++------------
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/plugins/python/python.c b/plugins/python/python.c
index 148097f..ed299ff 100644
--- a/plugins/python/python.c
+++ b/plugins/python/python.c
@@ -445,10 +445,12 @@ py_pread (void *handle, void *buf,
PyObject *obj = handle;
PyObject *fn;
PyObject *r;
+ Py_buffer view = {0};
+ int ret = -1;
if (!callback_defined ("pread", &fn)) {
nbdkit_error ("%s: missing callback: %s", script, "pread");
- return -1;
+ return ret;
}
PyErr_Clear ();
@@ -456,25 +458,30 @@ py_pread (void *handle, void *buf,
r = PyObject_CallFunction (fn, "OiL", obj, count, offset, NULL);
Py_DECREF (fn);
if (check_python_failure ("pread") == -1)
- return -1;
+ return ret;
- if (!PyByteArray_Check (r)) {
- nbdkit_error ("%s: value returned from pread method is not a byte array",
+ if (PyObject_GetBuffer (r, &view, PyBUF_SIMPLE) == -1) {
+ nbdkit_error ("%s: value returned from pread does not support the "
+ "buffer protocol",
script);
- Py_DECREF (r);
- return -1;
+ goto out;
}
- if (PyByteArray_Size (r) < count) {
- nbdkit_error ("%s: byte array returned from pread is too small", script);
- Py_DECREF (r);
- return -1;
+ if (view.len < count) {
+ nbdkit_error ("%s: buffer returned from pread is too small", script);
+ goto out;
}
- memcpy (buf, PyByteArray_AsString (r), count);
+ memcpy (buf, view.buf, count);
+ ret = 0;
+
+out:
+ if (view.obj)
+ PyBuffer_Release (&view);
+
Py_DECREF (r);
- return 0;
+ return ret;
}
static int
--
2.21.0
4 years, 6 months
[nbdkit PATCH] nbd: Add vsock_cid= transport option
by Eric Blake
With new enough libnbd, we already support vsock by virtue of uri=;
however, it's also nice to allow direct exposure of the
nbd_connect_vsock() api.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
As with commit 7ce9feef, there is no easy way to add testsuite
coverage for this.
plugins/nbd/nbdkit-nbd-plugin.pod | 30 +++++++++-----
plugins/nbd/nbd.c | 65 ++++++++++++++++++++++++-------
2 files changed, 72 insertions(+), 23 deletions(-)
diff --git a/plugins/nbd/nbdkit-nbd-plugin.pod b/plugins/nbd/nbdkit-nbd-plugin.pod
index 618058ac..151590d2 100644
--- a/plugins/nbd/nbdkit-nbd-plugin.pod
+++ b/plugins/nbd/nbdkit-nbd-plugin.pod
@@ -4,9 +4,9 @@ nbdkit-nbd-plugin - nbdkit nbd plugin
=head1 SYNOPSIS
- nbdkit nbd { socket=SOCKNAME | hostname=HOST [port=PORT] | [uri=]URI }
- [export=NAME] [retry=N] [shared=BOOL] [tls=MODE] [tls-certificates=DIR]
- [tls-verify=BOOL] [tls-username=NAME] [tls-psk=FILE]
+ nbdkit nbd { socket=SOCKNAME | { hostname=HOST | vsock_cid=CID } [port=PORT] |
+ [uri=]URI } [export=NAME] [retry=N] [shared=BOOL] [tls=MODE]
+ [tls-certificates=DIR] [tls-verify=BOOL] [tls-username=NAME] [tls-psk=FILE]
=head1 DESCRIPTION
@@ -27,8 +27,8 @@ TCP and use plaintext only on a Unix socket.
=head1 PARAMETERS
-One of B<socket>, B<hostname> or B<uri> must be provided to designate
-the server. The server can speak either new or old style
+One of B<socket>, B<hostname>, B<vsock_cid> or B<uri> must be provided
+to designate the server. The server can speak either new or old style
protocol. C<uri=> is a magic config key and may be omitted in most
cases. See L<nbdkit(1)/Magic parameters>.
@@ -47,8 +47,10 @@ Connect to the NBD server at the given remote C<HOST> using a TCP socket.
=item B<port=>PORT
-When B<hostname> is supplied, use B<PORT> instead of the default port
-10809.
+When B<hostname> or B<vsock_cid> is supplied, use B<PORT> instead of
+the default port 10809. For TCP, the port may be a 16-bit number or a
+non-numeric string used to look up the well-known port for a service
+name; for VSOCK, the port must be a 32-bit number.
=item B<export=>NAME
@@ -84,13 +86,23 @@ against libnbd:
When B<uri> is supplied, decode B<URI> to determine the address to
connect to. A URI can specify a TCP connection (such as
-C<nbd://localhost:10809/export>) or a Unix socket (such as
-C<nbd+unix:///export?socket=/path/to/sock>). Remember to use proper
+C<nbd://localhost:10809/export>), a Unix socket (such as
+C<nbd+unix:///export?socket=/path/to/sock>), or a vsock connection
+(such as C<nbd+vsock:///2:10809/export>). Remember to use proper
shell quoting to prevent B<URI> from accidentally being handled as a
shell glob. The B<uri> parameter is only available when the plugin was
compiled against libnbd with URI support; C<nbdkit --dump-plugin nbd>
will contain C<libnbd_uri=1> if this is the case.
+=item B<vsock_cid=>CID
+
+Connect to the NBD server at the given vsock cid (for example, in a
+guest VM, using the cid 2 will connect to a server in the host). This
+only works for platforms with the C<AF_VSOCK> family of sockets and
+libnbd new enough to use it; C<nbdkit --dump-plugin nbd> will contain
+C<libnbd_vsock=1> if this is the case. For more details on AF_VSOCK,
+see L<nbdkit-service(1)/AF_VSOCK>.
+
=item B<tls=>MODE
Selects which TLS mode to use with the server. If no other tls option
diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c
index d020beec..0bb5ff9f 100644
--- a/plugins/nbd/nbd.c
+++ b/plugins/nbd/nbd.c
@@ -46,6 +46,7 @@
#include <semaphore.h>
#include <poll.h>
#include <fcntl.h>
+#include <sys/socket.h>
#include <libnbd.h>
@@ -56,6 +57,12 @@
#include "cleanup.h"
#include "utils.h"
+#if !defined AF_VSOCK || !LIBNBD_HAVE_NBD_CONNECT_VSOCK
+#define USE_VSOCK 0
+#else
+#define USE_VSOCK 1
+#endif
+
/* The per-transaction details */
struct transaction {
int64_t cookie;
@@ -80,8 +87,15 @@ static char *sockname;
/* Connect to server via TCP socket */
static const char *hostname;
+
+/* Valid with TCP or VSOCK */
static const char *port;
+/* Connect to server via AF_VSOCK socket */
+static const char *raw_cid;
+static uint32_t cid;
+static uint32_t vport;
+
/* Connect to server via URI */
static const char *uri;
@@ -116,10 +130,10 @@ nbdplug_unload (void)
}
/* Called for each key=value passed on the command line. This plugin
- * accepts socket=<sockname>, hostname=<hostname>/port=<port>, or
- * [uri=]<uri> (exactly one connection required), and optional
- * parameters export=<name>, retry=<n>, shared=<bool> and various
- * tls settings.
+ * accepts socket=<sockname>, hostname=<hostname>/port=<port>,
+ * vsock_cid=<cid>/port=<port>, or [uri=]<uri> (exactly one connection
+ * required), and optional parameters export=<name>, retry=<n>,
+ * shared=<bool> and various tls settings.
*/
static int
nbdplug_config (const char *key, const char *value)
@@ -137,6 +151,10 @@ nbdplug_config (const char *key, const char *value)
hostname = value;
else if (strcmp (key, "port") == 0)
port = value;
+ else if (strcmp (key, "vsock_cid") == 0 ||
+ strcmp (key, "vsock-cid") == 0 ||
+ strcmp (key, "cid") == 0)
+ raw_cid = value;
else if (strcmp (key, "uri") == 0)
uri = value;
else if (strcmp (key, "export") == 0)
@@ -198,12 +216,8 @@ nbdplug_config_complete (void)
if (sockname) {
struct sockaddr_un sock;
- if (hostname || port) {
- nbdkit_error ("cannot mix Unix socket and TCP hostname/port parameters");
- return -1;
- }
- else if (uri) {
- nbdkit_error ("cannot mix Unix socket and URI parameters");
+ if (hostname || port || raw_cid || uri) {
+ nbdkit_error ("cannot mix Unix socket with other transport parameters");
return -1;
}
if (strlen (sockname) > sizeof sock.sun_path) {
@@ -212,13 +226,27 @@ nbdplug_config_complete (void)
}
}
else if (hostname) {
- if (uri) {
- nbdkit_error ("cannot mix TCP hostname/port and URI parameters");
+ if (uri || raw_cid) {
+ nbdkit_error ("cannot mix TCP hostname with other transport parameters");
return -1;
}
if (!port)
port = "10809";
}
+ else if (raw_cid) {
+#if !USE_VSOCK
+ nbdkit_error ("libnbd was compiled without vsock support");
+ return -1;
+#else
+ if (uri) {
+ nbdkit_error ("cannot mix VSOCK with other transport parameters");
+ return -1;
+ }
+ if (nbdkit_parse_uint32_t ("vsock_cid", raw_cid, &cid) == -1 ||
+ nbdkit_parse_uint32_t ("port", port, &vport) == -1)
+ return -1;
+#endif
+ }
else if (uri) {
struct nbd_handle *nbd = nbd_create ();
@@ -237,7 +265,8 @@ nbdplug_config_complete (void)
}
nbd_close (nbd);
} else {
- nbdkit_error ("must supply socket=, hostname= or uri= of external NBD server");
+ nbdkit_error ("must supply socket=, hostname=, vsock_cid= or uri= of "
+ "external NBD server");
return -1;
}
@@ -271,7 +300,8 @@ nbdplug_config_complete (void)
"[uri=]<URI> URI of an NBD socket to connect to (if supported).\n" \
"socket=<SOCKNAME> The Unix socket to connect to.\n" \
"hostname=<HOST> The hostname for the TCP socket to connect to.\n" \
- "port=<PORT> TCP port or service name to use (default 10809).\n" \
+ "vhost_cid=<CID> The cid for the VSOCK socket to connect to.\n" \
+ "port=<PORT> TCP/VHOST port or service name to use (default 10809).\n" \
"export=<NAME> Export name to connect to (default \"\").\n" \
"retry=<N> Retry connection up to N seconds (default 0).\n" \
"shared=<BOOL> True to share one server connection among all clients,\n" \
@@ -294,6 +324,7 @@ nbdplug_dump_plugin (void)
printf ("libnbd_version=%s\n", nbd_get_version (nbd));
printf ("libnbd_tls=%d\n", nbd_supports_tls (nbd));
printf ("libnbd_uri=%d\n", nbd_supports_uri (nbd));
+ printf ("libnbd_vsock=%d\n", USE_VSOCK);
nbd_close (nbd);
}
@@ -484,6 +515,12 @@ nbdplug_open_handle (int readonly)
r = nbd_connect_uri (h->nbd, uri);
else if (sockname)
r = nbd_connect_unix (h->nbd, sockname);
+ else if (raw_cid)
+#if !USE_VSOCK
+ abort ();
+#else
+ r = nbd_connect_vsock (h->nbd, cid, vport);
+#endif
else
r = nbd_connect_tcp (h->nbd, hostname, port);
if (r == -1) {
--
2.21.0
4 years, 6 months
Re: [Libguestfs] Dealing with ImageIO problems
by Richard W.M. Jones
[Adding the mailing list address back]
On Thu, Nov 21, 2019 at 12:44:38PM +0200, Nir Soffer wrote:
> On Thu, Nov 21, 2019 at 12:16 PM Richard W.M. Jones <rjones(a)redhat.com> wrote:
> >
> > On Wed, Nov 20, 2019 at 06:00:52PM +0200, Nir Soffer wrote:
> > > 5. can_zero fails, not sure why (Maybe Richard knows more about this)
> > >
> > > nbdkit: python[1]: debug: python: can_zero
> > > nbdkit: python[1]: debug: sending error reply: Operation not supported
> > >
> > > rhv-upload-plugin always reports that it can zero, I cannot explain this.
> >
> > I believe this is a bug in nbdkit’s python bindings. Will take a
> > closer look and reply in a new thread. I don't believe it affects
> > anything here however.
> >
> > > 6. writing the first sector fails
> > >
> > > python[1]: nbdkit: debug: vddk[3]python: pwrite count=65536 offset=0 fua=0:
> > > ...
> > > nbdkit: python[1]: error:
> > > /var/tmp/rhvupload.RShGz0/rhv-upload-plugin.py: pwrite: error:
> > > ['Traceback (most recent call last):\n', ' File
> > > "/var/tmp/rhvupload.RShGz0/rhv-upload-plugin.py", line 189, in
> > > pwrite\n http.endheaders()\n', ' File
> > > "/usr/lib64/python3.7/http/client.py", line 1247, in endheaders\n
> > > self._send_output(message_body, encode_chunked=encode_chunked)\n', '
> > > File "/usr/lib64/python3.7/http/client.py", line 1026, in
> > > _send_output\n self.send(msg)\n', ' File
> > > "/usr/lib64/python3.7/http/client.py", line 966, in send\n
> > > self.connect()\n', ' File
> > > "/var/tmp/rhvupload.RShGz0/rhv-upload-plugin.py", line 357, in
> > > connect\n self.sock.connect(self.path)\n', 'ConnectionRefusedError:
> > > [Errno 111] Connection refused\n']
> > >
> > > But it fails in send(), and we don't handle errors proeply around
> > > send(), only around getresponse().
> > > So the error is forgotten by the plugin.
> >
> > That sounds bad?
>
> Indeed, but we never had this issue in real environment.
>
> Basically we should handle any exception in the plugin as failure,
> but I'm not sure where we should fix it.
>
> We can add error handler around all the entry points
> (e.g. pwrite()), setting h['failed'] = True. But this makes it
> harder to write correct plugins; everyone has to ensure correct
> error handling instead of getting it right by default.
In nbdkit-python-plugin, any exception which escapes will be caught in
the C code and turned into an NBD error on the wire. The function
which does this is:
https://github.com/libguestfs/nbdkit/blob/a52ebb4e8d18a73b9ae009cd768bfab...
Since each plugin method corresponds* (* = mostly) to a single NBD
command on the wire, returning an error from the method signals an
error back to the NBD client. If the NBD client is ‘qemu-img convert’
then it will exit the first time it sees an error.
(The retry filter http://libguestfs.org/nbdkit-retry-filter.1.html
changes this behaviour, but we are not using that for output to
rhv-upload.)
> We can handle this in the python plugin (e.g. h['failed'] = True)
> but the handle is controlled by the plugin, so this does not sound
> like a good way.
The close() method is always called, whether or not there was an
error. The same connection handle (‘h’) is passed back to the close
method. It's quite safe and normal to store state in this handle.
nbdkit itself doesn't save the fact that there was an error during the
connection (it can't since it doesn't know if the error was
recoverable or not). But rhv-upload-plugin needs to handle failed vs
successful transfers differently. Thus we save the failure state in
the handle the first time we see an error.
> Maybe we can change close() to:
>
> def close(failed=True):
> if failed:
> # cleanup after some call failed
> ...
>
> # Cleanup after successful run
>
> The python plugin will remember unhandled errors and will call:
>
> close(failed=True)
>
> To write correct plugin author must not handle fatal errors in the plugin.
This changes how the Python plugin would work versus other plugins,
and the C code can't know if the NBD connection failed, because some
NBD errors are recoverable. Also the client does not send us any
error indication when it disconnects.
The answer is probably to wrap every method in a big try...except
which sets the failed flag in the handle.
> How do we handle errors in other plugins?
It's kind of easier to study this using the sh plugin. Here's an
example showing the order in which callbacks happen on error:
----------------------------------------------------------------------
nbdkit sh -fv - --run 'qemu-img convert $nbd /tmp/out' <<'EOF'
case "$1" in
get_size) echo 512 ;;
pread) echo 'EIO error' >&2; exit 1 ;;
*) exit 2 ;;
esac
EOF
----------------------------------------------------------------------
Running this gives the following output (filtered a bit to show the
key points):
$ bash test.sh
nbdkit: debug: sh: load
nbdkit: debug: sh: load: tmpdir: /tmp/nbdkitshB6KCs0
nbdkit: debug: sh: config key=script, value=-
nbdkit: debug: sh: config_complete
nbdkit: sh[1]: debug: sh: open readonly=0
nbdkit: sh[1]: debug: calling: /tmp/nbdkitshB6KCs0/inline-script.sh open false ""
nbdkit: sh[1]: debug: completed: /tmp/nbdkitshB6KCs0/inline-script.sh open: status 2
nbdkit: sh[1]: debug: sh: open returned handle 0x7face4002070
nbdkit: sh[1]: debug: sh: get_size
nbdkit: sh[1]: debug: calling: /tmp/nbdkitshB6KCs0/inline-script.sh get_size ""
nbdkit: sh[1]: debug: completed: /tmp/nbdkitshB6KCs0/inline-script.sh get_size: status 0
nbdkit: sh[1]: debug: sh: can_write
nbdkit: sh[1]: debug: calling: /tmp/nbdkitshB6KCs0/inline-script.sh can_write ""
nbdkit: sh[1]: debug: completed: /tmp/nbdkitshB6KCs0/inline-script.sh can_write: status 2
...
nbdkit: sh[1]: debug: sh: pread count=512 offset=0
nbdkit: sh[1]: debug: calling: /tmp/nbdkitshB6KCs0/inline-script.sh pread "" 512 0
nbdkit: sh[1]: debug: completed: /tmp/nbdkitshB6KCs0/inline-script.sh pread: status 1
nbdkit: sh[1]: error: /tmp/nbdkitshB6KCs0/inline-script.sh: error
nbdkit: sh[1]: debug: sending error reply: Input/output error
nbdkit: sh[1]: debug: client sent NBD_CMD_DISC, closing connection
qemu-img: Could not open 'nbd:localhost:10809': Could not read image for determining its format: Input/output error
nbdkit: sh[1]: debug: sh: close
nbdkit: sh[1]: debug: calling: /tmp/nbdkitshB6KCs0/inline-script.sh close ""
nbdkit: sh[1]: debug: completed: /tmp/nbdkitshB6KCs0/inline-script.sh close: status 2
nbdkit: debug: sh: unload plugin
nbdkit: debug: calling: /tmp/nbdkitshB6KCs0/inline-script.sh unload
nbdkit: debug: completed: /tmp/nbdkitshB6KCs0/inline-script.sh unload: status 2
Notice in particular how close() and unload() are both called in the
plugin even though qemu-img convert has already exited with an error.
Also the same handle (empty string above) is passed back to close().
> We probably should continue to the mailing list.
>
> Nir
Thanks, 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
4 years, 6 months
[PATCH] rhv-upload: Handle any error in NBD handlers
by Nir Soffer
Currently we handle only HTTP errors like getting unexpected response
code from imageio server. However if we could not send the request, or
some other error is raised, we failed to mark the handle as failed, and
finalized the transfer in close(). This may fool virt-v2v to create a VM
with an empty or partially uploaded disk.
Decorate all the NBD hander functions with a @failing decorator,
ensuring that any error in the decorated function will mark the
handle as failed.
Since errors are handled now by @failing decorator, remove the code to
mark a handle as failed in request_failed().
---
I tested 3 flows by injecting errors in imageio server:
## Error in flush()
nbdkit: python[1]: debug: pwrite count=65536 offset=6442385408 fua=0
(100.00/100%)
nbdkit: python[1]: debug: flush
unexpected response from imageio server:
could not flush
403: Forbidden
b'no flush for you!'
nbdkit: python[1]: error: /home/nsoffer/src/virt-v2v/tmp/rhvupload.bwlJfk/rhv-upload-plugin.py: flush: error: ['Traceback (most recent call last):\n', ' File "/home/nsoffer/src/virt-v2v/tmp/rhvupload.bwlJfk/rhv-upload-plugin.py", line 86, in wrapper\n return func(h, *args)\n', ' File "/home/nsoffer/src/virt-v2v/tmp/rhvupload.bwlJfk/rhv-upload-plugin.py", line 323, in flush\n request_failed(r, "could not flush")\n', ' File "/home/nsoffer/src/virt-v2v/tmp/rhvupload.bwlJfk/rhv-upload-plugin.py", line 165, in request_failed\n raise RuntimeError("%s: %d %s: %r" % (msg, status, reason, body[:200]))\n', "RuntimeError: could not flush: 403 Forbidden: b'no flush for you!'\n"]
nbdkit: python[1]: debug: sending error reply: Input/output error
nbdkit: python[1]: debug: flush
unexpected response from imageio server:
could not flush
403: Forbidden
b'no flush for you!'
nbdkit: python[1]: error: /home/nsoffer/src/virt-v2v/tmp/rhvupload.bwlJfk/rhv-upload-plugin.py: flush: error: ['Traceback (most recent call last):\n', ' File "/home/nsoffer/src/virt-v2v/tmp/rhvupload.bwlJfk/rhv-upload-plugin.py", line 86, in wrapper\n return func(h, *args)\n', ' File "/home/nsoffer/src/virt-v2v/tmp/rhvupload.bwlJfk/rhv-upload-plugin.py", line 323, in flush\n request_failed(r, "could not flush")\n', ' File "/home/nsoffer/src/virt-v2v/tmp/rhvupload.bwlJfk/rhv-upload-plugin.py", line 165, in request_failed\n raise RuntimeError("%s: %d %s: %r" % (msg, status, reason, body[:200]))\n', "RuntimeError: could not flush: 403 Forbidden: b'no flush for you!'\n"]
nbdkit: python[1]: debug: sending error reply: Input/output error
virtual copying rate: 3992.5 M bits/sec
nbdkit: python[1]: debug: client sent NBD_CMD_DISC, closing connection
nbdkit: python[1]: debug: close
canceling transfer a46a6646-6eb5-44c9-8234-de5b1779daa5
## Error in pwrite()
nbdkit: python[1]: debug: pwrite count=65536 offset=0 fua=0
unexpected response from imageio server:
could not write sector offset 0 size 65536
403: Forbidden
b'no put for you!'
nbdkit: python[1]: error: /home/nsoffer/src/virt-v2v/tmp/rhvupload.mVaSya/rhv-upload-plugin.py: pwrite: error: ['Traceback (most recent call last):\n', ' File "/home/nsoffer/src/virt-v2v/tmp/rhvupload.mVaSya/rhv-upload-plugin.py", line 86, in wrapper\n return func(h, *args)\n', ' File "/home/nsoffer/src/virt-v2v/tmp/rhvupload.mVaSya/rhv-upload-plugin.py", line 218, in pwrite\n (offset, count))\n', ' File "/home/nsoffer/src/virt-v2v/tmp/rhvupload.mVaSya/rhv-upload-plugin.py", line 165, in request_failed\n raise RuntimeError("%s: %d %s: %r" % (msg, status, reason, body[:200]))\n', "RuntimeError: could not write sector offset 0 size 65536: 403 Forbidden: b'no put for you!'\n"]
nbdkit: python[1]: debug: sending error reply: Input/output error
qemu-img: error while writing sector 0: Input/output error
nbdkit: python[1]: debug: flush
nbdkit: python[1]: debug: flush
nbdkit: python[1]: debug: client sent NBD_CMD_DISC, closing connection
nbdkit: python[1]: debug: close
canceling transfer d12874ff-be5a-4db2-b911-2b125f004b4c
virt-v2v: error: qemu-img command failed, see earlier errors
## Error to connect to unix socket.
Simulated by changing reporting non existing socket path.
nbdkit: python[1]: debug: pwrite count=65536 offset=0 fua=0
nbdkit: python[1]: error: /home/nsoffer/src/virt-v2v/tmp/rhvupload.YOMKDV/rhv-upload-plugin.py: pwrite: error: ['Traceback (most recent call last):\n', ' File "/home/nsoffer/src/virt-v2v/tmp/rhvupload.YOMKDV/rhv-upload-plugin.py", line 86, in wrapper\n return func(h, *args)\n', ' File "/home/nsoffer/src/virt-v2v/tmp/rhvupload.YOMKDV/rhv-upload-plugin.py", line 207, in pwrite\n http.endheaders()\n', ' File "/usr/lib64/python3.7/http/client.py", line 1247, in endheaders\n self._send_output(message_body, encode_chunked=encode_chunked)\n', ' File "/usr/lib64/python3.7/http/client.py", line 1026, in _send_output\n self.send(msg)\n', ' File "/usr/lib64/python3.7/http/client.py", line 966, in send\n self.connect()\n', ' File "/home/nsoffer/src/virt-v2v/tmp/rhvupload.YOMKDV/rhv-upload-plugin.py", line 378, in connect\n self.sock.connect(self.path)\n', 'ConnectionRefusedError: [Errno 111] Connection refused\n']
nbdkit: python[1]: debug: sending error reply: Input/output error
qemu-img: error while writing sector 0: Input/output error
nbdkit: python[1]: debug: flush
nbdkit: python[1]: error: /home/nsoffer/src/virt-v2v/tmp/rhvupload.YOMKDV/rhv-upload-plugin.py: flush: error: ['Traceback (most recent call last):\n', ' File "/home/nsoffer/src/virt-v2v/tmp/rhvupload.YOMKDV/rhv-upload-plugin.py", line 86, in wrapper\n return func(h, *args)\n', ' File "/home/nsoffer/src/virt-v2v/tmp/rhvupload.YOMKDV/rhv-upload-plugin.py", line 319, in flush\n http.request("PATCH", h[\'path\'], body=buf, headers=headers)\n', ' File "/usr/lib64/python3.7/http/client.py", line 1252, in request\n self._send_request(method, url, body, headers, encode_chunked)\n', ' File "/usr/lib64/python3.7/http/client.py", line 1263, in _send_request\n self.putrequest(method, url, **skips)\n', ' File "/usr/lib64/python3.7/http/client.py", line 1108, in putrequest\n raise CannotSendRequest(self.__state)\n', 'http.client.CannotSendRequest: Request-sent\n']
nbdkit: python[1]: debug: sending error reply: Input/output error
nbdkit: python[1]: debug: flush
nbdkit: python[1]: error: /home/nsoffer/src/virt-v2v/tmp/rhvupload.YOMKDV/rhv-upload-plugin.py: flush: error: ['Traceback (most recent call last):\n', ' File "/home/nsoffer/src/virt-v2v/tmp/rhvupload.YOMKDV/rhv-upload-plugin.py", line 86, in wrapper\n return func(h, *args)\n', ' File "/home/nsoffer/src/virt-v2v/tmp/rhvupload.YOMKDV/rhv-upload-plugin.py", line 319, in flush\n http.request("PATCH", h[\'path\'], body=buf, headers=headers)\n', ' File "/usr/lib64/python3.7/http/client.py", line 1252, in request\n self._send_request(method, url, body, headers, encode_chunked)\n', ' File "/usr/lib64/python3.7/http/client.py", line 1263, in _send_request\n self.putrequest(method, url, **skips)\n', ' File "/usr/lib64/python3.7/http/client.py", line 1108, in putrequest\n raise CannotSendRequest(self.__state)\n', 'http.client.CannotSendRequest: Request-sent\n']
nbdkit: python[1]: debug: sending error reply: Input/output error
nbdkit: python[1]: debug: client sent NBD_CMD_DISC, closing connection
nbdkit: python[1]: debug: close
canceling transfer a3a1d037-c15e-4b70-b8f9-3a809d690be9
v2v/rhv-upload-plugin.py | 45 +++++++++++++++++++++++++++++-----------
1 file changed, 33 insertions(+), 12 deletions(-)
diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py
index 43fea18b..7ed521f3 100644
--- a/v2v/rhv-upload-plugin.py
+++ b/v2v/rhv-upload-plugin.py
@@ -17,6 +17,7 @@
# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
import builtins
+import functools
import json
import logging
import socket
@@ -72,6 +73,22 @@ def parse_username():
parsed = urlparse(params['output_conn'])
return parsed.username or "admin@internal"
+def failing(func):
+ """
+ Decorator marking the handle as failed if any expection is raised in the
+ decorated function. This is used in close() to cleanup properly after
+ failures.
+ """
+ @functools.wraps(func)
+ def wrapper(h, *args):
+ try:
+ return func(h, *args)
+ except:
+ h['failed'] = True
+ raise
+
+ return wrapper
+
def open(readonly):
connection = sdk.Connection(
url = params['output_conn'],
@@ -115,22 +132,21 @@ def open(readonly):
'path': destination_url.path,
}
+@failing
def can_trim(h):
return h['can_trim']
+@failing
def can_flush(h):
return h['can_flush']
+@failing
def get_size(h):
return params['disk_size']
# Any unexpected HTTP response status from the server will end up calling this
-# function which logs the full error, sets the failed state, and raises a
-# RuntimeError exception.
-def request_failed(h, r, msg):
- # Setting the failed flag in the handle will cancel the transfer on close.
- h['failed'] = True
-
+# function which logs the full error, and raises a RuntimeError exception.
+def request_failed(r, msg):
status = r.status
reason = r.reason
try:
@@ -152,6 +168,7 @@ def request_failed(h, r, msg):
# For examples of working code to read/write from the server, see:
# https://github.com/oVirt/ovirt-imageio/blob/master/daemon/test/server_tes...
+@failing
def pread(h, count, offset):
http = h['http']
transfer = h['transfer']
@@ -165,12 +182,13 @@ def pread(h, count, offset):
r = http.getresponse()
# 206 = HTTP Partial Content.
if r.status != 206:
- request_failed(h, r,
+ request_failed(r,
"could not read sector offset %d size %d" %
(offset, count))
return r.read()
+@failing
def pwrite(h, buf, offset):
http = h['http']
transfer = h['transfer']
@@ -194,12 +212,13 @@ def pwrite(h, buf, offset):
r = http.getresponse()
if r.status != 200:
- request_failed(h, r,
+ request_failed(r,
"could not write sector offset %d size %d" %
(offset, count))
r.read()
+@failing
def zero(h, count, offset, may_trim):
http = h['http']
@@ -223,7 +242,7 @@ def zero(h, count, offset, may_trim):
r = http.getresponse()
if r.status != 200:
- request_failed(h, r,
+ request_failed(r,
"could not zero sector offset %d size %d" %
(offset, count))
@@ -257,12 +276,13 @@ def emulate_zero(h, count, offset):
r = http.getresponse()
if r.status != 200:
- request_failed(h, r,
+ request_failed(r,
"could not write zeroes offset %d size %d" %
(offset, count))
r.read()
+@failing
def trim(h, count, offset):
http = h['http']
@@ -279,12 +299,13 @@ def trim(h, count, offset):
r = http.getresponse()
if r.status != 200:
- request_failed(h, r,
+ request_failed(r,
"could not trim sector offset %d size %d" %
(offset, count))
r.read()
+@failing
def flush(h):
http = h['http']
@@ -298,7 +319,7 @@ def flush(h):
r = http.getresponse()
if r.status != 200:
- request_failed(h, r, "could not flush")
+ request_failed(r, "could not flush")
r.read()
--
2.21.0
4 years, 6 months
Extending the nbdkit python plugin
by Richard W.M. Jones
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
4 years, 6 months
[PATCH 0/2] rhv-upload: Complete refactoring
by Nir Soffer
Fix NameError introduced by inocomplete change to extract create_transfer() by
extracting cancel_transfer() function.
Finally extract finallize_transfer() function, dealing with the poorly
documented and over complicated oVirt SDK.
Tested with:
libguestfs-1.40.2-4.fc30.x86_64
nbdkit-1.12.8-1.fc30.x86_64
Tested flows:
- Successful import
- Error in close() - by injecting long sleep in vdsm verifaction step
- Error in close() - by injecting error in vdsm verification step
- Error in open() - by injecting error in imageio OPTIONS handler
- Error in zero() - by injecting error in imageio PATCH handler
- Error in flush() - by injecting error in imageio PATCH handler
Nir Soffer (2):
rhv-upload: Extract cancel_transfer() function
rhv-upload: Extract finalize_transfer() function
v2v/rhv-upload-plugin.py | 118 ++++++++++++++++++++++++++-------------
1 file changed, 78 insertions(+), 40 deletions(-)
--
2.21.0
4 years, 6 months