On 04/07/2016 07:55 AM, Alex Bligh wrote: > This is offered as a straw-man for comment. The rationale for it > has been discussed on-list and is in the documentation. > > This applies on top of: > [PATCHv5] docs/proto.md: Clarify SHOULD / MUST / MAY etc > > so it can take advantage of the 'exposes' language. > > Signed-off-by: Alex Bligh <alex@...872...> > --- > doc/proto.md | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 59 insertions(+) > > @@ -608,6 +609,11 @@ The following request types exist: > The values of the length and offset fields in a disconnect request > are not defined. > > + A client SHOULD NOT send `NBD_CMD_DISC` if `NBD_FLAG_SEND_CLOSE` > + is set in the transmission flags field, but SHOULD use > + `NBD_CMD_CLOSE` instead; in such circumstances if it cannot wait > + for inflight requests to complete it SHOULD simply disconnect. maybe s/in such circumstances if it cannot wait/in circumstances where it cannot wait/ I'd also consider adding something like: If `NBD_FLAG_SEND_CLOSE` is not set, a client SHOULD ensure all network traffic has been flushed to the socket after sending `NBD_CMD_DISC` (especially important if using TLS), and wait for the server to disconnect, to give the server a chance to close down cleanly; however, the client MAY close the connection if the server is taking too long to respond. Also, do we know if any servers reply to NBD_CMD_DISC, or do they all disconnect? qemu-nbd does not reply; but if someone does reply, or if we think someone might write an implementation that does reply, we could add: The server MAY reply to this command with `NBD_REP_ACK`, or MAY immediately close the connection. > + > * `NBD_CMD_FLUSH` (3) > > A flush request; a write barrier. The server MUST NOT send a > @@ -638,6 +644,59 @@ The following request types exist: > > Defined by the experimental `WRITE_ZEROES` extension; see below. > > +* `NBD_CMD_CLOSE` (7) > + > + A close request. The server MUST handle all outstanding > + requests (there should be none by the spec), and then MUST send an > + `NBD_REP_ACK`. The server MUST NOT send an error reply. > + The server MUST NOT send anything after sending an `NBD_REP_ACK` > + in response to an `NBD_CMD_DISC`. s/DISC/CLOSE/ > + > + If the server receives `NBD_CMD_CLOSE` it MUST treat > + the close as a safe close (i.e. one where all operations have > + been peformed) at the point where the request is received s/peformed/performed/ > + (this relies on the client complying to the requirement of > + no outstanding requests). > + > + A client MUST wait until there are no outstanding requests > + before sending an `NBD_CMD_CLOSE`, and following this > + point it MUST treat either the receipt of an `NBD_REP_ACK` > + or a TCP level disconnect as a safe close. Maybe add: When `NBD_CMD_SEND_FLUSH` is set in the transmission flags, a client SHOULD use `NBD_CMD_FLUSH` as the final command before `NBD_CMD_CLOSE` Or is that too specific? > + > + A disconnect in any other circumstances MUST be considered an > + unsafe close, except for NBD_CMD_DISC sent or received without missing `` around NBD_CMD_DISC > + outstanding requests, which the server or client MAY treat > + as it wishes. > + > + A client MUST NOT send anything to the server after sending an > + `NBD_CMD_CLOSE` command. On receipt of an `NBD_REP_ACK` in > + response to a `NBD_CMD_CLOSE`, the client MUST close the > + connection. > + > + The server MAY close the connection after sending `NBD_REP_ACK` > + provided it has first ensured all network traffic has been > + flushed to the socket (especially important if using TLS). > + > + The length and offset fields in an `NBD_CMD_CLOSE` request > + MUST both be zero. > + > + A client MUST NOT send a `NBD_CMD_CLOSE` unless > + `NBD_FLAG_SEND_CLOSE` was set in the transmission flags > + field. > + > + The rationale for `NBD_CMD_CLOSE` is as follows. When a > + client sends an `NBD_CMD_DISC`, it does not know whether it > + has been processed or not, add: because the server is not required to reply, > hence if it sees a disconnection it > + cannot easily distinguish this from an error. When a client > + sends an `NBD_CMD_DISC`, it (logically) also wishes to close > + the connection; clients often do so to avoid the situation > + where a server never closes the connection. This close often > + happens before the server has received or processed the > + `NBD_CMD_DISC`, particularly when TLS is enabled. This > + extension allows both client and server to be sure a connection > + has been closed safely and to differentiate TCP level disconnects > + from closes of other sorts. Makes sense overall. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature