On 1/16/19 4:15 AM, Vladimir Sementsov-Ogievskiy wrote: >> @@ -347,7 +350,8 @@ static int nbd_opt_go(QIOChannel *ioc, NBDExportInfo >> *info, Error **errp) >> * flags still 0 is a witness of a broken server. */ >> info->flags = 0; >> >> - 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. >
Can do.
>> + /* Send NBD_OPT_ABORT as a courtesy before hanging up */
>> + nbd_send_opt_abort(ioc);
>> + break;
>> + case 1: /* newstyle, but limited to EXPORT_NAME */
>> + error_setg(errp, "Server does not support export lists");
>> + /* We can't even send NBD_OPT_ABORT, so merely hang up */
>
> But, on the other hand, why not to send it? It will be unknown for the server,
> so, it will have to close the connection accordingly to the protocol, isn't it
> better a bit?
The NBD spec says that if the server did not advertise fixed newstyle,
then the client must not send any other NBD_OPT. And servers that don't
support fixed newstyle are rather rare, so it doesn't really hurt if we
aren't courteous to them.
>
>> + goto out;
>> + case 0: /* oldstyle, parse length and flags */
>> + array = g_new0(NBDExportInfo, 1);
>> + array->name = g_strdup("");
>> + count = 1;
>> +
>> + if (nbd_negotiate_finish_oldstyle(ioc, array, errp) < 0) {
>> + return -EINVAL;
>
> goto out, you mean.
Indeed. Thanks for spotting it.
> And with at least this one fixed:
> Reviewed-by: Vladimir Sementsov-Ogievskiy <[email protected]>
>
>
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
