Re: [Qemu-devel] [PATCH v2 3/8] nbd: BLOCK_STATUS for standard get_block_status function: server part

[Eric Blake]

On 03/13/2018 08:47 AM, Eric Blake wrote:
> On 03/12/2018 10:21 AM, Vladimir Sementsov-Ogievskiy wrote:
> > Minimal realization: only one extent in server answer is supported.
> >
> > Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> > ---

> > +/* nbd_negotiate_meta_queries
> > + * Handle NBD_OPT_LIST_META_CONTEXT and NBD_OPT_SET_META_CONTEXT
> > + *
> > + * @meta may be NULL, if caller isn't interested in selected contexts 
> > (for
> > + *     NBD_OPT_LIST_META_CONTEXT)
> > + *
> > + * Return -errno on I/O error, 0 if option was completely handled by
> > + * sending a reply about inconsistent lengths, or 1 on success. */
>
> Comment is wrong - this function never returns 1 (nor should it, as 
> nbd_negotiate_options() expects a return of 1 only from NBD_OPT_ABORT).
>
> > +static int nbd_negotiate_meta_queries(NBDClient *client,
> > +                                      NBDExportMetaContexts *meta, 
> > Error **errp)
> > +{
> > +    int ret;
> > +    NBDExport *exp;
> > +    NBDExportMetaContexts local_meta;
> > +    uint32_t nb_queries;
> > +    int i;
> > +
> > +    assert(client->structured_reply);
>
> Perhaps worth a comment that this is safe because we already filtered it 
> out at the caller.
>
> > +
> > +    if (!meta) {
> > +        meta = &local_meta;
> > +    }

Or, we could check here, and even base our decision on whether to change 
'meta' due to client->opt...


> > @@ -856,6 +1064,22 @@ static int nbd_negotiate_options(NBDClient 
> > *client, uint16_t myflags,
> >                   }
> >                   break;
> > +            case NBD_OPT_LIST_META_CONTEXT:
> > +            case NBD_OPT_SET_META_CONTEXT:
> > +                if (!client->structured_reply) {
> > +                    ret = nbd_opt_invalid(
> > +                            client, errp,
> > +                            "request option '%s' when structured reply "
> > +                            "is not negotiated", 
> > nbd_opt_lookup(option));
> > +                } else if (option == NBD_OPT_LIST_META_CONTEXT) {
> > +                    ret = nbd_negotiate_meta_queries(client, NULL, 
> > errp);
> > +                } else {
> > +                    ret = nbd_negotiate_meta_queries(client,
> > +                                                     
> > &client->export_meta,
> > +                                                     errp);
> > +                }

Then here, we just do a single ret = nbd_negotiate_meta_queries().

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org