Re: [PATCH 1/2] nbd: Add an unstable query for the client state

[Eric Blake <eblake@redhat.com>]

On Tue, Nov 10, 2026 at 04:07:35PM +0100, guillaume vincke wrote:
> On 11/10/26 13:53, Markus Armbruster wrote:
> 
> Thanks for the review. Let me clarify before posting a v2.
> 
> > 
> > Why block-core.json and not block-export.json?
> 
> If I get this right, block-export.json covers the server part. I
> thought adding this next to BlockdevOptionsNbd was a better fit. I can
> change that in v2 if you think block-export.json is a better match.

I'll trust Markus on this part of the review.

> > > +#
> > > +# @quit: Client will not reconnect.
> > 
> > What does this one mean exactly?
> 
> The client stopped for good. Requests fail and it never reconnects.
> This state is reached on fatal error or teardown.

So either the server gave up on the client (a protocol error such as
an incorrect magic number that means the server has lost sync and
prefers not to further entertain further input from the client), or
the client gracefully terminated (the NBD_OPT_ABORT during handshake
or NBD_CMD_DISC during normal operation for the client to tell the
server it is done). I don't know if we can tell those apart, nor if we
need to.  For debugging purposes, merely knowing that the connection
is done (even if the fd is still in the process of being cleaned up)
is already good enough for most purposes.

> 
> > 
> > > +#
> > > +# Since: 11.2
> > 
> > Making 11.2 seems unlikely.
> 
> Sure. Would updating this to 12.0 make more sense ?

Agreed - this is too much on the "new feature" side to make soft
freeze; even if it only exists to make debugging easier and the iotest
that is now in 11.2 a bit more robust, I think aiming the new QMP
command for 12.0 is better.

> > > +#
> > > +# @state: The client state.
> > > +#
> > > +# @connector: The connector details.  Only present while a connection
> > > +#     attempt is running or its channel is parked.
> > > +#
> > > +# Since: 11.2
> > > +##
> > > +{ 'struct': 'XDbgNbdClient',
> > > +  'data': { 'state': 'XDbgNbdClientState',
> > > +            '*connector': 'XDbgNbdClientConnector' } }
> > 
> > How the combined state machine works is not clear to me.  Is it
> > documented anywhere?
> 
> I did not find a proper documentation. I found the comments on the code
> to be quite explicit though. The connector only runs in a connecting-*
> state. In that state, when processing a request, the client tries to
> get the channel from the connector. When the connector has a valid
> channel but the client did not request it, the connector is in parked
> state. Do you think this command should document that behavior?
> 
> > 
> > > +
> > > +##
> > > +# @x-debug-query-nbd-client:
> > > +#     Get the NBD client state.
> > > +#
> > > +# @node-name: node name of the NBD client to query.
> > 
> > How would I find the valid node names?  query-block-exports?
> 
> The node-name is listed by query-named-block-nodes or is the one passed
> to blockdev-add. I will add a reference to those commands in the
> docstring to clarify.

The export name need not match the node name (although it is common
for management apps to set the export name to copy the node name).

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