From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60670) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aum3Y-0007N7-RS for qemu-devel@nongnu.org; Mon, 25 Apr 2016 15:20:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aum3X-0006MX-OL for qemu-devel@nongnu.org; Mon, 25 Apr 2016 15:20:28 -0400 References: <1461368452-10389-1-git-send-email-eblake@redhat.com> <1461368452-10389-37-git-send-email-eblake@redhat.com> <076E3F4D-5811-4920-880A-80E81664C587@alex.org.uk> From: Eric Blake Message-ID: <571E6DF5.5080202@redhat.com> Date: Mon, 25 Apr 2016 13:20:21 -0600 MIME-Version: 1.0 In-Reply-To: <076E3F4D-5811-4920-880A-80E81664C587@alex.org.uk> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="JbbDEm0itr03IMreWgMihRSwi7Xdx40JD" Subject: Re: [Qemu-devel] [PATCH v3 36/44] nbd: Improve handling of shutdown requests List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Bligh Cc: "qemu-devel@nongnu.org" , Kevin Wolf , Paolo Bonzini , qemu block , Max Reitz This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --JbbDEm0itr03IMreWgMihRSwi7Xdx40JD Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 04/25/2016 03:47 AM, Alex Bligh wrote: >=20 > On 23 Apr 2016, at 00:40, Eric Blake wrote: >=20 >> NBD commit 6d34500b clarified how clients and servers are supposed >> to behave before closing a connection. It added NBD_REP_ERR_SHUTDOWN >> (for the server to announce it is about to go away during option >> haggling, so the client should quit sending NBD_OPT_* other than >> NBD_OPT_ABORT) and ESHUTDOWN (for the server to announce it is about >> to go away during transmission, so the client should quit sending >> NBD_CMD_* other than NBD_CMD_DISC). It also clarified that >> NBD_OPT_ABORT gets a reply, while NBD_CMD_DISC does not. >> >> This patch merely adds the missing reply to NBD_OPT_ABORT and teaches >> the client to recognize server errors. Actually teaching the server >> to send NBD_REP_ERR_SHUTDOWN or ESHUTDOWN would require knowing that >> the server has been requested to shut down soon (maybe we could do >> that by installing a SIGINT handler in qemu-nbd, which transitions >> from RUNNING to a new state that waits for the client to react, >> rather than just out-right quitting). >> >> Signed-off-by: Eric Blake >> --- >> @@ -484,6 +486,10 @@ static int nbd_negotiate_options(NBDClient *clien= t) >> if (ret < 0) { >> return ret; >> } >> + /* Let the client keep trying, unless they asked to q= uit */ >> + if (clientflags =3D=3D NBD_OPT_ABORT) { >=20 > OK that's totally confusing. clientflags is not the client flags. clien= tflags > is the NBD option ID, which happens to be the two bytes after the NBD O= PT magic, > which is the client flags if we were doing oldstyle negotiation, not ne= wstyle > negotiation. Yes, 'clientflags' is a poor name; I should rename it in a separate patch. It is the option-negotiation command type. >=20 > Except: >=20 >> + return -EINVAL; >> + } >> break; >> } >> } else if (fixedNewstyle) { >=20 > So the above is for NewStyle (not fixedNewStyle)? The above is for fixedNewStyle when TLS is not negotiated yet; the 'else if' is for fixedNewStyle after TLS has been negotiated. Prior to TLS, we have to special-case NBD_OPT_ABORT and actually quit. >=20 > In which case more than one option isn't even supported, so what's the > stuff purporting to handle TLS doing there? >=20 > Confused ... Sounds like a cleanup patch as a prerequisite on my next respin would help, then. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --JbbDEm0itr03IMreWgMihRSwi7Xdx40JD Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJXHm31AAoJEKeha0olJ0NqlZQH/2Bz/gwpG3e3iBXvlbVVmw1G Zm7csFF9yYU83233+ss0MVN3oqZP3fNLpMXzg9JzCHTMVTvuFa8QYhwuIqZ+rzFj w8EQ/g0petXqskhw0ZAQGh59L3AighuoR2h/gFdI1wlRgtHi9Y41hTNF4Mze6fCj WeyjlJUEmJqDpI6sjqqxJB1CO0k/NsitXgOJMLjI5fbLaJ04XhEBr1PWqlBCXM2c T2nVsXGWTnzBbj/tXA1nubJcOOpIEUXO6h8AZRVPvZXyfHIpjKdw11AoBKha8eCp NNZ/cbYEuVsRhnrC+0ZR+zlx1CaJIARyreaZYv4gTtEr/lmBAoJ7wwjwKrAd8F0= =Liqa -----END PGP SIGNATURE----- --JbbDEm0itr03IMreWgMihRSwi7Xdx40JD--