[{"id":1760429,"web_url":"http://patchwork.ozlabs.org/comment/1760429/","msgid":"","list_archive_url":null,"date":"2017-08-30T19:34:59","subject":"Re: [Qemu-devel] [PATCH] io: add new qio_channel_{readv, writev,\n\tread, write}_all functions","submitter":{"id":6591,"url":"http://patchwork.ozlabs.org/api/people/6591/","name":"Eric Blake","email":"eblake@redhat.com"},"content":"On 08/30/2017 08:56 AM, Daniel P. Berrange wrote:\n> These functions wait until they are able to read / write the full\n> requested data buffer(s).\n> \n\nHmm, sounds like the NBD code would benefit from using this in a\nfollowup patch.\n\n> Signed-off-by: Daniel P. Berrange \n> ---\n> \n> This patch combines these two previous proposals:\n> \n> - https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg01536.html\n> - https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg04041.html\n> \n> and switches the test suite over to use the new APIs so we get\n> coverage by all the tests/test-io-channel-* test programs\n> \n\n> /**\n> + * qio_channel_readv_all:\n> + * @ioc: the channel object\n> + * @iov: the array of memory regions to read data into\n> + * @niov: the length of the @iov array\n> + * @errp: pointer to a NULL-initialized error object\n> + *\n> + * Read data from the IO channel, storing it in the\n> + * memory regions referenced by @iov. Each element\n> + * in the @iov will be fully populated with data\n> + * before the next one is used. The @niov parameter\n> + * specifies the total number of elements in @iov.\n> + *\n> + * The function will wait for all requested data\n> + * to be read, yielding from the current couroutine\n\ns/couroutine/coroutine/\n\n> + * if required.\n> + *\n> + * If end-of-file occurrs before all requested data\n\ns/occurrs/occurs/\n\n> + * has been read, an error will be reported.\n> + *\n> + * Returns: 0 if all bytes were read, or -1 on error\n> + */\n\n> +/**\n> + * qio_channel_writev_all:\n> + * @ioc: the channel object\n> + * @iov: the array of memory regions to write data from\n> + * @niov: the length of the @iov array\n> + * @errp: pointer to a NULL-initialized error object\n> + *\n> + * Write data to the IO channel, reading it from the\n> + * memory regions referenced by @iov. Each element\n> + * in the @iov will be fully sent, before the next\n> + * one is used. The @niov parameter specifies the\n> + * total number of elements in @iov.\n> + *\n> + * The function will wait for all requested data\n> + * to be written, yielding from the current couroutine\n\ns/couroutine/coroutine/\n\n> +++ b/io/channel.c\n\n> +int qio_channel_readv_all(QIOChannel *ioc,\n\n> + while (nlocal_iov > 0) {\n> + ssize_t len;\n> + len = qio_channel_readv(ioc, local_iov, nlocal_iov, errp);\n> + if (len == QIO_CHANNEL_ERR_BLOCK) {\n> + qio_channel_wait(ioc, G_IO_IN);\n> + continue;\n> + } else if (len < 0) {\n> + error_setg_errno(errp, EIO,\n> + \"Channel was not able to read full iov\");\n\nShould we report -len instead of EIO here?\n\n> +int qio_channel_writev_all(QIOChannel *ioc,\n\n> + while (nlocal_iov > 0) {\n> + ssize_t len;\n> + len = qio_channel_writev(ioc, local_iov, nlocal_iov, errp);\n> + if (len == QIO_CHANNEL_ERR_BLOCK) {\n> + qio_channel_wait(ioc, G_IO_OUT);\n> + continue;\n> + }\n> + if (len < 0) {\n> + error_setg_errno(errp, EIO,\n> + \"Channel was not able to write full iov\");\n\nand again\n\nWith the typos fixed, and either an explanation why EIO is better or\nelse a fix to preserve errno,\n\nReviewed-by: Eric Blake ","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=)","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=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 3xjG211Z27z9sP5\n\tfor ;\n\tThu, 31 Aug 2017 05:35:35 +1000 (AEST)","from localhost ([::1]:52365 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dn8lq-00008S-Nl\n\tfor incoming@patchwork.ozlabs.org; Wed, 30 Aug 2017 15:35:26 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:44622)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dn8lW-00005y-Tu\n\tfor qemu-devel@nongnu.org; Wed, 30 Aug 2017 15:35:08 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dn8lS-0005aI-OD\n\tfor qemu-devel@nongnu.org; Wed, 30 Aug 2017 15:35:06 -0400","from mx1.redhat.com ([209.132.183.28]:36306)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dn8lS-0005ZB-Ej\n\tfor qemu-devel@nongnu.org; Wed, 30 Aug 2017 15:35:02 -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 1A1A7356D9\n\tfor ; Wed, 30 Aug 2017 19:35:01 +0000 (UTC)","from [10.10.122.186] (ovpn-122-186.rdu2.redhat.com [10.10.122.186])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 4004217106;\n\tWed, 30 Aug 2017 19:35:00 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 1A1A7356D9","To":"\"Daniel P. Berrange\" , qemu-devel@nongnu.org","References":"<20170830135611.27678-1-berrange@redhat.com>","From":"Eric Blake ","Openpgp":"url=http://people.redhat.com/eblake/eblake.gpg","Organization":"Red Hat, Inc.","Message-ID":"","Date":"Wed, 30 Aug 2017 14:34:59 -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":"<20170830135611.27678-1-berrange@redhat.com>","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"HbsNbdoi59hmTQJkFSQjaftEdUxUlgMgP\"","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\tWed, 30 Aug 2017 19:35:01 +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] io: add new qio_channel_{readv, writev,\n\tread, write}_all functions","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":"Paolo Bonzini , Juan Quintela ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1760769,"web_url":"http://patchwork.ozlabs.org/comment/1760769/","msgid":"<20170831091723.GC17315@redhat.com>","list_archive_url":null,"date":"2017-08-31T09:17:23","subject":"Re: [Qemu-devel] [PATCH] io: add new qio_channel_{readv, writev,\n\tread, write}_all functions","submitter":{"id":2694,"url":"http://patchwork.ozlabs.org/api/people/2694/","name":"Daniel P. Berrangé","email":"berrange@redhat.com"},"content":"On Wed, Aug 30, 2017 at 02:34:59PM -0500, Eric Blake wrote:\n> On 08/30/2017 08:56 AM, Daniel P. Berrange wrote:\n> > These functions wait until they are able to read / write the full\n> > requested data buffer(s).\n> > \n> \n> Hmm, sounds like the NBD code would benefit from using this in a\n> followup patch.\n> \n> > Signed-off-by: Daniel P. Berrange \n> > ---\n> > \n> > This patch combines these two previous proposals:\n> > \n> > - https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg01536.html\n> > - https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg04041.html\n> > \n> > and switches the test suite over to use the new APIs so we get\n> > coverage by all the tests/test-io-channel-* test programs\n> > \n> \n> > /**\n> > + * qio_channel_readv_all:\n> > + * @ioc: the channel object\n> > + * @iov: the array of memory regions to read data into\n> > + * @niov: the length of the @iov array\n> > + * @errp: pointer to a NULL-initialized error object\n> > + *\n> > + * Read data from the IO channel, storing it in the\n> > + * memory regions referenced by @iov. Each element\n> > + * in the @iov will be fully populated with data\n> > + * before the next one is used. The @niov parameter\n> > + * specifies the total number of elements in @iov.\n> > + *\n> > + * The function will wait for all requested data\n> > + * to be read, yielding from the current couroutine\n> \n> s/couroutine/coroutine/\n> \n> > + * if required.\n> > + *\n> > + * If end-of-file occurrs before all requested data\n> \n> s/occurrs/occurs/\n> \n> > + * has been read, an error will be reported.\n> > + *\n> > + * Returns: 0 if all bytes were read, or -1 on error\n> > + */\n> \n> > +/**\n> > + * qio_channel_writev_all:\n> > + * @ioc: the channel object\n> > + * @iov: the array of memory regions to write data from\n> > + * @niov: the length of the @iov array\n> > + * @errp: pointer to a NULL-initialized error object\n> > + *\n> > + * Write data to the IO channel, reading it from the\n> > + * memory regions referenced by @iov. Each element\n> > + * in the @iov will be fully sent, before the next\n> > + * one is used. The @niov parameter specifies the\n> > + * total number of elements in @iov.\n> > + *\n> > + * The function will wait for all requested data\n> > + * to be written, yielding from the current couroutine\n> \n> s/couroutine/coroutine/\n> \n> > +++ b/io/channel.c\n> \n> > +int qio_channel_readv_all(QIOChannel *ioc,\n> \n> > + while (nlocal_iov > 0) {\n> > + ssize_t len;\n> > + len = qio_channel_readv(ioc, local_iov, nlocal_iov, errp);\n> > + if (len == QIO_CHANNEL_ERR_BLOCK) {\n> > + qio_channel_wait(ioc, G_IO_IN);\n> > + continue;\n> > + } else if (len < 0) {\n> > + error_setg_errno(errp, EIO,\n> > + \"Channel was not able to read full iov\");\n> \n> Should we report -len instead of EIO here?\n\nNo, QIO methods never return errno values, since channel implementations\nare not guaranteed to be calling commands that return errnos. eg the\nTLS wrapper calls gnutls which has no errno values reported.\n\nIf fact, though, we should just not call error_setg_errno() at all,\nsince the qio_channel_readv method has already populated 'errp'.\n\n> \n> > +int qio_channel_writev_all(QIOChannel *ioc,\n> \n> > + while (nlocal_iov > 0) {\n> > + ssize_t len;\n> > + len = qio_channel_writev(ioc, local_iov, nlocal_iov, errp);\n> > + if (len == QIO_CHANNEL_ERR_BLOCK) {\n> > + qio_channel_wait(ioc, G_IO_OUT);\n> > + continue;\n> > + }\n> > + if (len < 0) {\n> > + error_setg_errno(errp, EIO,\n> > + \"Channel was not able to write full iov\");\n> \n> and again\n> \n> With the typos fixed, and either an explanation why EIO is better or\n> else a fix to preserve errno,\n\nAgain, we should not call error_setg_errno at all, since errp is\nalready populated.\n\n> \n> Reviewed-by: Eric Blake \n\n\n\n\nRegards,\nDaniel","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=)","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=berrange@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 3xjcHv6GlBz9sRW\n\tfor ;\n\tThu, 31 Aug 2017 19:18:51 +1000 (AEST)","from localhost ([::1]:54616 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dnLcf-0002ra-W0\n\tfor incoming@patchwork.ozlabs.org; Thu, 31 Aug 2017 05:18:50 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:53478)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dnLbQ-0002Sz-7l\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 05:17:33 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dnLbM-0007hp-US\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 05:17:32 -0400","from mx1.redhat.com ([209.132.183.28]:51116)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dnLbM-0007hg-LQ\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 05:17:28 -0400","from smtp.corp.redhat.com\n\t(int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12])\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 CB327267D5\n\tfor ; Thu, 31 Aug 2017 09:17:27 +0000 (UTC)","from redhat.com (unknown [10.42.22.189])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 8C513B5156;\n\tThu, 31 Aug 2017 09:17:26 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com CB327267D5","Date":"Thu, 31 Aug 2017 10:17:23 +0100","From":"\"Daniel P. Berrange\" ","To":"Eric Blake ","Message-ID":"<20170831091723.GC17315@redhat.com>","References":"<20170830135611.27678-1-berrange@redhat.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.12","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.30]);\n\tThu, 31 Aug 2017 09:17:27 +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] io: add new qio_channel_{readv, writev,\n\tread, write}_all functions","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","Reply-To":"\"Daniel P. Berrange\" ","Cc":"Paolo Bonzini , qemu-devel@nongnu.org,\n\tJuan Quintela ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]