[PATCH v3 0/2] nbd/client-connection: Fix client returning a closed channel

Guillaume Vincke posted 2 patches 1 week ago
Maintainers: Eric Blake <eblake@redhat.com>, Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
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
[PATCH v3 0/2] nbd/client-connection: Fix client returning a closed channel
Posted by Guillaume Vincke 1 week ago
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/
v2: https://patchew.org/QEMU/20261016084136.233836-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.

v2 -> v3:
- patch 2: Restrict the test to qcow2. The luks CI failure was the
missing key-secret and the parallels failure was the dirty-image repair
message. Neither was timing.

v3 does not use a more robust method than sleeping to wait for the NBD
client to reach the desired state. An option would be to add a QMP
unstable debug query reporting the NBD client state and whether the
connect thread has a channel ready. The test would poll this instead of
sleeping. Would that be an acceptable follow-up?

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
Re: [PATCH v3 0/2] nbd/client-connection: Fix client returning a closed channel
Posted by Eric Blake 6 days, 5 hours ago
On Thu, Nov 05, 2026 at 05:41:22PM +0100, Guillaume Vincke wrote:
> 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.
> 

> 
> v3 does not use a more robust method than sleeping to wait for the NBD
> client to reach the desired state. An option would be to add a QMP
> unstable debug query reporting the NBD client state and whether the
> connect thread has a channel ready. The test would poll this instead of
> sleeping. Would that be an acceptable follow-up?

I think an unstable query for current NBD server state (is the server
up, and if so, which exports is it serving and what is the state of
all client(s) attached to each server) would indeed make sense.  That
will probably miss this release, but I still think this series is okay
for now.  Queueing it up as a bug fix, for my upcoming pull request
intended to make it into rc0.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org
Re: [PATCH v3 0/2] nbd/client-connection: Fix client returning a closed channel
Posted by guillaume vincke 3 days, 8 hours ago
On 11/6/26 20:45, Eric Blake wrote:
> On Thu, Nov 05, 2026 at 05:41:22PM +0100, Guillaume Vincke wrote:
>> 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.
>>
> 
>>
>> v3 does not use a more robust method than sleeping to wait for the NBD
>> client to reach the desired state. An option would be to add a QMP
>> unstable debug query reporting the NBD client state and whether the
>> connect thread has a channel ready. The test would poll this instead of
>> sleeping. Would that be an acceptable follow-up?
> 
> I think an unstable query for current NBD server state (is the server
> up, and if so, which exports is it serving and what is the state of
> all client(s) attached to each server) would indeed make sense.  That
> will probably miss this release, but I still think this series is okay
> for now.  Queueing it up as a bug fix, for my upcoming pull request
> intended to make it into rc0.
> 

Thanks for the review and for queueing it. I sent a new patchset with
a QMP command at:
https://patchew.org/QEMU/20261109160609.1434289-1-guillaume.vincke@ovhcloud.com/
It does not access the server's internals but the client's, since the
test uses qemu-nbd that does not have QMP. Let me know if that still
fits your expectations.