* [PATCH v2 1/3] io/channel: introduce qio_channel_pread{v, }_all() and preadv_all_eof()
2026-03-18 14:01 ` [PATCH v2 0/3] " Junjie Cao
@ 2026-03-18 14:01 ` Junjie Cao
2026-03-24 10:51 ` Daniel P. Berrangé via qemu development
2026-03-18 14:01 ` [PATCH v2 2/3] migration/file: fix type mismatch and NULL deref in multifd_file_recv_data Junjie Cao
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Junjie Cao @ 2026-03-18 14:01 UTC (permalink / raw)
To: qemu-devel; +Cc: peterx, farosas, berrange, junjie.cao
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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 1/3] io/channel: introduce qio_channel_pread{v, }_all() and preadv_all_eof()
2026-03-18 14:01 ` [PATCH v2 1/3] io/channel: introduce qio_channel_pread{v, }_all() and preadv_all_eof() Junjie Cao
@ 2026-03-24 10:51 ` Daniel P. Berrangé via qemu development
0 siblings, 0 replies; 12+ messages in thread
From: Daniel P. Berrangé via qemu development @ 2026-03-24 10:51 UTC (permalink / raw)
To: Junjie Cao; +Cc: qemu-devel, peterx, farosas
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 :|
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 2/3] migration/file: fix type mismatch and NULL deref in multifd_file_recv_data
2026-03-18 14:01 ` [PATCH v2 0/3] " Junjie Cao
2026-03-18 14:01 ` [PATCH v2 1/3] io/channel: introduce qio_channel_pread{v, }_all() and preadv_all_eof() Junjie Cao
@ 2026-03-18 14:01 ` Junjie Cao
2026-03-24 10:53 ` Daniel P. Berrangé
2026-03-18 14:01 ` [PATCH v2 3/3] tests/unit: add pread_all and preadv_all tests for io channel file Junjie Cao
2026-03-24 8:27 ` [PATCH v2 0/3] migration/file: fix type mismatch and NULL deref in multifd_file_recv_data Junjie Cao
3 siblings, 1 reply; 12+ messages in thread
From: Junjie Cao @ 2026-03-18 14:01 UTC (permalink / raw)
To: qemu-devel; +Cc: peterx, farosas, berrange, junjie.cao
multifd_file_recv_data() stores the return value of qio_channel_pread()
(ssize_t) in a size_t variable. On I/O error the -1 return value wraps
to SIZE_MAX, producing a nonsensical read size in the error message.
More critically, a short read (0 <= ret < data->size) is possible when
the migration file is truncated. In that case qio_channel_pread()
returns a non-negative value without setting *errp. The function then
calls error_prepend(errp, ...) which dereferences *errp -- a NULL
pointer -- crashing QEMU.
Fix both issues by switching to qio_channel_pread_all() introduced in
the previous commit, which retries on short reads and treats
end-of-file as an error, so the caller no longer needs to check the
byte count manually. Add ERRP_GUARD() so that error_prepend() works
correctly even when errp is &error_fatal or NULL.
Suggested-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Junjie Cao <junjie.cao@intel.com>
---
migration/file.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/migration/file.c b/migration/file.c
index 5618aced49..b0d3affa4c 100644
--- a/migration/file.c
+++ b/migration/file.c
@@ -254,16 +254,15 @@ int file_write_ramblock_iov(QIOChannel *ioc, const struct iovec *iov,
int multifd_file_recv_data(MultiFDRecvParams *p, Error **errp)
{
+ ERRP_GUARD();
MultiFDRecvData *data = p->data;
- size_t ret;
-
- ret = qio_channel_pread(p->c, (char *) data->opaque,
- data->size, data->file_offset, errp);
- if (ret != data->size) {
- error_prepend(errp,
- "multifd recv (%u): read 0x%zx, expected 0x%zx",
- p->id, ret, data->size);
- return -1;
+ int ret;
+
+ ret = qio_channel_pread_all(p->c, (char *) data->opaque,
+ data->size, data->file_offset, errp);
+ if (ret < 0) {
+ error_prepend(errp, "multifd recv (%u): ", p->id);
+ return ret;
}
return 0;
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 2/3] migration/file: fix type mismatch and NULL deref in multifd_file_recv_data
2026-03-18 14:01 ` [PATCH v2 2/3] migration/file: fix type mismatch and NULL deref in multifd_file_recv_data Junjie Cao
@ 2026-03-24 10:53 ` Daniel P. Berrangé
0 siblings, 0 replies; 12+ messages in thread
From: Daniel P. Berrangé @ 2026-03-24 10:53 UTC (permalink / raw)
To: Junjie Cao; +Cc: qemu-devel, peterx, farosas
On Wed, Mar 18, 2026 at 10:01:12PM +0800, Junjie Cao wrote:
> multifd_file_recv_data() stores the return value of qio_channel_pread()
> (ssize_t) in a size_t variable. On I/O error the -1 return value wraps
> to SIZE_MAX, producing a nonsensical read size in the error message.
>
> More critically, a short read (0 <= ret < data->size) is possible when
> the migration file is truncated. In that case qio_channel_pread()
> returns a non-negative value without setting *errp. The function then
> calls error_prepend(errp, ...) which dereferences *errp -- a NULL
> pointer -- crashing QEMU.
>
> Fix both issues by switching to qio_channel_pread_all() introduced in
> the previous commit, which retries on short reads and treats
> end-of-file as an error, so the caller no longer needs to check the
> byte count manually. Add ERRP_GUARD() so that error_prepend() works
> correctly even when errp is &error_fatal or NULL.
>
> Suggested-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Junjie Cao <junjie.cao@intel.com>
> ---
> migration/file.c | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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 :|
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 3/3] tests/unit: add pread_all and preadv_all tests for io channel file
2026-03-18 14:01 ` [PATCH v2 0/3] " Junjie Cao
2026-03-18 14:01 ` [PATCH v2 1/3] io/channel: introduce qio_channel_pread{v, }_all() and preadv_all_eof() Junjie Cao
2026-03-18 14:01 ` [PATCH v2 2/3] migration/file: fix type mismatch and NULL deref in multifd_file_recv_data Junjie Cao
@ 2026-03-18 14:01 ` Junjie Cao
2026-03-24 8:27 ` [PATCH v2 0/3] migration/file: fix type mismatch and NULL deref in multifd_file_recv_data Junjie Cao
3 siblings, 0 replies; 12+ messages in thread
From: Junjie Cao @ 2026-03-18 14:01 UTC (permalink / raw)
To: qemu-devel; +Cc: peterx, farosas, berrange, junjie.cao
Add five targeted test cases for the new qio_channel_pread{v,}_all()
and preadv_all_eof() helpers introduced earlier in this series:
preadv_all: read from a non-zero offset into two iovecs
pread_all: single-buffer wrapper smoke test
preadv_all_eof/clean: clean EOF (offset == file length) returns 0
preadv_all_eof/partial: partial data then EOF returns -1
preadv_all/eof-is-error: strict wrapper turns clean EOF into -1
All tests create a 16-byte file with known content and exercise the
read helpers against it, verifying return values, error state, and
(where applicable) buffer contents. The tests are guarded by
CONFIG_PREADV since the underlying preadv support is optional.
Signed-off-by: Junjie Cao <junjie.cao@intel.com>
---
tests/unit/test-io-channel-file.c | 137 ++++++++++++++++++++++++++++++
1 file changed, 137 insertions(+)
diff --git a/tests/unit/test-io-channel-file.c b/tests/unit/test-io-channel-file.c
index 1977006ce9..97a2a5b5cb 100644
--- a/tests/unit/test-io-channel-file.c
+++ b/tests/unit/test-io-channel-file.c
@@ -102,6 +102,131 @@ static void test_io_channel_fd(void)
}
+#ifdef CONFIG_PREADV
+#define TEST_PREAD_FILE "tests/test-io-channel-pread.txt"
+#define TEST_PREAD_PATTERN "ABCDEFGHIJKLMNOP" /* 16 bytes */
+#define TEST_PREAD_LEN 16
+
+static QIOChannel *create_pread_test_file(void)
+{
+ int fd;
+
+ fd = open(TEST_PREAD_FILE, O_CREAT | O_TRUNC | O_WRONLY, 0600);
+ g_assert_cmpint(fd, >=, 0);
+ g_assert_cmpint(write(fd, TEST_PREAD_PATTERN, TEST_PREAD_LEN),
+ ==, TEST_PREAD_LEN);
+ close(fd);
+
+ return QIO_CHANNEL(qio_channel_file_new_path(
+ TEST_PREAD_FILE, O_RDONLY, 0,
+ &error_abort));
+}
+
+static void test_io_channel_preadv_all(void)
+{
+ QIOChannel *ioc;
+ char buf1[4], buf2[4];
+ struct iovec iov[2] = {
+ { .iov_base = buf1, .iov_len = sizeof(buf1) },
+ { .iov_base = buf2, .iov_len = sizeof(buf2) },
+ };
+ int ret;
+
+ ioc = create_pread_test_file();
+
+ /* Read 8 bytes from offset 4 into two iovecs: "EFGH" + "IJKL" */
+ ret = qio_channel_preadv_all(ioc, iov, 2, 4, &error_abort);
+ g_assert_cmpint(ret, ==, 0);
+ g_assert_cmpmem(buf1, 4, "EFGH", 4);
+ g_assert_cmpmem(buf2, 4, "IJKL", 4);
+
+ unlink(TEST_PREAD_FILE);
+ object_unref(OBJECT(ioc));
+}
+
+static void test_io_channel_pread_all(void)
+{
+ QIOChannel *ioc;
+ char buf[8];
+ int ret;
+
+ ioc = create_pread_test_file();
+
+ /* Read 8 bytes from offset 8: "IJKLMNOP" */
+ ret = qio_channel_pread_all(ioc, buf, sizeof(buf), 8, &error_abort);
+ g_assert_cmpint(ret, ==, 0);
+ g_assert_cmpmem(buf, 8, "IJKLMNOP", 8);
+
+ unlink(TEST_PREAD_FILE);
+ object_unref(OBJECT(ioc));
+}
+
+static void test_io_channel_preadv_all_eof_clean(void)
+{
+ QIOChannel *ioc;
+ Error *err = NULL;
+ char buf[8];
+ struct iovec iov = { .iov_base = buf, .iov_len = sizeof(buf) };
+ int ret;
+
+ ioc = create_pread_test_file();
+
+ /* Read from offset == file length: clean EOF, expect 0 and no error */
+ ret = qio_channel_preadv_all_eof(ioc, &iov, 1, TEST_PREAD_LEN, &err);
+ g_assert_cmpint(ret, ==, 0);
+ g_assert_null(err);
+
+ unlink(TEST_PREAD_FILE);
+ object_unref(OBJECT(ioc));
+}
+
+static void test_io_channel_preadv_all_eof_partial(void)
+{
+ QIOChannel *ioc;
+ Error *err = NULL;
+ char buf[8];
+ struct iovec iov = { .iov_base = buf, .iov_len = sizeof(buf) };
+ int ret;
+
+ ioc = create_pread_test_file();
+
+ /*
+ * Read 8 bytes from offset 12: only 4 bytes available before EOF.
+ * Expect -1 (partial data then EOF is an error) and err set.
+ */
+ ret = qio_channel_preadv_all_eof(ioc, &iov, 1, 12, &err);
+ g_assert_cmpint(ret, ==, -1);
+ g_assert_nonnull(err);
+ error_free(err);
+
+ unlink(TEST_PREAD_FILE);
+ object_unref(OBJECT(ioc));
+}
+
+static void test_io_channel_preadv_all_eof_is_error(void)
+{
+ QIOChannel *ioc;
+ Error *err = NULL;
+ char buf[8];
+ struct iovec iov = { .iov_base = buf, .iov_len = sizeof(buf) };
+ int ret;
+
+ ioc = create_pread_test_file();
+
+ /*
+ * Clean EOF through the strict wrapper: should be translated to -1.
+ */
+ ret = qio_channel_preadv_all(ioc, &iov, 1, TEST_PREAD_LEN, &err);
+ g_assert_cmpint(ret, ==, -1);
+ g_assert_nonnull(err);
+ error_free(err);
+
+ unlink(TEST_PREAD_FILE);
+ object_unref(OBJECT(ioc));
+}
+#endif /* CONFIG_PREADV */
+
+
#ifndef _WIN32
static void test_io_channel_pipe(bool async)
{
@@ -147,6 +272,18 @@ int main(int argc, char **argv)
g_test_add_func("/io/channel/file", test_io_channel_file);
g_test_add_func("/io/channel/file/rdwr", test_io_channel_file_rdwr);
g_test_add_func("/io/channel/file/fd", test_io_channel_fd);
+#ifdef CONFIG_PREADV
+ g_test_add_func("/io/channel/file/preadv-all",
+ test_io_channel_preadv_all);
+ g_test_add_func("/io/channel/file/pread-all",
+ test_io_channel_pread_all);
+ g_test_add_func("/io/channel/file/preadv-all-eof/clean",
+ test_io_channel_preadv_all_eof_clean);
+ g_test_add_func("/io/channel/file/preadv-all-eof/partial",
+ test_io_channel_preadv_all_eof_partial);
+ g_test_add_func("/io/channel/file/preadv-all/eof-is-error",
+ test_io_channel_preadv_all_eof_is_error);
+#endif
#ifndef _WIN32
g_test_add_func("/io/channel/pipe/sync", test_io_channel_pipe_sync);
g_test_add_func("/io/channel/pipe/async", test_io_channel_pipe_async);
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 0/3] migration/file: fix type mismatch and NULL deref in multifd_file_recv_data
2026-03-18 14:01 ` [PATCH v2 0/3] " Junjie Cao
` (2 preceding siblings ...)
2026-03-18 14:01 ` [PATCH v2 3/3] tests/unit: add pread_all and preadv_all tests for io channel file Junjie Cao
@ 2026-03-24 8:27 ` Junjie Cao
2026-03-24 10:54 ` Daniel P. Berrangé
3 siblings, 1 reply; 12+ messages in thread
From: Junjie Cao @ 2026-03-24 8:27 UTC (permalink / raw)
To: qemu-devel; +Cc: peterx, berrange, farosas, junjie.cao
Hi Peter, Daniel,
I wanted to follow up on this series. Looking back, I went
straight to adding new public API in io/channel. I should
have confirmed my approach and whether the Suggested-by
tag was appropriate here with you before sending. Sorry for that.
I see two possible paths and would appreciate your guidance:
(a) Keep it purely a fix in migration/file.c: add a local
retry loop around qio_channel_pread() in
multifd_file_recv_data(), fix the ssize_t/size_t
mismatch, and use ERRP_GUARD() so error_prepend()
is safe.
(b) If pread_all() retry helpers are worth having as a
general facility in io/channel, I'm happy to propose
them as a separate RFC so we can discuss the API design
on its own merits.
I'm leaning toward (a) and can send a v3 quickly.
Does that sound right?
Thanks,
Junjie
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v2 0/3] migration/file: fix type mismatch and NULL deref in multifd_file_recv_data
2026-03-24 8:27 ` [PATCH v2 0/3] migration/file: fix type mismatch and NULL deref in multifd_file_recv_data Junjie Cao
@ 2026-03-24 10:54 ` Daniel P. Berrangé
2026-03-26 15:21 ` Peter Xu
0 siblings, 1 reply; 12+ messages in thread
From: Daniel P. Berrangé @ 2026-03-24 10:54 UTC (permalink / raw)
To: Junjie Cao; +Cc: qemu-devel, peterx, farosas
On Tue, Mar 24, 2026 at 04:27:38PM +0800, Junjie Cao wrote:
> Hi Peter, Daniel,
>
> I wanted to follow up on this series. Looking back, I went
> straight to adding new public API in io/channel. I should
> have confirmed my approach and whether the Suggested-by
> tag was appropriate here with you before sending. Sorry for that.
>
> I see two possible paths and would appreciate your guidance:
>
> (a) Keep it purely a fix in migration/file.c: add a local
> retry loop around qio_channel_pread() in
> multifd_file_recv_data(), fix the ssize_t/size_t
> mismatch, and use ERRP_GUARD() so error_prepend()
> is safe.
>
> (b) If pread_all() retry helpers are worth having as a
> general facility in io/channel, I'm happy to propose
> them as a separate RFC so we can discuss the API design
> on its own merits.
>
> I'm leaning toward (a) and can send a v3 quickly.
> Does that sound right?
Doing (b) is the right approach. We want these helpers in the
general code, so we don't create technical debt that has to be
fixed next time something else wants the same APIs.
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 :|
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/3] migration/file: fix type mismatch and NULL deref in multifd_file_recv_data
2026-03-24 10:54 ` Daniel P. Berrangé
@ 2026-03-26 15:21 ` Peter Xu
0 siblings, 0 replies; 12+ messages in thread
From: Peter Xu @ 2026-03-26 15:21 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Junjie Cao, qemu-devel, farosas
On Tue, Mar 24, 2026 at 10:54:38AM +0000, Daniel P. Berrangé wrote:
> On Tue, Mar 24, 2026 at 04:27:38PM +0800, Junjie Cao wrote:
> > Hi Peter, Daniel,
> >
> > I wanted to follow up on this series. Looking back, I went
> > straight to adding new public API in io/channel. I should
> > have confirmed my approach and whether the Suggested-by
> > tag was appropriate here with you before sending. Sorry for that.
> >
> > I see two possible paths and would appreciate your guidance:
> >
> > (a) Keep it purely a fix in migration/file.c: add a local
> > retry loop around qio_channel_pread() in
> > multifd_file_recv_data(), fix the ssize_t/size_t
> > mismatch, and use ERRP_GUARD() so error_prepend()
> > is safe.
> >
> > (b) If pread_all() retry helpers are worth having as a
> > general facility in io/channel, I'm happy to propose
> > them as a separate RFC so we can discuss the API design
> > on its own merits.
> >
> > I'm leaning toward (a) and can send a v3 quickly.
> > Does that sound right?
>
> Doing (b) is the right approach. We want these helpers in the
> general code, so we don't create technical debt that has to be
> fixed next time something else wants the same APIs.
Sorry for a late response, Junjie. I agree with Dan.
--
Peter Xu
^ permalink raw reply [flat|nested] 12+ messages in thread