On 04/09/2016 03:51 AM, Alex Bligh wrote: > > I can't remember now exactly what Eric was suggesting (I think it was > a flag that effectively made NBD_CMD_DISC have the roughly the same > semantics as a close, i.e. gave a reply). Whether a flag that the client uses on NBD_CMD_DISC to request a server reply, or a new command, or anything else, I'm open to ideas, if we even still need this. > > The space of available flags is limited. Why use one up? Before you > say 'well you're using a flag bit anyway', I'm using a flag bit > from the server to say it supports this, which we'd need on Eric's > proposal too (I think he converted over to the command approach). > >> We can just say that the client MUST ensure that enough time has elapsed >> for the TLS layer to pass the NBD_CMD_DISC command on to the server, > > How would the client know that? I'm using Go's TLS library, and there is > no way (as far as I can tell) to ensure that. Likewise - if it's qemu's fault for not flushing the outgoing queue, then what's the right way to get that NBD_CMD_DISC flushed? > An alternative approach would be to load all the effort onto the client, > I suppose, i.e. first wait until all inflight requests have been > transmitted, then send an NBD_CMD_FLUSH, wait for the reply to that, > and then send an NBD_CMD_DISC. But in that case, we don't even need > NBD_CMD_DISC. The client might as well just disconnect. And in fact, that's what qemu clients already do. I found while implementing the NBD_CMD_CLOSE idea that qemu _already_ calls and waits for a final flush, so that there are no outstanding requests when it finally issues NBD_CMD_DISC. > > To me the whole concept of NBD_CMD_DISC is problematic. NBD is > in essence an RPC protocol. Normally in RPC protocols it's for the > client to close the connection when it knows it's done. The reasons > for an explicit disconnect command are firstly so the client knows > that any session state has been safely recorded (NBD_CMD_FLUSH > and waiting for inflight requests does that), and secondly so the > server knows the disconnect is intentional (that doesn't work with > NBD_CMD_DISC as in the wild we see clients closing the connection > immediately having sent NBD_CMD_DISC because they don't know how > long to wait for the client to 'reply' by closing the connection - > try 'qemu-img info nbd://...' for an obvious example), and this > makes the server think it's an unsafe disconnection. NBD_CMD_DISC > is currently no better than simply disconnecting. > > If you're concerned about saving commands, the easy answer is this: > > Disconnection > ============= > > Client side > ----------- > > Where the client wishes to disconnect safely, it MUST follow the following > procedure: > > * First it must wait until there are no inflight commands, i.e. every > request that it has sent has been replied to. From this point onwards > it MUST NOT send further commands. > > * Second, if the flag NBD_FLAG_SEND_FLUSH is set, it must sent a > NBD_CMD_FLUSH command, and wait for the reply. > > * At this point it closes the TCP session. And I think qemu already complies with this. > > > Server side > ----------- > > There is no explicit command to disconnect or close a connection. > If the server did not set NBD_FLAG_SEND_FLUSH, any closing of the > TCP session at a point where there are no inflight commands is a safe > close. If the server did set NBD_FLAG_SEND_FLUSH, any closing of the > TCP session where the last command replied to was NBD_CMD_FLUSH and > there are no inflight commands is a safe close. > > > > That way we can delete the pointless NBD_CMD_DISC, an do the whole thing > with -1 new commands and no new flags. > > Why not drop the NBD_CMD_FLUSH and just do the flush on a disconnect? > Because of incompletely processed requests or sent but not processed > replies. > -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature