All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] nbd/client-connection: Fix client returning a closed channel
@ 2026-10-16  8:41 Guillaume Vincke
  2026-10-16  8:41 ` [PATCH v2 1/2] " Guillaume Vincke
  2026-10-16  8:41 ` [PATCH v2 2/2] iotests: Add coverage for NBD connect thread start delay Guillaume Vincke
  0 siblings, 2 replies; 6+ messages in thread
From: Guillaume Vincke @ 2026-10-16  8:41 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-block, Eric Blake, Vladimir Sementsov-Ogievskiy, Kevin Wolf,
	Hanna Reitz, Guillaume Vincke

NBD client relies on lazy reconnection on error. There is a case where
nbd_co_establish_connection() will hand back a channel disconnected by
its peer. This leads to the request running on the dead channel. When
reconnect-delay is set to 0 (the default), this bug prevents the
nbd-connect thread from running on the first operation. This patch set
addresses this specific case.

Found while searching for a way to force client reconnection to simplify
the resize of idle nodes which will be the subject of another patch set.

v1: https://patchew.org/QEMU/20260925155346.357703-1-guillaume.vincke@ovhcloud.com/

v1 -> v2:
- patch 1: Pick up Eric Blake's formatting changes.
- patch 2: Start the connect thread after the NBD server so the test no
longer depends on the 1s connect retry, which made v1 fail in CI. Raise
the connect wait to 1s.

Guillaume Vincke (2):
  nbd/client-connection: Fix client returning a closed channel
  iotests: Add coverage for NBD connect thread start delay

 nbd/client-connection.c                       | 32 ++++++++++
 tests/qemu-iotests/tests/nbd-stale-connection | 60 +++++++++++++++++++
 .../tests/nbd-stale-connection.out            | 17 ++++++
 3 files changed, 109 insertions(+)
 create mode 100755 tests/qemu-iotests/tests/nbd-stale-connection
 create mode 100644 tests/qemu-iotests/tests/nbd-stale-connection.out

-- 
2.43.0



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/2] nbd/client-connection: Fix client returning a closed channel
  2026-10-16  8:41 [PATCH v2 0/2] nbd/client-connection: Fix client returning a closed channel Guillaume Vincke
@ 2026-10-16  8:41 ` Guillaume Vincke
  2026-10-16  8:41 ` [PATCH v2 2/2] iotests: Add coverage for NBD connect thread start delay Guillaume Vincke
  1 sibling, 0 replies; 6+ messages in thread
From: Guillaume Vincke @ 2026-10-16  8:41 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-block, Eric Blake, Vladimir Sementsov-Ogievskiy, Kevin Wolf,
	Hanna Reitz, Guillaume Vincke

When a server closes the connection after a successful handshake but
before running any operations, the channel returned by
nbd_co_establish_connection() is the closed one and the connect thread
is not started.

When configured with reconnect-delay=0 (the default), this situation
generates at least two IO errors. The first one happens when trying to
issue an operation to the closed socket. The connect thread will be
started on the next request, which may happen long after, and will
create the second error as nbd_co_establish_connection() returns a NULL
channel.

Peeking the channel to detect FIN or RST avoids the first IO error and
does not delay the starting of the connect thread.

Signed-off-by: Guillaume Vincke <guillaume.vincke@ovhcloud.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 nbd/client-connection.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/nbd/client-connection.c b/nbd/client-connection.c
index 6a4f080717..aec62934d1 100644
--- a/nbd/client-connection.c
+++ b/nbd/client-connection.c
@@ -274,6 +274,23 @@ void nbd_client_connection_release(NBDClientConnection *conn)
     }
 }
 
+/* Only true when the peer closed the channel. Does not detect if dead. */
+static bool nbd_channel_is_closed(QIOChannel *ioc)
+{
+    char data;
+    struct iovec iov = { .iov_base = &data, .iov_len = 1 };
+    ssize_t len = -1;
+
+    if (!qio_channel_has_feature(ioc, QIO_CHANNEL_FEATURE_READ_MSG_PEEK)) {
+        return false;
+    }
+
+    len = qio_channel_readv_full(
+        ioc, &iov, 1, NULL, NULL, QIO_CHANNEL_READ_FLAG_MSG_PEEK, NULL);
+
+    return len == 0 || (len < 0 && len != QIO_CHANNEL_ERR_BLOCK);
+}
+
 /*
  * Get a new connection in context of @conn:
  *   if the thread is running, wait for completion
@@ -295,6 +312,7 @@ nbd_co_establish_connection(NBDClientConnection *conn, NBDExportInfo *info,
                             bool blocking, Error **errp)
 {
     QemuThread thread;
+    QIOChannel *ioc;
 
     if (conn->do_negotiation) {
         assert(info);
@@ -308,6 +326,20 @@ nbd_co_establish_connection(NBDClientConnection *conn, NBDExportInfo *info,
         assert(!conn->wait_co);
 
         if (!conn->running) {
+            if (conn->sioc) {
+                /* discard if the socket got closed by its peer */
+                ioc = QIO_CHANNEL(conn->sioc);
+                if (!qio_channel_set_blocking(ioc, false, NULL) ||
+                    nbd_channel_is_closed(ioc)) {
+
+                    qio_channel_close(ioc, NULL);
+                    object_unref(OBJECT(conn->sioc));
+                    conn->sioc = NULL;
+                    object_unref(OBJECT(conn->ioc));
+                    conn->ioc = NULL;
+                }
+            }
+
             if (conn->sioc) {
                 /* Previous attempt finally succeeded in background */
                 if (conn->do_negotiation) {
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/2] iotests: Add coverage for NBD connect thread start delay
  2026-10-16  8:41 [PATCH v2 0/2] nbd/client-connection: Fix client returning a closed channel Guillaume Vincke
  2026-10-16  8:41 ` [PATCH v2 1/2] " Guillaume Vincke
@ 2026-10-16  8:41 ` Guillaume Vincke
  2026-10-16 14:13   ` Eric Blake
  1 sibling, 1 reply; 6+ messages in thread
From: Guillaume Vincke @ 2026-10-16  8:41 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-block, Eric Blake, Vladimir Sementsov-Ogievskiy, Kevin Wolf,
	Hanna Reitz, Guillaume Vincke

Test the connect thread starting condition with reconnect-delay=0 when
the server disconnects after a successful handshake but before running
operations. Prior to the change to nbd_co_establish_connection(), this
test would demonstrate two I/O errors. The tests 264 and 277 covering
reconnection do not use reconnect-delay=0 so this case is not exercised.

Add a test that provokes multiple errors in order to put an NBD client
in the above-mentioned situation and shows that the first operation
starts the connect thread and leads to successful subsequent I/O.

Signed-off-by: Guillaume Vincke <guillaume.vincke@ovhcloud.com>
---
 tests/qemu-iotests/tests/nbd-stale-connection | 60 +++++++++++++++++++
 .../tests/nbd-stale-connection.out            | 17 ++++++
 2 files changed, 77 insertions(+)
 create mode 100755 tests/qemu-iotests/tests/nbd-stale-connection
 create mode 100644 tests/qemu-iotests/tests/nbd-stale-connection.out

diff --git a/tests/qemu-iotests/tests/nbd-stale-connection b/tests/qemu-iotests/tests/nbd-stale-connection
new file mode 100755
index 0000000000..09e00d8b3e
--- /dev/null
+++ b/tests/qemu-iotests/tests/nbd-stale-connection
@@ -0,0 +1,60 @@
+#!/usr/bin/env python3
+# group: quick
+#
+# Test the NBD client stale channel discard
+#
+# Copyright (C) 2026 OVHcloud
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+import time
+import iotests
+from iotests import qemu_nbd_popen, qemu_img_create, file_path, log
+
+
+# estimated duration required by the connect thread to reach a running server
+MAX_NBD_CONNECT_DURATION = 1
+
+iotests.script_initialize()
+nbd_sock = file_path('nbd-sock', base_dir=iotests.sock_dir)
+disk = file_path('disk')
+qemu_img_create('-f', iotests.imgfmt, disk, '1M')
+
+with iotests.VM() as vm:
+
+    vm.launch()
+
+    with qemu_nbd_popen('-k', nbd_sock, '-f', iotests.imgfmt, disk):
+        vm.cmd('blockdev-add', {
+            'driver': 'nbd',
+            'node-name': 'nbd0',
+            'server': {'type': 'unix', 'path': nbd_sock},
+        })
+        iotests.log('=== NBD Device connected ===')
+        log(vm.qmp('x-qemu-io', device='nbd0', command='write -P 0x5a 0 64k'))
+
+    iotests.log('=== NBD server down. Detect the break and start the connect '
+                'thread ===')
+    # client was in CONNECTED state a new operation updates to CONNECTING
+    log(vm.qmp('x-qemu-io', device='nbd0', command='write -P 0x5a 0 64k'))
+
+    with qemu_nbd_popen('-k', nbd_sock, '-f', iotests.imgfmt, disk):
+        iotests.log('=== NBD server up. Wait for the connect thread to park '
+                    'the channel ===')
+        # in CONNECTING, the operation starts the connect thread
+        log(vm.qmp('x-qemu-io', device='nbd0', command='write -P 0x5a 0 64k'))
+        # wait for the connect thread to get a new channel and stop
+        time.sleep(MAX_NBD_CONNECT_DURATION)
+
+    iotests.log('=== NBD server down. The parked channel is closed ===')
+
+    with qemu_nbd_popen('-k', nbd_sock, '-f', iotests.imgfmt, disk):
+        iotests.log('=== NBD server up. Restart the connect thread ===')
+        # operation will determine that the channel got closed and restart the
+        # connect thread
+        log(vm.qmp('x-qemu-io', device='nbd0', command='write -P 0x5a 0 64k'))
+        # Wait for it to succeed
+        iotests.log('=== Wait for the connect thread to park the new channel. '
+                    'Subsequent operations are successful ===')
+        time.sleep(MAX_NBD_CONNECT_DURATION)
+        log(vm.qmp('x-qemu-io', device='nbd0', command='write -P 0x5a 0 64k'))
diff --git a/tests/qemu-iotests/tests/nbd-stale-connection.out b/tests/qemu-iotests/tests/nbd-stale-connection.out
new file mode 100644
index 0000000000..065ba329e9
--- /dev/null
+++ b/tests/qemu-iotests/tests/nbd-stale-connection.out
@@ -0,0 +1,17 @@
+Start NBD server
+=== NBD Device connected ===
+{"return": {}}
+Kill NBD server
+=== NBD server down. Detect the break and start the connect thread ===
+{"error": {"class": "GenericError", "desc": "write failed: Input/output error"}}
+Start NBD server
+=== NBD server up. Wait for the connect thread to park the channel ===
+{"error": {"class": "GenericError", "desc": "write failed: Input/output error"}}
+Kill NBD server
+=== NBD server down. The parked channel is closed ===
+Start NBD server
+=== NBD server up. Restart the connect thread ===
+{"error": {"class": "GenericError", "desc": "write failed: Input/output error"}}
+=== Wait for the connect thread to park the new channel. Subsequent operations are successful ===
+{"return": {}}
+Kill NBD server
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] iotests: Add coverage for NBD connect thread start delay
  2026-10-16  8:41 ` [PATCH v2 2/2] iotests: Add coverage for NBD connect thread start delay Guillaume Vincke
@ 2026-10-16 14:13   ` Eric Blake
  2026-10-16 15:15     ` guillaume vincke
  2026-10-16 15:22     ` Eric Blake
  0 siblings, 2 replies; 6+ messages in thread
From: Eric Blake @ 2026-10-16 14:13 UTC (permalink / raw)
  To: Guillaume Vincke
  Cc: qemu-devel, qemu-block, Vladimir Sementsov-Ogievskiy, Kevin Wolf,
	Hanna Reitz

On Fri, Oct 16, 2026 at 10:41:36AM +0200, Guillaume Vincke wrote:
> Test the connect thread starting condition with reconnect-delay=0 when
> the server disconnects after a successful handshake but before running
> operations. Prior to the change to nbd_co_establish_connection(), this
> test would demonstrate two I/O errors. The tests 264 and 277 covering
> reconnection do not use reconnect-delay=0 so this case is not exercised.
> 
> Add a test that provokes multiple errors in order to put an NBD client
> in the above-mentioned situation and shows that the first operation
> starts the connect thread and leads to successful subsequent I/O.
> 
> Signed-off-by: Guillaume Vincke <guillaume.vincke@ovhcloud.com>
> ---
>  tests/qemu-iotests/tests/nbd-stale-connection | 60 +++++++++++++++++++
>  .../tests/nbd-stale-connection.out            | 17 ++++++
>  2 files changed, 77 insertions(+)
>  create mode 100755 tests/qemu-iotests/tests/nbd-stale-connection
>  create mode 100644 tests/qemu-iotests/tests/nbd-stale-connection.out

> +
> +# estimated duration required by the connect thread to reach a running server
> +MAX_NBD_CONNECT_DURATION = 1
> +

> +
> +    with qemu_nbd_popen('-k', nbd_sock, '-f', iotests.imgfmt, disk):
> +        iotests.log('=== NBD server up. Wait for the connect thread to park '
> +                    'the channel ===')
> +        # in CONNECTING, the operation starts the connect thread
> +        log(vm.qmp('x-qemu-io', device='nbd0', command='write -P 0x5a 0 64k'))
> +        # wait for the connect thread to get a new channel and stop
> +        time.sleep(MAX_NBD_CONNECT_DURATION)

Timeout-based tests are always risky for false negatives when the CI
system has more load than the development system.  But comparing this
version of the patch to v1, it looks like you managed to get much more
reliable witness points.  I'll be running this through testing on my
own branch of gitlab several times to see if I can tease out any more
failures, but hopefully it is ready to use. I'll reply again once I've
made a decision on whether to include it in my current pull request or
defer it to my next batch of patches.

> +
> +    iotests.log('=== NBD server down. The parked channel is closed ===')
> +
> +    with qemu_nbd_popen('-k', nbd_sock, '-f', iotests.imgfmt, disk):
> +        iotests.log('=== NBD server up. Restart the connect thread ===')
> +        # operation will determine that the channel got closed and restart the
> +        # connect thread
> +        log(vm.qmp('x-qemu-io', device='nbd0', command='write -P 0x5a 0 64k'))
> +        # Wait for it to succeed
> +        iotests.log('=== Wait for the connect thread to park the new channel. '
> +                    'Subsequent operations are successful ===')
> +        time.sleep(MAX_NBD_CONNECT_DURATION)
> +        log(vm.qmp('x-qemu-io', device='nbd0', command='write -P 0x5a 0 64k'))
> diff --git a/tests/qemu-iotests/tests/nbd-stale-connection.out b/tests/qemu-iotests/tests/nbd-stale-connection.out
> new file mode 100644
> index 0000000000..065ba329e9
> --- /dev/null
> +++ b/tests/qemu-iotests/tests/nbd-stale-connection.out
> @@ -0,0 +1,17 @@
> +Start NBD server
> +=== NBD Device connected ===
> +{"return": {}}
> +Kill NBD server
> +=== NBD server down. Detect the break and start the connect thread ===
> +{"error": {"class": "GenericError", "desc": "write failed: Input/output error"}}
> +Start NBD server
> +=== NBD server up. Wait for the connect thread to park the channel ===
> +{"error": {"class": "GenericError", "desc": "write failed: Input/output error"}}
> +Kill NBD server
> +=== NBD server down. The parked channel is closed ===
> +Start NBD server
> +=== NBD server up. Restart the connect thread ===
> +{"error": {"class": "GenericError", "desc": "write failed: Input/output error"}}
> +=== Wait for the connect thread to park the new channel. Subsequent operations are successful ===
> +{"return": {}}
> +Kill NBD server
> -- 
> 2.43.0
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] iotests: Add coverage for NBD connect thread start delay
  2026-10-16 14:13   ` Eric Blake
@ 2026-10-16 15:15     ` guillaume vincke
  2026-10-16 15:22     ` Eric Blake
  1 sibling, 0 replies; 6+ messages in thread
From: guillaume vincke @ 2026-10-16 15:15 UTC (permalink / raw)
  To: eblake
  Cc: qemu-devel, qemu-block, Vladimir Sementsov-Ogievskiy, Kevin Wolf,
	Hanna Reitz

On 10/16/26 16:13, Eric Blake wrote:
> On Fri, Oct 16, 2026 at 10:41:36AM +0200, Guillaume Vincke wrote:
>> Test the connect thread starting condition with reconnect-delay=0 when
>> the server disconnects after a successful handshake but before running
>> operations. Prior to the change to nbd_co_establish_connection(), this
>> test would demonstrate two I/O errors. The tests 264 and 277 covering
>> reconnection do not use reconnect-delay=0 so this case is not exercised.
>>
>> Add a test that provokes multiple errors in order to put an NBD client
>> in the above-mentioned situation and shows that the first operation
>> starts the connect thread and leads to successful subsequent I/O.
>>
>> Signed-off-by: Guillaume Vincke <guillaume.vincke@ovhcloud.com>
>> ---
>>   tests/qemu-iotests/tests/nbd-stale-connection | 60 +++++++++++++++++++
>>   .../tests/nbd-stale-connection.out            | 17 ++++++
>>   2 files changed, 77 insertions(+)
>>   create mode 100755 tests/qemu-iotests/tests/nbd-stale-connection
>>   create mode 100644 tests/qemu-iotests/tests/nbd-stale-connection.out
> 
>> +
>> +# estimated duration required by the connect thread to reach a running server
>> +MAX_NBD_CONNECT_DURATION = 1
>> +
> 
>> +
>> +    with qemu_nbd_popen('-k', nbd_sock, '-f', iotests.imgfmt, disk):
>> +        iotests.log('=== NBD server up. Wait for the connect thread to park '
>> +                    'the channel ===')
>> +        # in CONNECTING, the operation starts the connect thread
>> +        log(vm.qmp('x-qemu-io', device='nbd0', command='write -P 0x5a 0 64k'))
>> +        # wait for the connect thread to get a new channel and stop
>> +        time.sleep(MAX_NBD_CONNECT_DURATION)
> 
> Timeout-based tests are always risky for false negatives when the CI
> system has more load than the development system.  But comparing this
> version of the patch to v1, it looks like you managed to get much more
> reliable witness points.  I'll be running this through testing on my
> own branch of gitlab several times to see if I can tease out any more
> failures, but hopefully it is ready to use. I'll reply again once I've
> made a decision on whether to include it in my current pull request or
> defer it to my next batch of patches.
> 

Thanks. I agree that sleeping does not make the test very
deterministic. I have another series in preparation that relies on the
same kind of waiting, so I'd like to replace that.
I did not find a proper way to determine the NBD client connection
state. If you know one, let me know and I'd be happy to use it.

>> +
>> +    iotests.log('=== NBD server down. The parked channel is closed ===')
>> +
>> +    with qemu_nbd_popen('-k', nbd_sock, '-f', iotests.imgfmt, disk):
>> +        iotests.log('=== NBD server up. Restart the connect thread ===')
>> +        # operation will determine that the channel got closed and restart the
>> +        # connect thread
>> +        log(vm.qmp('x-qemu-io', device='nbd0', command='write -P 0x5a 0 64k'))
>> +        # Wait for it to succeed
>> +        iotests.log('=== Wait for the connect thread to park the new channel. '
>> +                    'Subsequent operations are successful ===')
>> +        time.sleep(MAX_NBD_CONNECT_DURATION)
>> +        log(vm.qmp('x-qemu-io', device='nbd0', command='write -P 0x5a 0 64k'))
>> diff --git a/tests/qemu-iotests/tests/nbd-stale-connection.out b/tests/qemu-iotests/tests/nbd-stale-connection.out
>> new file mode 100644
>> index 0000000000..065ba329e9
>> --- /dev/null
>> +++ b/tests/qemu-iotests/tests/nbd-stale-connection.out
>> @@ -0,0 +1,17 @@
>> +Start NBD server
>> +=== NBD Device connected ===
>> +{"return": {}}
>> +Kill NBD server
>> +=== NBD server down. Detect the break and start the connect thread ===
>> +{"error": {"class": "GenericError", "desc": "write failed: Input/output error"}}
>> +Start NBD server
>> +=== NBD server up. Wait for the connect thread to park the channel ===
>> +{"error": {"class": "GenericError", "desc": "write failed: Input/output error"}}
>> +Kill NBD server
>> +=== NBD server down. The parked channel is closed ===
>> +Start NBD server
>> +=== NBD server up. Restart the connect thread ===
>> +{"error": {"class": "GenericError", "desc": "write failed: Input/output error"}}
>> +=== Wait for the connect thread to park the new channel. Subsequent operations are successful ===
>> +{"return": {}}
>> +Kill NBD server
>> -- 
>> 2.43.0
>>
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] iotests: Add coverage for NBD connect thread start delay
  2026-10-16 14:13   ` Eric Blake
  2026-10-16 15:15     ` guillaume vincke
@ 2026-10-16 15:22     ` Eric Blake
  1 sibling, 0 replies; 6+ messages in thread
From: Eric Blake @ 2026-10-16 15:22 UTC (permalink / raw)
  To: Guillaume Vincke
  Cc: qemu-devel, qemu-block, Vladimir Sementsov-Ogievskiy, Kevin Wolf,
	Hanna Reitz

On Fri, Oct 16, 2026 at 09:13:57AM -0500, Eric Blake wrote:
> On Fri, Oct 16, 2026 at 10:41:36AM +0200, Guillaume Vincke wrote:
> > Test the connect thread starting condition with reconnect-delay=0 when
> > the server disconnects after a successful handshake but before running
> > operations. Prior to the change to nbd_co_establish_connection(), this
> > test would demonstrate two I/O errors. The tests 264 and 277 covering
> > reconnection do not use reconnect-delay=0 so this case is not exercised.
> > 
> > Add a test that provokes multiple errors in order to put an NBD client
> > in the above-mentioned situation and shows that the first operation
> > starts the connect thread and leads to successful subsequent I/O.
> > 
> > Signed-off-by: Guillaume Vincke <guillaume.vincke@ovhcloud.com>
> > ---
> >  tests/qemu-iotests/tests/nbd-stale-connection | 60 +++++++++++++++++++
> >  .../tests/nbd-stale-connection.out            | 17 ++++++
> >  2 files changed, 77 insertions(+)
> >  create mode 100755 tests/qemu-iotests/tests/nbd-stale-connection
> >  create mode 100644 tests/qemu-iotests/tests/nbd-stale-connection.out
> 
> > +
> > +# estimated duration required by the connect thread to reach a running server
> > +MAX_NBD_CONNECT_DURATION = 1
> > +
> 
> > +
> > +    with qemu_nbd_popen('-k', nbd_sock, '-f', iotests.imgfmt, disk):
> > +        iotests.log('=== NBD server up. Wait for the connect thread to park '
> > +                    'the channel ===')
> > +        # in CONNECTING, the operation starts the connect thread
> > +        log(vm.qmp('x-qemu-io', device='nbd0', command='write -P 0x5a 0 64k'))
> > +        # wait for the connect thread to get a new channel and stop
> > +        time.sleep(MAX_NBD_CONNECT_DURATION)
> 
> Timeout-based tests are always risky for false negatives when the CI
> system has more load than the development system.  But comparing this
> version of the patch to v1, it looks like you managed to get much more
> reliable witness points.  I'll be running this through testing on my
> own branch of gitlab several times to see if I can tease out any more
> failures, but hopefully it is ready to use. I'll reply again once I've
> made a decision on whether to include it in my current pull request or
> defer it to my next batch of patches.
> 
> > +
> > +    iotests.log('=== NBD server down. The parked channel is closed ===')
> > +
> > +    with qemu_nbd_popen('-k', nbd_sock, '-f', iotests.imgfmt, disk):
> > +        iotests.log('=== NBD server up. Restart the connect thread ===')
> > +        # operation will determine that the channel got closed and restart the
> > +        # connect thread
> > +        log(vm.qmp('x-qemu-io', device='nbd0', command='write -P 0x5a 0 64k'))
> > +        # Wait for it to succeed
> > +        iotests.log('=== Wait for the connect thread to park the new channel. '
> > +                    'Subsequent operations are successful ===')
> > +        time.sleep(MAX_NBD_CONNECT_DURATION)
> > +        log(vm.qmp('x-qemu-io', device='nbd0', command='write -P 0x5a 0 64k'))

It fared better for qcow2, but luks failed:

https://gitlab.com/ebblake/qemu/-/jobs/17236935977

> ▶ 51/52 luks nbd-stale-connection                                            FAIL          
> 51/52 block-luks-optional+block-thorough - qemu:io-luks-nbd-stale-connection FAIL             3.90s   exit status 1
> >>> RUST_BACKTRACE=1 PYTHON=/builds/ebblake/qemu/build/pyvenv/bin/python3.12 ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 LD_LIBRARY_PATH=/builds/ebblake/qemu/build/subprojects/libvfio-user/lib MALLOC_PERTURB_=100 MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 MESON_TEST_ITERATION=1 /builds/ebblake/qemu/build/pyvenv/bin/python3.12 /builds/ebblake/qemu/build/../tests/qemu-iotests/check -tap -luks nbd-stale-connection --source-dir /builds/ebblake/qemu/tests/qemu-iotests --build-dir /builds/ebblake/qemu/build/tests/qemu-iotests
> ――――――――――――――――――――――――――――――――――――― ✀  ―――――――――――――――――――――――――――――――――――――
> stderr:
> WARNING: iotests is being run from outside of the configure-time python virtual environment
> current python: /builds/ebblake/qemu/build/pyvenv/bin/python3.12
> pyvenv python:  /builds/ebblake/qemu/build/pyvenv/bin/python3
> Individual python tests will be executed inside the pyvenv,
> but the test runner will continue to run outside.
> Please use the meson run script:
> 	/builds/ebblake/qemu/build/run /builds/ebblake/qemu/build/../tests/qemu-iotests/check -tap -luks nbd-stale-connection --source-dir /builds/ebblake/qemu/tests/qemu-iotests --build-dir /builds/ebblake/qemu/build/tests/qemu-iotests
> --- /builds/ebblake/qemu/tests/qemu-iotests/tests/nbd-stale-connection.out
> +++ /builds/ebblake/qemu/build/scratch/luks-file-nbd-stale-connection/nbd-stale-connection.out.bad
> @@ -1,17 +1,13 @@
>  Start NBD server
> -=== NBD Device connected ===
> -{"return": {}}
> +qemu-nbd: Failed to blk_new_open '/builds/ebblake/qemu/build/scratch/luks-file-nbd-stale-connection/5998-disk': Parameter 'key-secret' is required for cipher
>  Kill NBD server

and for Parallels:

> https://gitlab.com/ebblake/qemu/-/jobs/17236935979
> 
> --- /builds/ebblake/qemu/tests/qemu-iotests/tests/nbd-stale-connection.out
> +++ /builds/ebblake/qemu/build/scratch/parallels-file-nbd-stale-connection/nbd-stale-connection.out.bad
> @@ -5,11 +5,14 @@
>  === NBD server down. Detect the break and start the connect thread ===
>  {"error": {"class": "GenericError", "desc": "write failed: Input/output error"}}
>  Start NBD server
> +Repairing image was not closed correctly
> +Repairing space leaked at the end of the image 135266304
>  === NBD server up. Wait for the connect thread to park the channel ===
> ...

But those failures are distinct from race conditions causing
unexpected socket status; and may be easier to fix them by just
skipping the test for those formats.

At any rate, it will be easier to exclude this series from my current
pull request, and just pick it up in the next round, once we decide
whether skipping luks and parallels is the best course of action, or
try to make the test even more robust.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-10-16 15:23 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-16  8:41 [PATCH v2 0/2] nbd/client-connection: Fix client returning a closed channel Guillaume Vincke
2026-10-16  8:41 ` [PATCH v2 1/2] " Guillaume Vincke
2026-10-16  8:41 ` [PATCH v2 2/2] iotests: Add coverage for NBD connect thread start delay Guillaume Vincke
2026-10-16 14:13   ` Eric Blake
2026-10-16 15:15     ` guillaume vincke
2026-10-16 15:22     ` Eric Blake

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.