[nbdkit PATCH] server: Correct logic when filter fails .prepare
If .prepare fails, we do not want to call .finalize (similar to how if .open fails, we do not want to call .close). However, the logic in backend_finalize was checking on the wrong condition ('was the connection opened' rather than 'was the connection prepared'), which led to an assertion failure. Simple reproducer: $ nbdkit -U - -f --filter cache --run 'qemu-io -r -c quit $nbd' sh - <<\EOF
case $1 in get_size) echo oops >&2; exit 1 ;; *) exit 2 ;; esac EOF nbdkit: sh[1]: error: /tmp/nbdkitshYvAQbz/inline-script.sh: oops nbdkit: backend.c:206: backend_finalize: Assertion `h->state & HANDLE_CONNECTED' failed. qemu-io: can't open device nbd:unix:/tmp/nbdkit60FUTw/socket: Failed to read option reply: Unexpected end-of-file before all bytes were read nbdkit: nbdkit command was killed by signal 6
With this patch, the command now fails gracefully: nbdkit: sh[1]: error: /tmp/nbdkitshie18Lp/inline-script.sh: oops qemu-io: can't open device nbd:unix:/tmp/nbdkitqvAOlr/socket: Requested export not available Fixes: ffa98c8d Signed-off-by: Eric Blake <eblake@redhat.com> --- Pushing this to master and stable-1.16 server/backend.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/server/backend.c b/server/backend.c index 8bfa8525..753f5cca 100644 --- a/server/backend.c +++ b/server/backend.c @@ -221,15 +221,13 @@ backend_finalize (struct backend *b) if (h->state & HANDLE_FAILED) return -1; - if (h->handle) { - assert (h->state & HANDLE_CONNECTED); + assert (h->state & HANDLE_OPEN); + if (h->state & HANDLE_CONNECTED) { if (b->finalize (b, h->handle) == -1) { h->state |= HANDLE_FAILED; return -1; } } - else - assert (! (h->state & HANDLE_CONNECTED)); if (b->i) return backend_finalize (b->next); -- 2.24.1
On 2/11/20 7:24 PM, Eric Blake wrote:
If .prepare fails, we do not want to call .finalize (similar to how if .open fails, we do not want to call .close). However, the logic in backend_finalize was checking on the wrong condition ('was the connection opened' rather than 'was the connection prepared'), which led to an assertion failure.
Simple reproducer:
$ nbdkit -U - -f --filter cache --run 'qemu-io -r -c quit $nbd' sh - <<\EOF
case $1 in get_size) echo oops >&2; exit 1 ;; *) exit 2 ;; esac EOF nbdkit: sh[1]: error: /tmp/nbdkitshYvAQbz/inline-script.sh: oops nbdkit: backend.c:206: backend_finalize: Assertion `h->state & HANDLE_CONNECTED' failed. qemu-io: can't open device nbd:unix:/tmp/nbdkit60FUTw/socket: Failed to read option reply: Unexpected end-of-file before all bytes were read nbdkit: nbdkit command was killed by signal 6
With this patch, the command now fails gracefully: nbdkit: sh[1]: error: /tmp/nbdkitshie18Lp/inline-script.sh: oops qemu-io: can't open device nbd:unix:/tmp/nbdkitqvAOlr/socket: Requested export not available
Fixes: ffa98c8d Signed-off-by: Eric Blake <eblake@redhat.com> ---
Pushing this to master and stable-1.16
I need to quit pushing things before letting 'make check' finish; this change now makes test-reopen-retry-fail.sh hang with a different assertion:
server/backend.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/server/backend.c b/server/backend.c index 8bfa8525..753f5cca 100644 --- a/server/backend.c +++ b/server/backend.c @@ -221,15 +221,13 @@ backend_finalize (struct backend *b) if (h->state & HANDLE_FAILED) return -1;
- if (h->handle) { - assert (h->state & HANDLE_CONNECTED); + assert (h->state & HANDLE_OPEN);
this code is not always true. I'm pushing the fixup as well.
+ if (h->state & HANDLE_CONNECTED) { if (b->finalize (b, h->handle) == -1) { h->state |= HANDLE_FAILED; return -1; } } - else - assert (! (h->state & HANDLE_CONNECTED));
if (b->i) return backend_finalize (b->next);
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On Tue, Feb 11, 2020 at 07:24:43PM -0600, Eric Blake wrote:
If .prepare fails, we do not want to call .finalize (similar to how if .open fails, we do not want to call .close). However, the logic in backend_finalize was checking on the wrong condition ('was the connection opened' rather than 'was the connection prepared'), which led to an assertion failure.
Simple reproducer:
$ nbdkit -U - -f --filter cache --run 'qemu-io -r -c quit $nbd' sh - <<\EOF
case $1 in get_size) echo oops >&2; exit 1 ;; *) exit 2 ;; esac EOF nbdkit: sh[1]: error: /tmp/nbdkitshYvAQbz/inline-script.sh: oops nbdkit: backend.c:206: backend_finalize: Assertion `h->state & HANDLE_CONNECTED' failed. qemu-io: can't open device nbd:unix:/tmp/nbdkit60FUTw/socket: Failed to read option reply: Unexpected end-of-file before all bytes were read nbdkit: nbdkit command was killed by signal 6
With this patch, the command now fails gracefully: nbdkit: sh[1]: error: /tmp/nbdkitshie18Lp/inline-script.sh: oops qemu-io: can't open device nbd:unix:/tmp/nbdkitqvAOlr/socket: Requested export not available
Fixes: ffa98c8d Signed-off-by: Eric Blake <eblake@redhat.com> ---
Pushing this to master and stable-1.16
server/backend.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/server/backend.c b/server/backend.c index 8bfa8525..753f5cca 100644 --- a/server/backend.c +++ b/server/backend.c @@ -221,15 +221,13 @@ backend_finalize (struct backend *b) if (h->state & HANDLE_FAILED) return -1;
- if (h->handle) { - assert (h->state & HANDLE_CONNECTED); + assert (h->state & HANDLE_OPEN); + if (h->state & HANDLE_CONNECTED) { if (b->finalize (b, h->handle) == -1) { h->state |= HANDLE_FAILED; return -1; } } - else - assert (! (h->state & HANDLE_CONNECTED));
if (b->i) return backend_finalize (b->next);
ACK 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
participants (2)
-
Eric Blake -
Richard W.M. Jones