Commit 72b6ffc76653214b69a94a7b1643ff80df134486 ("nbd-client: Fix
regression when server sends garbage") improved NBD client behavior when
the connection enters a broken state.
The following still does not behave as expected:
$ qemu-nbd -p 1234 -x drive0 -f qcow2 test.qcow2
$ qemu-system-x86_64 -M accel=kvm -m 1G \
-drive if=virtio,id=drive0,file=nbd://localhost:1234/drive0,format=raw
$ pkill qemu-nbd
(qemu) quit
...hang...
QEMU should be able to quit even when the connection was previously
closed by the NBD server. Currently the nbd_read_reply_entry()
coroutine terminates without letting in-flight requests know that the
connection is dead.
This patch flags the connection as dead so in-flight requests can
complete.
Reported-by: Longxiang Lyu <lolyu@redhat.com>
Cc: Eric Blake <eblake@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/nbd-client.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 422ecb4307..5a5fe02015 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -80,6 +80,7 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
error_report_err(local_err);
}
if (ret <= 0) {
+ s->quit = true;
break;
}
@@ -107,9 +108,6 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
qemu_coroutine_yield();
}
- if (ret < 0) {
- s->quit = true;
- }
nbd_recv_coroutines_enter_all(s);
s->read_reply_co = NULL;
}
--
2.13.5
On Mon, Aug 21, 2017 at 5:15 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote: > (qemu) quit > ...hang... By the way, the same issue is present in QEMU 2.9. This is not a 2.10 regression. Stefan
On 08/21/2017 11:27 AM, Stefan Hajnoczi wrote: > On Mon, Aug 21, 2017 at 5:15 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote: >> (qemu) quit >> ...hang... > > By the way, the same issue is present in QEMU 2.9. This is not a 2.10 > regression. Most likely, though, it IS a regression introduced in 2.9 and not present in 2.8. I'll add this to my list of 2.11 NBD patches. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On 08/21/2017 02:01 PM, Eric Blake wrote: > On 08/21/2017 11:27 AM, Stefan Hajnoczi wrote: >> On Mon, Aug 21, 2017 at 5:15 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote: >>> (qemu) quit >>> ...hang... >> >> By the way, the same issue is present in QEMU 2.9. This is not a 2.10 >> regression. > > Most likely, though, it IS a regression introduced in 2.9 and not > present in 2.8. I'll add this to my list of 2.11 NBD patches. Since it looks like we're doing -rc4 and including other NBD patches for that, I'm adding this to my 2.10 NBD queue, and will send a pull request shortly... -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On 08/23/2017 09:25 AM, Eric Blake wrote: > On 08/21/2017 02:01 PM, Eric Blake wrote: >> On 08/21/2017 11:27 AM, Stefan Hajnoczi wrote: >>> On Mon, Aug 21, 2017 at 5:15 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote: >>>> (qemu) quit >>>> ...hang... >>> >>> By the way, the same issue is present in QEMU 2.9. This is not a 2.10 >>> regression. >> >> Most likely, though, it IS a regression introduced in 2.9 and not >> present in 2.8. I'll add this to my list of 2.11 NBD patches. > > Since it looks like we're doing -rc4 and including other NBD patches for > that, I'm adding this to my 2.10 NBD queue, On IRC, Stefan informed me that 'nbd-client: avoid spurious qio_channel_yield() re-entry' obsoletes this one. > and will send a pull request > shortly... This part is still true -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On 08/21/2017 01:15 PM, Stefan Hajnoczi wrote: > Commit 72b6ffc76653214b69a94a7b1643ff80df134486 ("nbd-client: Fix > regression when server sends garbage") improved NBD client behavior when > the connection enters a broken state. > > The following still does not behave as expected: > > $ qemu-nbd -p 1234 -x drive0 -f qcow2 test.qcow2 > $ qemu-system-x86_64 -M accel=kvm -m 1G \ > -drive if=virtio,id=drive0,file=nbd://localhost:1234/drive0,format=raw > $ pkill qemu-nbd > (qemu) quit > ...hang... > > QEMU should be able to quit even when the connection was previously > closed by the NBD server. Currently the nbd_read_reply_entry() > coroutine terminates without letting in-flight requests know that the > connection is dead. > > This patch flags the connection as dead so in-flight requests can > complete. > > Reported-by: Longxiang Lyu <lolyu@redhat.com> > Cc: Eric Blake <eblake@redhat.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Acked-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > block/nbd-client.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/block/nbd-client.c b/block/nbd-client.c > index 422ecb4307..5a5fe02015 100644 > --- a/block/nbd-client.c > +++ b/block/nbd-client.c > @@ -80,6 +80,7 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque) > error_report_err(local_err); > } > if (ret <= 0) { > + s->quit = true; > break; > } > > @@ -107,9 +108,6 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque) > qemu_coroutine_yield(); > } > > - if (ret < 0) { > - s->quit = true; > - } > nbd_recv_coroutines_enter_all(s); > s->read_reply_co = NULL; > } >
On Mon, Aug 21, 2017 at 05:15:39PM +0100, Stefan Hajnoczi wrote: > Commit 72b6ffc76653214b69a94a7b1643ff80df134486 ("nbd-client: Fix > regression when server sends garbage") improved NBD client behavior when > the connection enters a broken state. > > The following still does not behave as expected: > > $ qemu-nbd -p 1234 -x drive0 -f qcow2 test.qcow2 > $ qemu-system-x86_64 -M accel=kvm -m 1G \ > -drive if=virtio,id=drive0,file=nbd://localhost:1234/drive0,format=raw > $ pkill qemu-nbd > (qemu) quit > ...hang... > > QEMU should be able to quit even when the connection was previously > closed by the NBD server. Currently the nbd_read_reply_entry() > coroutine terminates without letting in-flight requests know that the > connection is dead. > > This patch flags the connection as dead so in-flight requests can > complete. > > Reported-by: Longxiang Lyu <lolyu@redhat.com> > Cc: Eric Blake <eblake@redhat.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > block/nbd-client.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) This patch is superceded by "[PATCH] nbd-client: avoid spurious qio_channel_yield() re-entry". Stefan
© 2016 - 2024 Red Hat, Inc.