[{"id":1763925,"web_url":"http://patchwork.ozlabs.org/comment/1763925/","msgid":"<20170906085153.GC15510@redhat.com>","list_archive_url":null,"date":"2017-09-06T08:51:53","subject":"Re: [Qemu-devel] [PATCH 2/3] io: Add new qio_channel_read{,\n\tv}_all_eof functions","submitter":{"id":2694,"url":"http://patchwork.ozlabs.org/api/people/2694/","name":"Daniel P. Berrangé","email":"berrange@redhat.com"},"content":"On Tue, Sep 05, 2017 at 02:11:13PM -0500, Eric Blake wrote:\n> Some callers want to distinguish between clean EOF (no bytes read)\n> vs. a short read (at least one byte read, but EOF encountered\n> before reaching the desired length), as it allows clients the\n> ability to do a graceful shutdown when a server shuts down at\n> defined safe points in the protocol, rather than treating all\n> shutdown scenarios as an error due to EOF. However, we don't want\n> to require all callers to have to check for early EOF. So add\n> another wrapper function that can be used by the callers that care\n> about the distinction.\n> \n> Signed-off-by: Eric Blake \n> ---\n> include/io/channel.h | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++\n> io/channel.c | 48 +++++++++++++++++++++++++++++++++++++++--------\n> 2 files changed, 93 insertions(+), 8 deletions(-)\n> \n> diff --git a/include/io/channel.h b/include/io/channel.h\n> index 8f25893c45..3995e243a3 100644\n> --- a/include/io/channel.h\n> +++ b/include/io/channel.h\n> @@ -269,6 +269,36 @@ ssize_t qio_channel_writev_full(QIOChannel *ioc,\n> Error **errp);\n> \n> /**\n> + * qio_channel_readv_all_eof:\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 coroutine\n> + * if required.\n> + *\n> + * If end-of-file occurs before any data is read,\n> + * no error is reported; otherwise, if it occurs\n> + * before all requested data has been read, an error\n> + * will be reported.\n> + *\n> + * Returns: 1 if all bytes were read, 0 if end-of-file\n> + * occurs without data, or -1 on error\n> + */\n> +int qio_channel_readv_all_eof(QIOChannel *ioc,\n> + const struct iovec *iov,\n> + size_t niov,\n> + Error **errp);\n> +\n> +/**\n> * qio_channel_readv_all:\n> * @ioc: the channel object\n> * @iov: the array of memory regions to read data into\n> @@ -383,6 +413,28 @@ ssize_t qio_channel_write(QIOChannel *ioc,\n> Error **errp);\n> \n> /**\n> + * qio_channel_read_all_eof:\n> + * @ioc: the channel object\n> + * @buf: the memory region to read data into\n> + * @buflen: the number of bytes to @buf\n> + * @errp: pointer to a NULL-initialized error object\n> + *\n> + * Reads @buflen bytes into @buf, possibly blocking or (if the\n> + * channel is non-blocking) yielding from the current coroutine\n> + * multiple times until the entire content is read. If end-of-file\n> + * occurs immediately it is not an error, but if it occurs after\n> + * data has been read it will return an error rather than a\n> + * short-read. Otherwise behaves as qio_channel_read().\n> + *\n> + * Returns: 1 if all bytes were read, 0 if end-of-file occurs\n> + * without data, or -1 on error\n> + */\n> +int qio_channel_read_all_eof(QIOChannel *ioc,\n> + char *buf,\n> + size_t buflen,\n> + Error **errp);\n> +\n> +/**\n> * qio_channel_read_all:\n> * @ioc: the channel object\n> * @buf: the memory region to read data into\n> @@ -401,6 +453,7 @@ int qio_channel_read_all(QIOChannel *ioc,\n> char *buf,\n> size_t buflen,\n> Error **errp);\n> +\n> /**\n> * qio_channel_write_all:\n> * @ioc: the channel object\n> diff --git a/io/channel.c b/io/channel.c\n> index 9e62794cab..ec4b86de7c 100644\n> --- a/io/channel.c\n> +++ b/io/channel.c\n> @@ -86,16 +86,16 @@ ssize_t qio_channel_writev_full(QIOChannel *ioc,\n> }\n> \n> \n> -\n> -int qio_channel_readv_all(QIOChannel *ioc,\n> - const struct iovec *iov,\n> - size_t niov,\n> - Error **errp)\n> +int qio_channel_readv_all_eof(QIOChannel *ioc,\n> + const struct iovec *iov,\n> + size_t niov,\n> + Error **errp)\n> {\n> int ret = -1;\n> struct iovec *local_iov = g_new(struct iovec, niov);\n> struct iovec *local_iov_head = local_iov;\n> unsigned int nlocal_iov = niov;\n> + bool partial = false;\n> \n> nlocal_iov = iov_copy(local_iov, nlocal_iov,\n> iov, niov,\n> @@ -114,21 +114,43 @@ int qio_channel_readv_all(QIOChannel *ioc,\n> } else if (len < 0) {\n> goto cleanup;\n> } else if (len == 0) {\n> - error_setg(errp,\n> - \"Unexpected end-of-file before all bytes were read\");\n> + if (partial) {\n> + error_setg(errp,\n> + \"Unexpected end-of-file before all bytes were read\");\n> + } else {\n> + ret = 0;\n> + }\n> goto cleanup;\n> }\n> \n> + partial = true;\n> iov_discard_front(&local_iov, &nlocal_iov, len);\n> }\n> \n> - ret = 0;\n> + ret = 1;\n> \n> cleanup:\n> g_free(local_iov_head);\n> return ret;\n> }\n> \n> +int qio_channel_readv_all(QIOChannel *ioc,\n> + const struct iovec *iov,\n> + size_t niov,\n> + Error **errp)\n> +{\n> + int ret = qio_channel_readv_all_eof(ioc, iov, niov, errp);\n> +\n> + if (ret == 0) {\n> + ret = -1;\n> + error_setg(errp,\n> + \"Unexpected end-of-file before all bytes were read\");\n> + } else if (ret == 1) {\n> + ret = 0;\n> + }\n> + return ret;\n> +}\n> +\n> int qio_channel_writev_all(QIOChannel *ioc,\n> const struct iovec *iov,\n> size_t niov,\n> @@ -205,6 +227,16 @@ ssize_t qio_channel_write(QIOChannel *ioc,\n> }\n> \n> \n> +int qio_channel_read_all_eof(QIOChannel *ioc,\n> + char *buf,\n> + size_t buflen,\n> + Error **errp)\n> +{\n> + struct iovec iov = { .iov_base = buf, .iov_len = buflen };\n> + return qio_channel_readv_all_eof(ioc, &iov, 1, errp);\n> +}\n> +\n> +\n> int qio_channel_read_all(QIOChannel *ioc,\n> char *buf,\n> size_t buflen,\n\nAcked-by: Daniel P. Berrange \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 3xnHQx183fz9sBZ\n\tfor ;\n\tWed, 6 Sep 2017 18:52:41 +1000 (AEST)","from localhost ([::1]:35022 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dpW4d-00075T-B2\n\tfor incoming@patchwork.ozlabs.org; Wed, 06 Sep 2017 04:52:39 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:55174)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dpW43-00070t-Ff\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 04:52:08 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dpW42-0005Ly-3C\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 04:52:03 -0400","from mx1.redhat.com ([209.132.183.28]:51886)\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 1dpW3y-0005KC-GF; Wed, 06 Sep 2017 04:51:58 -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 5B472356C8;\n\tWed, 6 Sep 2017 08:51:57 +0000 (UTC)","from redhat.com (unknown [10.42.22.189])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 085F2627D7;\n\tWed, 6 Sep 2017 08:51:55 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 5B472356C8","Date":"Wed, 6 Sep 2017 09:51:53 +0100","From":"\"Daniel P. Berrange\" ","To":"Eric Blake ","Message-ID":"<20170906085153.GC15510@redhat.com>","References":"<20170905191114.5959-1-eblake@redhat.com>\n\t<20170905191114.5959-3-eblake@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20170905191114.5959-3-eblake@redhat.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","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, 06 Sep 2017 08:51:57 +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 2/3] io: Add new qio_channel_read{,\n\tv}_all_eof 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":"pbonzini@redhat.com, qemu-devel@nongnu.org, qemu-block@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]