Re: [Libguestfs] [PATCH nbdkit v2] protocol: Implement NBD_OPT_GO.
On 08/06/2018 07:03 AM, Richard W.M. Jones wrote:
Actually I was wrong, there *is* one substantive change over v1,
which
is this:
> + /* The client is buggy. The last option must be NBD_OPT_GO or
> + * NBD_OPT_EXPORT_NAME.
> + */
> + else {
> + nbdkit_error ("client options list didn't finish with NBD_OPT_GO
"
> + "or NBD_OPT_EXPORT_NAME");
> + return -1;
> + }
As I pointed out in the other mail, this else clause is unreachable.
> +
> return 0;
Current behaviour in nbdkit is that if the last requested option is a
recognized option, but is not NBD_OPT_EXPORT_NAME then nbdkit will
send the handshake_finish struct and continue to transport mode.
In the current code, there is no such thing as a last option except for
EXPORT_NAME - you are in a for loop that is only broken when you exceed
too many options (in which case you kill the connection) or when you
encountered EXPORT_NAME.
However I don't think nbdkit is currently behaving correctly.
In any case the proposed behaviour (above) is that in this case we
will close the connection abruptly and record an error in the log.
(There's no way to return an error back to the client when we get into
this situation.)
And how can we get in that situation in the first place?
I am not certain if this is correct or not, but I believe it is: the
NBD protocol says the only way to exit option mode and enter
transmission mode is:
"Transmission mode can be entered (by the client sending
NBD_OPT_EXPORT_NAME or by the server responding to an NBD_OPT_GO
with NBD_REP_ACK)."
(The other two ways mentioned in that section are about terminating
the connection completely).
You are correct that if EXPORT_NAME fails, or that if you don't think
the client is reaching a successful GO fast enough, then your only
recourse is to hang up on the client (preferably after sending a
last-ditch error message to the client explaining why, when that is
still permitted).
This also implies that the number of options must never be zero,
which
the spec does say in passing:
"At this point, we move on to option haggling, during which point
the client can send one or (in fixed newstyle) more options to the
~~~
server."
The current nbdkit code assumes this, so we're OK there.
Indeed - if the client hangs up without having ever sent GO or
EXPORT_NAME, then they gave up on negotiation first, and we have nothing
further to say to them.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org