Re: [Qemu-devel] [PATCH] io: add new qio_channel_{readv, writev, read, write}_all functions

[Eric Blake]

On 08/30/2017 08:56 AM, Daniel P. Berrange wrote:
> These functions wait until they are able to read / write the full
> requested data buffer(s).
> 

Hmm, sounds like the NBD code would benefit from using this in a
followup patch.

> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
> 
> This patch combines these two previous proposals:
> 
>  - https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg01536.html
>  - https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg04041.html
> 
> and switches the test suite over to use the new APIs so we get
> coverage by all the tests/test-io-channel-*  test programs
> 

>  /**
> + * qio_channel_readv_all:
> + * @ioc: the channel object
> + * @iov: the array of memory regions to read data into
> + * @niov: the length of the @iov array
> + * @errp: pointer to a NULL-initialized error object
> + *
> + * Read data from the IO channel, storing it in the
> + * memory regions referenced by @iov. Each element
> + * in the @iov will be fully populated with data
> + * before the next one is used. The @niov parameter
> + * specifies the total number of elements in @iov.
> + *
> + * The function will wait for all requested data
> + * to be read, yielding from the current couroutine

s/couroutine/coroutine/

> + * if required.
> + *
> + * If end-of-file occurrs before all requested data

s/occurrs/occurs/

> + * has been read, an error will be reported.
> + *
> + * Returns: 0 if all bytes were read, or -1 on error
> + */

> +/**
> + * qio_channel_writev_all:
> + * @ioc: the channel object
> + * @iov: the array of memory regions to write data from
> + * @niov: the length of the @iov array
> + * @errp: pointer to a NULL-initialized error object
> + *
> + * Write data to the IO channel, reading it from the
> + * memory regions referenced by @iov. Each element
> + * in the @iov will be fully sent, before the next
> + * one is used. The @niov parameter specifies the
> + * total number of elements in @iov.
> + *
> + * The function will wait for all requested data
> + * to be written, yielding from the current couroutine

s/couroutine/coroutine/

> +++ b/io/channel.c

> +int qio_channel_readv_all(QIOChannel *ioc,

> +    while (nlocal_iov > 0) {
> +        ssize_t len;
> +        len = qio_channel_readv(ioc, local_iov, nlocal_iov, errp);
> +        if (len == QIO_CHANNEL_ERR_BLOCK) {
> +            qio_channel_wait(ioc, G_IO_IN);
> +            continue;
> +        } else if (len < 0) {
> +            error_setg_errno(errp, EIO,
> +                             "Channel was not able to read full iov");

Should we report -len instead of EIO here?

> +int qio_channel_writev_all(QIOChannel *ioc,

> +    while (nlocal_iov > 0) {
> +        ssize_t len;
> +        len = qio_channel_writev(ioc, local_iov, nlocal_iov, errp);
> +        if (len == QIO_CHANNEL_ERR_BLOCK) {
> +            qio_channel_wait(ioc, G_IO_OUT);
> +            continue;
> +        }
> +        if (len < 0) {
> +            error_setg_errno(errp, EIO,
> +                             "Channel was not able to write full iov");

and again

With the typos fixed, and either an explanation why EIO is better or
else a fix to preserve errno,

Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature