[PATCH v2 1/3] io/channel: introduce qio_channel_pread{v, }_all() and preadv_all_eof()

Junjie Cao posted 3 patches 2 weeks, 5 days ago
Maintainers: "Daniel P. Berrangé" <berrange@redhat.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>
[PATCH v2 1/3] io/channel: introduce qio_channel_pread{v, }_all() and preadv_all_eof()
Posted by Junjie Cao 2 weeks, 5 days ago
qio_channel_pread() and qio_channel_preadv() perform a single read
and may return a short result.  Callers that need all bytes currently
have to open-code a retry loop or simply treat a short read as an
error.

Introduce three new helpers following the existing read_all / readv_all
pattern:

  qio_channel_preadv_all_eof()  – retry loop; returns 1 on success,
                                   0 on clean EOF, -1 on error.
  qio_channel_preadv_all()      – wraps _eof; treats early EOF as
                                   error; returns 0 / -1.
  qio_channel_pread_all()       – single-buffer convenience wrapper
                                   around preadv_all().

These advance the file offset internally after each partial read, so
callers only need a single call.  All three are marked
coroutine_mixed_fn, consistent with the existing _all helpers.

Suggested-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Junjie Cao <junjie.cao@intel.com>
---
 include/io/channel.h | 63 ++++++++++++++++++++++++++++++++
 io/channel.c         | 85 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 148 insertions(+)

diff --git a/include/io/channel.h b/include/io/channel.h
index 1b02350437..d5b14c8c77 100644
--- a/include/io/channel.h
+++ b/include/io/channel.h
@@ -634,6 +634,69 @@ ssize_t qio_channel_preadv(QIOChannel *ioc, const struct iovec *iov,
 ssize_t qio_channel_pread(QIOChannel *ioc, void *buf, size_t buflen,
                           off_t offset, Error **errp);
 
+/**
+ * qio_channel_preadv_all_eof:
+ * @ioc: the channel object
+ * @iov: the array of memory regions to read data into
+ * @niov: the length of the @iov array
+ * @offset: the starting offset in the channel to read from
+ * @errp: pointer to a NULL-initialized error object
+ *
+ * Reads data contiguously from the channel into @iov starting at
+ * @offset, retrying on short reads until all requested bytes have
+ * been read.  The offset is advanced internally after each partial
+ * read.
+ *
+ * Returns: 1 if all bytes were read, 0 if no data could be read
+ *          before encountering end-of-file, or -1 on error
+ */
+int coroutine_mixed_fn qio_channel_preadv_all_eof(QIOChannel *ioc,
+                                                  const struct iovec *iov,
+                                                  size_t niov,
+                                                  off_t offset,
+                                                  Error **errp);
+
+/**
+ * qio_channel_preadv_all:
+ * @ioc: the channel object
+ * @iov: the array of memory regions to read data into
+ * @niov: the length of the @iov array
+ * @offset: the starting offset in the channel to read from
+ * @errp: pointer to a NULL-initialized error object
+ *
+ * Reads data contiguously from the channel into @iov starting at
+ * @offset, retrying on short reads until all requested bytes have
+ * been read.  Unlike qio_channel_preadv_all_eof(), end-of-file
+ * before all data has been read is treated as an error.
+ *
+ * Returns: 0 if all bytes were read, or -1 on error
+ */
+int coroutine_mixed_fn qio_channel_preadv_all(QIOChannel *ioc,
+                                              const struct iovec *iov,
+                                              size_t niov,
+                                              off_t offset,
+                                              Error **errp);
+
+/**
+ * qio_channel_pread_all:
+ * @ioc: the channel object
+ * @buf: the memory region to read data into
+ * @buflen: the number of bytes to read into @buf
+ * @offset: the starting offset in the channel to read from
+ * @errp: pointer to a NULL-initialized error object
+ *
+ * Reads @buflen bytes contiguously from the channel at @offset
+ * into @buf, retrying on short reads.  End-of-file before all data
+ * has been read is treated as an error.
+ *
+ * Returns: 0 if all bytes were read, or -1 on error
+ */
+int coroutine_mixed_fn qio_channel_pread_all(QIOChannel *ioc,
+                                             void *buf,
+                                             size_t buflen,
+                                             off_t offset,
+                                             Error **errp);
+
 /**
  * qio_channel_shutdown:
  * @ioc: the channel object
diff --git a/io/channel.c b/io/channel.c
index cc02d997a4..e18c40aa0b 100644
--- a/io/channel.c
+++ b/io/channel.c
@@ -507,6 +507,91 @@ ssize_t qio_channel_pread(QIOChannel *ioc, void *buf, size_t buflen,
     return qio_channel_preadv(ioc, &iov, 1, offset, errp);
 }
 
+int coroutine_mixed_fn qio_channel_preadv_all_eof(QIOChannel *ioc,
+                                                  const struct iovec *iov,
+                                                  size_t niov,
+                                                  off_t offset,
+                                                  Error **errp)
+{
+    int ret = -1;
+    struct iovec *local_iov = g_new(struct iovec, niov);
+    struct iovec *local_iov_head = local_iov;
+    unsigned int nlocal_iov = niov;
+    bool partial = false;
+
+    nlocal_iov = iov_copy(local_iov, nlocal_iov,
+                          iov, niov,
+                          0, iov_size(iov, niov));
+
+    while (nlocal_iov > 0) {
+        ssize_t len;
+        len = qio_channel_preadv(ioc, local_iov, nlocal_iov, offset, errp);
+
+        if (len == QIO_CHANNEL_ERR_BLOCK) {
+            qio_channel_wait_cond(ioc, G_IO_IN);
+            continue;
+        }
+
+        if (len == 0) {
+            if (!partial) {
+                ret = 0;
+                goto cleanup;
+            }
+            error_setg(errp,
+                       "Unexpected end-of-file before all data were read");
+            goto cleanup;
+        }
+
+        if (len < 0) {
+            goto cleanup;
+        }
+
+        partial = true;
+        offset += len;
+        iov_discard_front(&local_iov, &nlocal_iov, len);
+    }
+
+    ret = 1;
+
+ cleanup:
+    g_free(local_iov_head);
+    return ret;
+}
+
+int coroutine_mixed_fn qio_channel_preadv_all(QIOChannel *ioc,
+                                              const struct iovec *iov,
+                                              size_t niov,
+                                              off_t offset,
+                                              Error **errp)
+{
+    int ret = qio_channel_preadv_all_eof(ioc, iov, niov, offset, errp);
+
+    if (ret == 0) {
+        error_setg(errp,
+                   "Unexpected end-of-file before all data were read");
+        return -1;
+    }
+    if (ret == 1) {
+        return 0;
+    }
+
+    return ret;
+}
+
+int coroutine_mixed_fn qio_channel_pread_all(QIOChannel *ioc,
+                                             void *buf,
+                                             size_t buflen,
+                                             off_t offset,
+                                             Error **errp)
+{
+    struct iovec iov = {
+        .iov_base = buf,
+        .iov_len = buflen
+    };
+
+    return qio_channel_preadv_all(ioc, &iov, 1, offset, errp);
+}
+
 int qio_channel_shutdown(QIOChannel *ioc,
                          QIOChannelShutdown how,
                          Error **errp)
-- 
2.43.0


Re: [PATCH v2 1/3] io/channel: introduce qio_channel_pread{v, }_all() and preadv_all_eof()
Posted by Daniel P. Berrangé via qemu development 1 week, 6 days ago
On Wed, Mar 18, 2026 at 10:01:11PM +0800, Junjie Cao wrote:
> qio_channel_pread() and qio_channel_preadv() perform a single read
> and may return a short result.  Callers that need all bytes currently
> have to open-code a retry loop or simply treat a short read as an
> error.
> 
> Introduce three new helpers following the existing read_all / readv_all
> pattern:
> 
>   qio_channel_preadv_all_eof()  – retry loop; returns 1 on success,
>                                    0 on clean EOF, -1 on error.
>   qio_channel_preadv_all()      – wraps _eof; treats early EOF as
>                                    error; returns 0 / -1.
>   qio_channel_pread_all()       – single-buffer convenience wrapper
>                                    around preadv_all().
> 
> These advance the file offset internally after each partial read, so
> callers only need a single call.  All three are marked
> coroutine_mixed_fn, consistent with the existing _all helpers.
> 
> Suggested-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Junjie Cao <junjie.cao@intel.com>
> ---
>  include/io/channel.h | 63 ++++++++++++++++++++++++++++++++
>  io/channel.c         | 85 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 148 insertions(+)

So currently we have:

   qio_channel_read
   qio_channel_read_all
   qio_channel_read_all_eof

   qio_channel_readv
   qio_channel_readv_all
   qio_channel_readv_all_eof

   qio_channel_readv_full
   qio_channel_readv_full_all
   qio_channel_readv_full_all_eof

   qio_channel_pread
   qio_channel_preadv

The obvious gaps in this API design pattern are

   qio_channel_pread_all
   qio_channel_pread_all_eof
   qio_channel_preadv_all
   qio_channel_preadv_all_eof

Your patch is fixing 3 of those 4 gaps. Can you also ensure
it adds qio_channel_pread_all_eof so we complete the set.


On the write side we have

   qio_channel_write
   qio_channel_write_all

   qio_channel_writev
   qio_channel_writev_all

   qio_channel_writev_full
   qio_channel_writev_full_all

   qio_channel_pwrite
   qio_channel_pwritev

Thus we're missing

   qio_channel_pwrite_all
   qio_channel_pwritev_all

Your patch only adds read variants, can you add the corresponding
write variants too.

The write variants can be a separate commit from the read
variants.

> diff --git a/include/io/channel.h b/include/io/channel.h
> index 1b02350437..d5b14c8c77 100644
> --- a/include/io/channel.h
> +++ b/include/io/channel.h
> @@ -634,6 +634,69 @@ ssize_t qio_channel_preadv(QIOChannel *ioc, const struct iovec *iov,
>  ssize_t qio_channel_pread(QIOChannel *ioc, void *buf, size_t buflen,
>                            off_t offset, Error **errp);
>  
> +/**
> + * qio_channel_preadv_all_eof:
> + * @ioc: the channel object
> + * @iov: the array of memory regions to read data into
> + * @niov: the length of the @iov array
> + * @offset: the starting offset in the channel to read from
> + * @errp: pointer to a NULL-initialized error object
> + *
> + * Reads data contiguously from the channel into @iov starting at
> + * @offset, retrying on short reads until all requested bytes have
> + * been read.  The offset is advanced internally after each partial
> + * read.
> + *
> + * Returns: 1 if all bytes were read, 0 if no data could be read
> + *          before encountering end-of-file, or -1 on error

Keep the descriptive text matching the other APIs as closely
as possible. eg

 * Reads @iov, possibly blocking or (if the channel is non-blocking)
 * yielding from the current coroutine multiple times until the entire
 * content is read. If end-of-file occurs immediately it is not an
 * error, but if it occurs after data has been read it will return
 * an error rather than a short-read. Otherwise behaves as
 * qio_channel_preadv().
 *
 * Returns: 1 if all bytes were read, 0 if end-of-file occurs
 *          without data, or -1 on error


And similarly copy the docs for the other APIs as closely
as possible.

> + */
> +int coroutine_mixed_fn qio_channel_preadv_all_eof(QIOChannel *ioc,
> +                                                  const struct iovec *iov,
> +                                                  size_t niov,
> +                                                  off_t offset,
> +                                                  Error **errp);
> +
> +/**
> + * qio_channel_preadv_all:
> + * @ioc: the channel object
> + * @iov: the array of memory regions to read data into
> + * @niov: the length of the @iov array
> + * @offset: the starting offset in the channel to read from
> + * @errp: pointer to a NULL-initialized error object
> + *
> + * Reads data contiguously from the channel into @iov starting at
> + * @offset, retrying on short reads until all requested bytes have
> + * been read.  Unlike qio_channel_preadv_all_eof(), end-of-file
> + * before all data has been read is treated as an error.
> + *
> + * Returns: 0 if all bytes were read, or -1 on error
> + */
> +int coroutine_mixed_fn qio_channel_preadv_all(QIOChannel *ioc,
> +                                              const struct iovec *iov,
> +                                              size_t niov,
> +                                              off_t offset,
> +                                              Error **errp);
> +
> +/**
> + * qio_channel_pread_all:
> + * @ioc: the channel object
> + * @buf: the memory region to read data into
> + * @buflen: the number of bytes to read into @buf
> + * @offset: the starting offset in the channel to read from
> + * @errp: pointer to a NULL-initialized error object
> + *
> + * Reads @buflen bytes contiguously from the channel at @offset
> + * into @buf, retrying on short reads.  End-of-file before all data
> + * has been read is treated as an error.
> + *
> + * Returns: 0 if all bytes were read, or -1 on error
> + */
> +int coroutine_mixed_fn qio_channel_pread_all(QIOChannel *ioc,
> +                                             void *buf,
> +                                             size_t buflen,
> +                                             off_t offset,
> +                                             Error **errp);
> +
>  /**
>   * qio_channel_shutdown:
>   * @ioc: the channel object
> diff --git a/io/channel.c b/io/channel.c
> index cc02d997a4..e18c40aa0b 100644
> --- a/io/channel.c
> +++ b/io/channel.c
> @@ -507,6 +507,91 @@ ssize_t qio_channel_pread(QIOChannel *ioc, void *buf, size_t buflen,
>      return qio_channel_preadv(ioc, &iov, 1, offset, errp);
>  }
>  
> +int coroutine_mixed_fn qio_channel_preadv_all_eof(QIOChannel *ioc,
> +                                                  const struct iovec *iov,
> +                                                  size_t niov,
> +                                                  off_t offset,
> +                                                  Error **errp)
> +{
> +    int ret = -1;
> +    struct iovec *local_iov = g_new(struct iovec, niov);
> +    struct iovec *local_iov_head = local_iov;
> +    unsigned int nlocal_iov = niov;
> +    bool partial = false;
> +
> +    nlocal_iov = iov_copy(local_iov, nlocal_iov,
> +                          iov, niov,
> +                          0, iov_size(iov, niov));
> +
> +    while (nlocal_iov > 0) {
> +        ssize_t len;
> +        len = qio_channel_preadv(ioc, local_iov, nlocal_iov, offset, errp);
> +
> +        if (len == QIO_CHANNEL_ERR_BLOCK) {
> +            qio_channel_wait_cond(ioc, G_IO_IN);
> +            continue;
> +        }
> +
> +        if (len == 0) {
> +            if (!partial) {
> +                ret = 0;
> +                goto cleanup;
> +            }
> +            error_setg(errp,
> +                       "Unexpected end-of-file before all data were read");
> +            goto cleanup;
> +        }
> +
> +        if (len < 0) {
> +            goto cleanup;
> +        }
> +
> +        partial = true;
> +        offset += len;
> +        iov_discard_front(&local_iov, &nlocal_iov, len);
> +    }
> +
> +    ret = 1;
> +
> + cleanup:
> +    g_free(local_iov_head);
> +    return ret;
> +}
> +
> +int coroutine_mixed_fn qio_channel_preadv_all(QIOChannel *ioc,
> +                                              const struct iovec *iov,
> +                                              size_t niov,
> +                                              off_t offset,
> +                                              Error **errp)
> +{
> +    int ret = qio_channel_preadv_all_eof(ioc, iov, niov, offset, errp);
> +
> +    if (ret == 0) {
> +        error_setg(errp,
> +                   "Unexpected end-of-file before all data were read");
> +        return -1;
> +    }
> +    if (ret == 1) {
> +        return 0;
> +    }
> +
> +    return ret;
> +}
> +
> +int coroutine_mixed_fn qio_channel_pread_all(QIOChannel *ioc,
> +                                             void *buf,
> +                                             size_t buflen,
> +                                             off_t offset,
> +                                             Error **errp)
> +{
> +    struct iovec iov = {
> +        .iov_base = buf,
> +        .iov_len = buflen
> +    };
> +
> +    return qio_channel_preadv_all(ioc, &iov, 1, offset, errp);
> +}
> +
>  int qio_channel_shutdown(QIOChannel *ioc,
>                           QIOChannelShutdown how,
>                           Error **errp)
> -- 
> 2.43.0
> 

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|