[{"id":1770551,"web_url":"http://patchwork.ozlabs.org/comment/1770551/","msgid":"<b016f0b6-c7bd-eef3-caee-47f7da7d8deb@redhat.com>","list_archive_url":null,"date":"2017-09-18T22:27:18","subject":"Re: [Qemu-devel] [PATCH v2 6/7] block/nbd-client: early fail\n\tnbd_read_reply_entry if s->quit is set","submitter":{"id":6591,"url":"http://patchwork.ozlabs.org/api/people/6591/","name":"Eric Blake","email":"eblake@redhat.com"},"content":"On 09/18/2017 08:59 AM, Vladimir Sementsov-Ogievskiy wrote:\n> Do not continue any operation if s->quit is set in parallel.\n> \n> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>\n> ---\n>  block/nbd-client.c | 7 +++----\n>  1 file changed, 3 insertions(+), 4 deletions(-)\n> \n> diff --git a/block/nbd-client.c b/block/nbd-client.c\n> index 280147e6a7..f80a4c5564 100644\n> --- a/block/nbd-client.c\n> +++ b/block/nbd-client.c\n> @@ -81,7 +81,7 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)\n>          if (ret < 0) {\n>              error_report_err(local_err);\n>          }\n> -        if (ret <= 0) {\n> +        if (ret <= 0 || s->quit) {\n>              break;\n>          }\n\nI'm still not convinced this helps: either nbd_receive_reply() already\nreturned an error, or we got a successful reply header, at which point\neither the command is done (no need to abort the command early - it\nsucceeded) or it is a read command (and we should detect at the point\nwhere we try to populate the qiov that we don't want to read any more).\nIt depends on your 3/7 patch for the fact that we even try to read the\nqiov directly in this while loop rather than in the coroutine handler,\nwhere Paolo questioned whether we need that change.\n\n> @@ -105,9 +105,8 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)\n>              assert(qiov != NULL);\n>              assert(s->requests[i].request->len ==\n>                     iov_size(qiov->iov, qiov->niov));\n> -            if (qio_channel_readv_all(s->ioc, qiov->iov, qiov->niov,\n> -                                      NULL) < 0)\n> -            {\n> +            ret = qio_channel_readv_all(s->ioc, qiov->iov, qiov->niov, NULL);\n> +            if (ret < 0 || s->quit) {\n>                  s->requests[i].ret = -EIO;\n>                  break;\n>              }\n\nThe placement here looks odd. Either we should not attempt the read\nbecause s->quit was already set (okay, your first addition makes sense\nin that light), or we succeeded at the read (at which point, why do we\nneed to claim it failed?).\n\nI'm trying to look forward to structured reads, where we will have to\ndeal with more than one server message in reply to a single client\nrequest.  For read, we just piece together portions of the qiov until\nthe server has sent us all the pieces.  But for block query, I really do\nthink we'll want to defer to specialized handlers for doing further\nreads (the amount of data to be read from the server is not known by the\nclient a priori, so it is a two-part read, one to get the length, and\nanother to read that remaining length); if we need some sort of callback\nfunction rather than cramming all the logic here in the main read loop,\nthen the qio_channel_readv_all for read commands would belong in the\ncallback, and it is more a question of the main read loop checking\nwhether there is an early quit condition before calling into the callback.","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=eblake@redhat.com"],"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 3xx0yC6sqFz9s7m\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 19 Sep 2017 08:28:03 +1000 (AEST)","from localhost ([::1]:39163 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1du4WI-00020t-3E\n\tfor incoming@patchwork.ozlabs.org; Mon, 18 Sep 2017 18:28:02 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:33784)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <eblake@redhat.com>) id 1du4Vm-0001zE-3a\n\tfor qemu-devel@nongnu.org; Mon, 18 Sep 2017 18:27:31 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <eblake@redhat.com>) id 1du4Vk-0004pL-PD\n\tfor qemu-devel@nongnu.org; Mon, 18 Sep 2017 18:27:30 -0400","from mx1.redhat.com ([209.132.183.28]:55468)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <eblake@redhat.com>)\n\tid 1du4Ve-0004ji-4x; Mon, 18 Sep 2017 18:27:22 -0400","from smtp.corp.redhat.com\n\t(int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15])\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 1BC36883A0;\n\tMon, 18 Sep 2017 22:27:21 +0000 (UTC)","from [10.10.124.97] (ovpn-124-97.rdu2.redhat.com [10.10.124.97])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id D063C5D6A4;\n\tMon, 18 Sep 2017 22:27:19 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 1BC36883A0","To":"Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,\n\tqemu-block@nongnu.org, qemu-devel@nongnu.org","References":"<20170918135935.255591-1-vsementsov@virtuozzo.com>\n\t<20170918135935.255591-7-vsementsov@virtuozzo.com>","From":"Eric Blake <eblake@redhat.com>","Openpgp":"url=http://people.redhat.com/eblake/eblake.gpg","Organization":"Red Hat, Inc.","Message-ID":"<b016f0b6-c7bd-eef3-caee-47f7da7d8deb@redhat.com>","Date":"Mon, 18 Sep 2017 17:27:18 -0500","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170918135935.255591-7-vsementsov@virtuozzo.com>","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"Q37wCm5Nw2XkvcMnC5JqwPm4PHWwdVk8D\"","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.15","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.26]);\n\tMon, 18 Sep 2017 22:27:21 +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 v2 6/7] block/nbd-client: early fail\n\tnbd_read_reply_entry if s->quit is set","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"kwolf@redhat.com, pbonzini@redhat.com, den@openvz.org, mreitz@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1770795,"web_url":"http://patchwork.ozlabs.org/comment/1770795/","msgid":"<48c3e73c-d142-7a46-a6be-b4c05349aa81@virtuozzo.com>","list_archive_url":null,"date":"2017-09-19T09:43:45","subject":"Re: [Qemu-devel] [PATCH v2 6/7] block/nbd-client: early fail\n\tnbd_read_reply_entry if s->quit is set","submitter":{"id":66592,"url":"http://patchwork.ozlabs.org/api/people/66592/","name":"Vladimir Sementsov-Ogievskiy","email":"vsementsov@virtuozzo.com"},"content":"19.09.2017 01:27, Eric Blake wrote:\n> On 09/18/2017 08:59 AM, Vladimir Sementsov-Ogievskiy wrote:\n>> Do not continue any operation if s->quit is set in parallel.\n>>\n>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>\n>> ---\n>>   block/nbd-client.c | 7 +++----\n>>   1 file changed, 3 insertions(+), 4 deletions(-)\n>>\n>> diff --git a/block/nbd-client.c b/block/nbd-client.c\n>> index 280147e6a7..f80a4c5564 100644\n>> --- a/block/nbd-client.c\n>> +++ b/block/nbd-client.c\n>> @@ -81,7 +81,7 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)\n>>           if (ret < 0) {\n>>               error_report_err(local_err);\n>>           }\n>> -        if (ret <= 0) {\n>> +        if (ret <= 0 || s->quit) {\n>>               break;\n>>           }\n> I'm still not convinced this helps: either nbd_receive_reply() already\n> returned an error, or we got a successful reply header, at which point\n> either the command is done (no need to abort the command early - it\n> succeeded) or it is a read command (and we should detect at the point\n> where we try to populate the qiov that we don't want to read any more).\n> It depends on your 3/7 patch for the fact that we even try to read the\n> qiov directly in this while loop rather than in the coroutine handler,\n> where Paolo questioned whether we need that change.\n>\n>> @@ -105,9 +105,8 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)\n>>               assert(qiov != NULL);\n>>               assert(s->requests[i].request->len ==\n>>                      iov_size(qiov->iov, qiov->niov));\n>> -            if (qio_channel_readv_all(s->ioc, qiov->iov, qiov->niov,\n>> -                                      NULL) < 0)\n>> -            {\n>> +            ret = qio_channel_readv_all(s->ioc, qiov->iov, qiov->niov, NULL);\n>> +            if (ret < 0 || s->quit) {\n>>                   s->requests[i].ret = -EIO;\n>>                   break;\n>>               }\n> The placement here looks odd. Either we should not attempt the read\n> because s->quit was already set (okay, your first addition makes sense\n> in that light), or we succeeded at the read (at which point, why do we\n> need to claim it failed?).\n>\n> I'm trying to look forward to structured reads, where we will have to\n> deal with more than one server message in reply to a single client\n> request.  For read, we just piece together portions of the qiov until\n> the server has sent us all the pieces.  But for block query, I really do\n> think we'll want to defer to specialized handlers for doing further\n> reads (the amount of data to be read from the server is not known by the\n> client a priori, so it is a two-part read, one to get the length, and\n> another to read that remaining length); if we need some sort of callback\n> function rather than cramming all the logic here in the main read loop,\n\nby patch 3 I do not mean in any way that I want to have all reading \nstaff in one function, ofcourse it should be refactored\nfor block-status addition. I just want to have all reading staff in one \ncoroutine. Reading from ioc is sequential, so it's\nnative to do it sequentially in one coroutine, without switches.\n\n> then the qio_channel_readv_all for read commands would belong in the\n> callback, and it is more a question of the main read loop checking\n> whether there is an early quit condition before calling into the callback.\n>\n\nIf I understand correctly, the discussion is:\n\nme: if something fails - fail all other parallel operations - it's \nsimple and clear, but we need to check s->quit after every possible yield\n\nyou: .... _not_ all other parallel operations. - may be it is better, \nbut in this case we need carefully define, which parallel operation we fail\n\n  on s->quit and which not and understand all effects of this division.","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","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 3xxHyX6RN4z9s7F\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 19 Sep 2017 19:44:20 +1000 (AEST)","from localhost ([::1]:41019 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1duF4l-0004s9-2L\n\tfor incoming@patchwork.ozlabs.org; Tue, 19 Sep 2017 05:44:19 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:57491)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <vsementsov@virtuozzo.com>) id 1duF4Q-0004r2-A1\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 05:43:59 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <vsementsov@virtuozzo.com>) id 1duF4P-00004q-8S\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 05:43:58 -0400","from mailhub.sw.ru ([195.214.232.25]:22780 helo=relay.sw.ru)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <vsementsov@virtuozzo.com>)\n\tid 1duF4O-0008Qq-Rt\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 05:43:57 -0400","from [172.16.24.243] (msk-vpn.virtuozzo.com [195.214.232.6])\n\tby relay.sw.ru (8.13.4/8.13.4) with ESMTP id v8J9hjIs008657;\n\tTue, 19 Sep 2017 12:43:45 +0300 (MSK)"],"To":"Eric Blake <eblake@redhat.com>, qemu-block@nongnu.org,\n\tqemu-devel@nongnu.org","References":"<20170918135935.255591-1-vsementsov@virtuozzo.com>\n\t<20170918135935.255591-7-vsementsov@virtuozzo.com>\n\t<b016f0b6-c7bd-eef3-caee-47f7da7d8deb@redhat.com>","From":"Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>","Message-ID":"<48c3e73c-d142-7a46-a6be-b4c05349aa81@virtuozzo.com>","Date":"Tue, 19 Sep 2017 12:43:45 +0300","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<b016f0b6-c7bd-eef3-caee-47f7da7d8deb@redhat.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"quoted-printable","X-MIME-Autoconverted":"from 8bit to quoted-printable by relay.sw.ru id\n\tv8J9hjIs008657","X-detected-operating-system":"by eggs.gnu.org: OpenBSD 3.x [fuzzy]","X-Received-From":"195.214.232.25","Subject":"Re: [Qemu-devel] [PATCH v2 6/7] block/nbd-client: early fail\n\tnbd_read_reply_entry if s->quit is set","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"kwolf@redhat.com, pbonzini@redhat.com, den@openvz.org, mreitz@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1770811,"web_url":"http://patchwork.ozlabs.org/comment/1770811/","msgid":"<268eb4fb-b61e-403c-0c5a-80bab84ef4e2@redhat.com>","list_archive_url":null,"date":"2017-09-19T10:03:48","subject":"Re: [Qemu-devel] [PATCH v2 6/7] block/nbd-client: early fail\n\tnbd_read_reply_entry if s->quit is set","submitter":{"id":2701,"url":"http://patchwork.ozlabs.org/api/people/2701/","name":"Paolo Bonzini","email":"pbonzini@redhat.com"},"content":"On 19/09/2017 11:43, Vladimir Sementsov-Ogievskiy wrote:\n>>\n>> I'm trying to look forward to structured reads, where we will have to\n>> deal with more than one server message in reply to a single client\n>> request.  For read, we just piece together portions of the qiov until\n>> the server has sent us all the pieces.  But for block query, I really do\n>> think we'll want to defer to specialized handlers for doing further\n>> reads (the amount of data to be read from the server is not known by the\n>> client a priori, so it is a two-part read, one to get the length, and\n>> another to read that remaining length); if we need some sort of callback\n>> function rather than cramming all the logic here in the main read loop,\n> \n> by patch 3 I do not mean in any way that I want to have all reading\n> staff in one function, ofcourse it should be refactored\n> for block-status addition. I just want to have all reading staff in one\n> coroutine. Reading from ioc is sequential, so it's\n> native to do it sequentially in one coroutine, without switches.\n\nBut why is that a goal?  See it as a state machine that goes between the\n\"waiting for header\" and \"waiting for payload\" states.  Reading header\ncorresponds to read_reply_co waiting on the socket (and reading when\nit's ready); reading payload corresponds to the request coroutine\nwaiting on the socket and reading when it's ready.\n\nPaolo","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=pbonzini@redhat.com"],"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 3xxJPj6qrNz9s7h\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 19 Sep 2017 20:04:25 +1000 (AEST)","from localhost ([::1]:41192 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1duFOC-0006Kn-4V\n\tfor incoming@patchwork.ozlabs.org; Tue, 19 Sep 2017 06:04:24 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:37783)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <pbonzini@redhat.com>) id 1duFNo-0006Ir-Ti\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 06:04:01 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <pbonzini@redhat.com>) id 1duFNn-0003KJ-Ve\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 06:04:00 -0400","from mx1.redhat.com ([209.132.183.28]:43602)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <pbonzini@redhat.com>)\n\tid 1duFNh-0003Hu-VA; Tue, 19 Sep 2017 06:03:54 -0400","from smtp.corp.redhat.com\n\t(int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15])\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 D3B51356F6;\n\tTue, 19 Sep 2017 10:03:52 +0000 (UTC)","from [10.36.117.61] (ovpn-117-61.ams2.redhat.com [10.36.117.61])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 632415D6A4;\n\tTue, 19 Sep 2017 10:03:50 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com D3B51356F6","To":"Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,\n\tEric Blake <eblake@redhat.com>, qemu-block@nongnu.org,\n\tqemu-devel@nongnu.org","References":"<20170918135935.255591-1-vsementsov@virtuozzo.com>\n\t<20170918135935.255591-7-vsementsov@virtuozzo.com>\n\t<b016f0b6-c7bd-eef3-caee-47f7da7d8deb@redhat.com>\n\t<48c3e73c-d142-7a46-a6be-b4c05349aa81@virtuozzo.com>","From":"Paolo Bonzini <pbonzini@redhat.com>","Message-ID":"<268eb4fb-b61e-403c-0c5a-80bab84ef4e2@redhat.com>","Date":"Tue, 19 Sep 2017 12:03:48 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<48c3e73c-d142-7a46-a6be-b4c05349aa81@virtuozzo.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.15","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.30]);\n\tTue, 19 Sep 2017 10:03:53 +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","Subject":"Re: [Qemu-devel] [PATCH v2 6/7] block/nbd-client: early fail\n\tnbd_read_reply_entry if s->quit is set","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"kwolf@redhat.com, den@openvz.org, mreitz@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1770881,"web_url":"http://patchwork.ozlabs.org/comment/1770881/","msgid":"<bb0f5896-7cb2-0a74-5ecd-2f9910ee7264@virtuozzo.com>","list_archive_url":null,"date":"2017-09-19T11:07:52","subject":"Re: [Qemu-devel] [PATCH v2 6/7] block/nbd-client: early fail\n\tnbd_read_reply_entry if s->quit is set","submitter":{"id":66592,"url":"http://patchwork.ozlabs.org/api/people/66592/","name":"Vladimir Sementsov-Ogievskiy","email":"vsementsov@virtuozzo.com"},"content":"19.09.2017 13:03, Paolo Bonzini wrote:\n> On 19/09/2017 11:43, Vladimir Sementsov-Ogievskiy wrote:\n>>> I'm trying to look forward to structured reads, where we will have to\n>>> deal with more than one server message in reply to a single client\n>>> request.  For read, we just piece together portions of the qiov until\n>>> the server has sent us all the pieces.  But for block query, I really do\n>>> think we'll want to defer to specialized handlers for doing further\n>>> reads (the amount of data to be read from the server is not known by the\n>>> client a priori, so it is a two-part read, one to get the length, and\n>>> another to read that remaining length); if we need some sort of callback\n>>> function rather than cramming all the logic here in the main read loop,\n>> by patch 3 I do not mean in any way that I want to have all reading\n>> staff in one function, ofcourse it should be refactored\n>> for block-status addition. I just want to have all reading staff in one\n>> coroutine. Reading from ioc is sequential, so it's\n>> native to do it sequentially in one coroutine, without switches.\n> But why is that a goal?  See it as a state machine that goes between the\n> \"waiting for header\" and \"waiting for payload\" states.  Reading header\n> corresponds to read_reply_co waiting on the socket (and reading when\n> it's ready); reading payload corresponds to the request coroutine\n> waiting on the socket and reading when it's ready.\n>\n> Paolo\n\n\nI just dislike public access to ioc for commands and extra coroutine \nswitching and synchronization.","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","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 3xxKvv2Nh5z9s7B\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 19 Sep 2017 21:12:11 +1000 (AEST)","from localhost ([::1]:41608 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1duGRl-00048V-DX\n\tfor incoming@patchwork.ozlabs.org; Tue, 19 Sep 2017 07:12:09 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:39041)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <vsementsov@virtuozzo.com>) id 1duGNm-0001xp-Lf\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 07:08:08 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <vsementsov@virtuozzo.com>) id 1duGNl-000307-JB\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 07:08:02 -0400","from mailhub.sw.ru ([195.214.232.25]:22636 helo=relay.sw.ru)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <vsementsov@virtuozzo.com>)\n\tid 1duGNl-0002yT-5h\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 07:08:01 -0400","from [172.16.24.243] (msk-vpn.virtuozzo.com [195.214.232.6])\n\tby relay.sw.ru (8.13.4/8.13.4) with ESMTP id v8JB7qsg002849;\n\tTue, 19 Sep 2017 14:07:52 +0300 (MSK)"],"To":"Paolo Bonzini <pbonzini@redhat.com>, Eric Blake <eblake@redhat.com>,\n\tqemu-block@nongnu.org, qemu-devel@nongnu.org","References":"<20170918135935.255591-1-vsementsov@virtuozzo.com>\n\t<20170918135935.255591-7-vsementsov@virtuozzo.com>\n\t<b016f0b6-c7bd-eef3-caee-47f7da7d8deb@redhat.com>\n\t<48c3e73c-d142-7a46-a6be-b4c05349aa81@virtuozzo.com>\n\t<268eb4fb-b61e-403c-0c5a-80bab84ef4e2@redhat.com>","From":"Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>","Message-ID":"<bb0f5896-7cb2-0a74-5ecd-2f9910ee7264@virtuozzo.com>","Date":"Tue, 19 Sep 2017 14:07:52 +0300","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<268eb4fb-b61e-403c-0c5a-80bab84ef4e2@redhat.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","X-detected-operating-system":"by eggs.gnu.org: OpenBSD 3.x [fuzzy]","X-Received-From":"195.214.232.25","Subject":"Re: [Qemu-devel] [PATCH v2 6/7] block/nbd-client: early fail\n\tnbd_read_reply_entry if s->quit is set","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"kwolf@redhat.com, den@openvz.org, mreitz@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}}]