[{"id":1803927,"web_url":"http://patchwork.ozlabs.org/comment/1803927/","msgid":"<025d95af-d978-32a2-6a13-754496b9cc19@redhat.com>","list_archive_url":null,"date":"2017-11-13T19:33:35","subject":"Re: [Qemu-devel] [PATCH v6 10/12] nbd/client: prepare\n\tnbd_receive_reply for structured reply","submitter":{"id":6591,"url":"http://patchwork.ozlabs.org/api/people/6591/","name":"Eric Blake","email":"eblake@redhat.com"},"content":"On 10/27/2017 05:40 AM, Eric Blake wrote:\n> From: Vladimir Sementsov-Ogievskiy \n> \n> In following patch nbd_receive_reply will be used both for simple\n> and structured reply header receiving.\n> NBDReply is altered into union of simple reply header and structured\n> reply chunk header, simple error translation moved to block/nbd-client\n> to be consistent with further structured reply error translation.\n> \n> Signed-off-by: Vladimir Sementsov-Ogievskiy \n> Signed-off-by: Eric Blake \n> \n\n> - trace_nbd_receive_reply(magic, reply->error, nbd_err_lookup(reply->error),\n> - reply->handle);\n> - reply->error = nbd_errno_to_system_errno(reply->error);\n> -\n> - if (reply->error == ESHUTDOWN) {\n> - /* This works even on mingw which lacks a native ESHUTDOWN */\n> - error_setg(errp, \"server shutting down\");\n> + trace_nbd_receive_simple_reply(reply->simple.error,\n> + nbd_err_lookup(reply->simple.error),\n> + reply->handle);\n> + if (reply->simple.error == NBD_ESHUTDOWN) {\n> + /* This works even on mingw which lacks a native ESHUTDOWN */\n> + error_setg(errp, \"server shutting down\");\n> + return -EINVAL;\n> + }\n> + break;\n> + case NBD_STRUCTURED_REPLY_MAGIC:\n> + ret = nbd_receive_structured_reply_chunk(ioc, &reply->structured, errp);\n> + if (ret < 0) {\n> + break;\n> + }\n> + trace_nbd_receive_structured_reply_chunk(reply->structured.flags,\n> + reply->structured.type,\n> + reply->structured.handle,\n> + reply->structured.length);\n> + break;\n\nOuch. This change means that we now handle NBD_ESHUTDOWN differently if\nit was sent as a simple message than if it is sent as a structured reply\nchunk. Furthermore, reading the NBD spec:\n\n> On a server shutdown, the server SHOULD wait for inflight requests to be serviced prior to initiating a hard disconnect. A server MAY speed this process up by issuing error replies. The error value issued in respect of these requests and any subsequently received requests SHOULD be ESHUTDOWN.\n> \n> If the client receives an ESHUTDOWN error it MUST initiate a soft disconnect.\n> \n> The client MAY issue a soft disconnect at any time, but SHOULD wait until there are no inflight requests first.\n\nWe had a pre-existing bug: our behavior in the simple case is wrong - we\nare causing a hard disconnect (the server kills the connection) instead\nof initiating a soft disconnect (wait for all other pending replies to\ncome in, then send no further requests other than NBD_CMD_DISC).\n\nA hard disconnect is not the end of the world, even if it is unclean; on\nthe other hand, not doing a disconnect at all means the client will\ncontinue to see ESHUTDOWN errors until it does shutdown. Meanwhile,\nrefactoring block/nbd-client.c to properly drain all existing requests\nwhile preventing future requests seems too risky for the 2.11 timeframe.\n But I don't like the discrepancy between the two styles, so for 2.11, I\nthink what I will do is rip out the special-casing of NBD_ESHUTDOWN.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3ybLVd0nNKz9sBZ\n\tfor ;\n\tTue, 14 Nov 2017 06:36:41 +1100 (AEDT)","from localhost ([::1]:56065 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1eEKX9-0004qn-5e\n\tfor incoming@patchwork.ozlabs.org; Mon, 13 Nov 2017 14:36:39 -0500","from eggs.gnu.org ([2001:4830:134:3::10]:45867)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1eEKUL-0003Hj-0a\n\tfor qemu-devel@nongnu.org; Mon, 13 Nov 2017 14:33:46 -0500","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1eEKUJ-0003h2-T8\n\tfor qemu-devel@nongnu.org; Mon, 13 Nov 2017 14:33:45 -0500","from mx1.redhat.com ([209.132.183.28]:50962)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1eEKUE-0003ds-0b; Mon, 13 Nov 2017 14:33:38 -0500","from smtp.corp.redhat.com\n\t(int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 262961366;\n\tMon, 13 Nov 2017 19:33:37 +0000 (UTC)","from [10.10.123.105] (ovpn-123-105.rdu2.redhat.com [10.10.123.105])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 17E0998E70;\n\tMon, 13 Nov 2017 19:33:35 +0000 (UTC)"],"To":"qemu-devel@nongnu.org","References":"<20171027104037.8319-1-eblake@redhat.com>\n\t<20171027104037.8319-11-eblake@redhat.com>","From":"Eric Blake ","Openpgp":"url=http://people.redhat.com/eblake/eblake.gpg","Organization":"Red Hat, Inc.","Message-ID":"<025d95af-d978-32a2-6a13-754496b9cc19@redhat.com>","Date":"Mon, 13 Nov 2017 13:33:35 -0600","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.4.0","MIME-Version":"1.0","In-Reply-To":"<20171027104037.8319-11-eblake@redhat.com>","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"mFsN9BhGp3lBLUqDmjkfJXJbMgF4nWapa\"","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.13","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.29]);\n\tMon, 13 Nov 2017 19:33:37 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","X-Content-Filtered-By":"Mailman/MimeDel 2.1.21","Subject":"Re: [Qemu-devel] [PATCH v6 10/12] nbd/client: prepare\n\tnbd_receive_reply for structured reply","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Kevin Wolf , pbonzini@redhat.com,\n\tvsementsov@virtuozzo.com, qemu-block@nongnu.org,\n\tMax Reitz ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]