On Mon, Jun 12, 2023 at 02:24:52PM -0500, Eric Blake wrote:
> On Mon, Jun 12, 2023 at 06:07:59PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> > On 08.06.23 16:56, Eric Blake wrote:
> > > The upcoming patches for 64-bit extensions requires various points in
> > > the protocol to make decisions based on what was negotiated. While we
> > > could easily add a 'bool extended_headers' alongside the existing
> > > 'bool structured_reply', this does not scale well if more modes are
> > > added in the future. Better is to expose the mode enum added in the
> > > previous patch out to a wider use in the code base.
> > >
> > > Where the code previously checked for structured_reply being set or
> > > clear, it now prefers checking for an inequality; this works because
> > > the nodes are in a continuum of increasing abilities, and allows us to
> > > touch fewer places if we ever insert other modes in the middle of the
> > > enum. There should be no semantic change in this patch.
> > >
> > > Signed-off-by: Eric Blake <ebl...@redhat.com>
> > > ---
> > >
> > > v4: new patch, expanding enum idea from v3 4/14
> > > ---
> >
> > [..]
> >
> > > diff --git a/nbd/server.c b/nbd/server.c
> > > index 8486b64b15d..bade4f7990c 100644
> > > --- a/nbd/server.c
> > > +++ b/nbd/server.c
>
> > > @@ -1261,13 +1262,13 @@ static int nbd_negotiate_options(NBDClient
> > > *client, Error **errp)
> > > case NBD_OPT_STRUCTURED_REPLY:
> > > if (length) {
> > > ret = nbd_reject_length(client, false, errp);
> > > - } else if (client->structured_reply) {
> > > + } else if (client->mode >= NBD_MODE_STRUCTURED) {
> > > ret = nbd_negotiate_send_rep_err(
> > > client, NBD_REP_ERR_INVALID, errp,
> > > "structured reply already negotiated");
> > > } else {
> > > ret = nbd_negotiate_send_rep(client, NBD_REP_ACK,
> > > errp);
> > > - client->structured_reply = true;
> > > + client->mode = NBD_MODE_STRUCTURED;
> >
> > Hmm. in all other cases in server code client.mode remains zero = OLDSTYLE,
> > which is not quite correct.
>
> Good catch. Consider this squashed in (note that as a server we NEVER
> talk NBD_MODE_OLDSTYLE - we ripped that out back in commit 7f7dfe2a;
> but whether we end up on EXPORT_NAME or SIMPLE depends on the client's
> response to our initial flag advertisement. The only reason I didn't
> spot it sooner is that in the server, all subsequent checks of
> client->mode grouped OLDSTYLE, EXPORT_NAME, and SIMPLE into the same
> handling.
To move things along, I have now staged 1-8 in my NBD queue for a pull
request, and will then repost this patch and the remainder of the
series as v5, to make it easier to pick up the final needed R-b.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
