Re: [Qemu-devel] [PATCH 07/10] io: add qio_channel_read/write_all

[Paolo Bonzini]

On 23/08/2017 07:08, Fam Zheng wrote:
> On Tue, 08/22 15:18, Paolo Bonzini wrote:
>> It is pretty common to read a fixed-size buffer from a socket.  Add a
>> function that does this, either with multiple reads (on blocking sockets)
>> or by yielding if called from a coroutine.
>>
>> Cc: Daniel P. Berrange <address@hidden>
>> Signed-off-by: Paolo Bonzini <address@hidden>
>> ---
>>  include/io/channel.h | 36 ++++++++++++++++++++++++++++++++++-
>>  io/channel.c         | 54 
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 89 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/io/channel.h b/include/io/channel.h
>> index db9bb022a1..9cfb4d081f 100644
>> --- a/include/io/channel.h
>> +++ b/include/io/channel.h
>> @@ -299,7 +299,7 @@ ssize_t qio_channel_writev(QIOChannel *ioc,
>>                             Error **errp);
>>  
>>  /**
>> - * qio_channel_readv:
>> + * qio_channel_read:
>>   * @ioc: the channel object
>>   * @buf: the memory region to read data into
>>   * @buflen: the length of @buf
>> @@ -315,6 +315,23 @@ ssize_t qio_channel_read(QIOChannel *ioc,
>>                           Error **errp);
>>  
>>  /**
>> + * qio_channel_read_all:
>> + * @ioc: the channel object
>> + * @buf: the memory region to read data into
>> + * @buflen: the number of bytes to @buf
>> + * @errp: pointer to a NULL-initialized error object
>> + *
>> + * Reads @buflen bytes into @buf, possibly blocking or (if the
>> + * channel is non-blocking) yielding from the current coroutine
>> + * multiple times until the entire content is read.  Otherwise
>> + * behaves as qio_channel_read().
>> + */
>> +ssize_t coroutine_fn qio_channel_read_all(QIOChannel *ioc,
>> +                                          char *buf,
>> +                                          size_t buflen,
>> +                                          Error **errp);
>> +
>> +/**
>>   * qio_channel_write:
>>   * @ioc: the channel object
>>   * @buf: the memory regions to send data from
>> @@ -331,6 +348,23 @@ ssize_t qio_channel_write(QIOChannel *ioc,
>>                            Error **errp);
>>  
>>  /**
>> + * qio_channel_write_all:
>> + * @ioc: the channel object
>> + * @buf: the memory region to write data into
>> + * @buflen: the number of bytes to @buf
>> + * @errp: pointer to a NULL-initialized error object
>> + *
>> + * Writes @buflen bytes from @buf, possibly blocking or (if the
>> + * channel is non-blocking) yielding from the current coroutine
>> + * multiple times until the entire content is written.  Otherwise
>> + * behaves as qio_channel_write().
>> + */
>> +ssize_t coroutine_fn qio_channel_write_all(QIOChannel *ioc,
>> +                                           const char *buf,
>> +                                           size_t buflen,
>> +                                           Error **errp);
>> +
>> +/**
>>   * qio_channel_set_blocking:
>>   * @ioc: the channel object
>>   * @enabled: the blocking flag state
>> diff --git a/io/channel.c b/io/channel.c
>> index 1cfb8b33a2..7ab3f4eede 100644
>> --- a/io/channel.c
>> +++ b/io/channel.c
>> @@ -113,6 +113,60 @@ ssize_t qio_channel_read(QIOChannel *ioc,
>>  }
>>  
>>  
>> +ssize_t qio_channel_read_all(QIOChannel *ioc,
>> +                             char *buf,
>> +                             size_t buflen,
>> +                             Error **errp)
>> +{
>> +    ssize_t total = 0;
>> +    while (buflen > 0) {
>> +        ssize_t n_read = qio_channel_read(ioc, buf, buflen, errp);
>> +
>> +        if (n_read == QIO_CHANNEL_ERR_BLOCK) {
>> +            assert(ioc->ctx);
>> +            qio_channel_yield(ioc, G_IO_IN);
>> +            continue;
>> +        }
>> +        if (n_read < 0) {
>> +            return n_read;
>> +        }
>> +
>> +        buf += n_read;
>> +        total += n_read;
>> +        buflen -= n_read;
>> +    }
>> +
>> +    return total;
>> +}
>> +
>> +
>> +ssize_t qio_channel_write_all(QIOChannel *ioc,
>> +                              const char *buf,
>> +                              size_t buflen,
>> +                              Error **errp)
>> +{
>> +    ssize_t total = 0;
>> +    while (buflen > 0) {
>> +        ssize_t n_written = qio_channel_write(ioc, buf, buflen, errp);
>> +
>> +        if (n_written == QIO_CHANNEL_ERR_BLOCK) {
>> +            assert(ioc->ctx);
>> +            qio_channel_yield(ioc, G_IO_OUT);
>> +            continue;
>> +        }
>> +        if (n_written < 0) {
>> +            return n_written;
>> +        }
>> +
>> +        buf += n_written;
>> +        total += n_written;
>> +        buflen -= n_written;
>> +    }
>> +
>> +    return total;
>> +}
>> +
>> +
>>  ssize_t qio_channel_write(QIOChannel *ioc,
>>                            const char *buf,
>>                            size_t buflen,
>> -- 
>> 2.13.5
> 
> Are there any caller depending on short read/write? Is it okay to change all 
> (or
> most) qio_channel_{read,write}* functions to this "full read/write" behavior?

No, it looks like I'm the first. :)  NBD also implements something
similar to this patch, but it uses qio_channel_{read,write}v.

Paolo