On 1/16/19 4:15 AM, Vladimir Sementsov-Ogievskiy wrote:

>> This patch adds the low-level client code for grabbing the list
>> of exports.  It benefits from the recent refactoring patches, as
>> well as a minor tweak of changing nbd_opt_go() to nbd_opt_info_or_go(),

>> -/* Returns -1 if NBD_OPT_GO proves the export @info->name cannot be
>> +/*
>> + * Returns -1 if NBD_OPT_GO proves the export @info->name cannot be
>>    * used, 0 if NBD_OPT_GO is unsupported (fall back to NBD_OPT_LIST and
>>    * NBD_OPT_EXPORT_NAME in that case), and > 0 if the export is good to
>> - * go (with the rest of @info populated). */
>> -static int nbd_opt_go(QIOChannel *ioc, NBDExportInfo *info, Error **errp)
>> + * go (with the rest of @info populated).
>> + */
> 
> Don't you want to upgrade a comment a little bit, to reflect support for OPT_INFO?

Yeah, but then the tweaks start to build up. I'll split the function
rename to a separate patch,

>>
>> -    trace_nbd_opt_go_start(info->name);
>> +    assert(opt == NBD_OPT_GO || opt == NBD_OPT_INFO);
>> +    trace_nbd_opt_go_start(nbd_opt_lookup(opt), info->name);
> 
> I'd prefer to upgrade trace-point name too, as well as other several trace_nbd_opt_go_* trace
> points in the function.

and improve the traces there too.

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