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

[Eric Blake <eblake@redhat.com>]

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 <vsementsov@virtuozzo.com>
>> ---
>>

>> +/* 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