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
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Find and fix application performance issues faster with Applications Manager Applications Manager provides deep performance insights into multiple tiers of your business applications. It resolves application problems quickly and reduces your MTTR. Get your free trial! http://pubads.g.doubleclick.net/ gampad/clk?id=1444514301&iu=/ca-pub-7940484522588532
_______________________________________________ Nbd-general mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/nbd-general
