[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 07/10] io: add qio_channel_read/write_all
From: |
Daniel P. Berrange |
Subject: |
Re: [Qemu-block] [PATCH 07/10] io: add qio_channel_read/write_all |
Date: |
Wed, 30 Aug 2017 13:52:01 +0100 |
User-agent: |
Mutt/1.8.3 (2017-05-23) |
On Tue, Aug 22, 2017 at 03:18:29PM +0200, 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(-)
This lacks test suite coverage.
Also this is more or less the same as code Juan has proposed too:
https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg01536.html
Rather than have dualing patch series, I'll post an update that
addresses my concerns with both patches, so we can merge these
new APIs independantly of your / Juan's patch series.
>
> 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;
> + }
This busy-loops on EOF ie when n_read == 0.
> +
> + 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
>
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
- Re: [Qemu-block] [Qemu-devel] [PATCH 06/10] scsi, file-posix: add support for persistent reservation management, (continued)