From: Daniel P. Berrangé <berrange@redhat.com>
A recent commit caused the virFDStreamRead method to loop reading data
until the provided buffer is full. Unfortunately the EOF handling was
not quiet correct.
* When seeing a virFDStreamMsg with length zero, it would still
loop trying to read more and then get an error that the thread
has quit.
* When seeing a virFDStreamMsg with length zero on subsequent
iterations, it would discard this message, which would in turn
prevent the caller from ever seeing the 'ret == 0' return value
indicating EOF. The caller would then try to read again and get
an error about the stream being closed.
Fixes: e23fd0b7fd36c41e6db49df4f4962762d3ef6ab0
Reported-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
src/util/virfdstream.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c
index cd2ede4501..9fe70be0b9 100644
--- a/src/util/virfdstream.c
+++ b/src/util/virfdstream.c
@@ -907,6 +907,7 @@ static int virFDStreamRead(virStreamPtr st, char *bytes, size_t nbytes)
virFDStreamMsg *msg = NULL;
size_t got = 0;
size_t bsz = 0;
+ bool isEOF;
more:
while (!(msg = fdst->msg)) {
@@ -945,6 +946,7 @@ static int virFDStreamRead(virStreamPtr st, char *bytes, size_t nbytes)
goto cleanup;
}
+ isEOF = msg->stream.data.len == 0;
bsz = msg->stream.data.len - msg->stream.data.offset;
if (nbytes < bsz)
bsz = nbytes;
@@ -956,12 +958,24 @@ static int virFDStreamRead(virStreamPtr st, char *bytes, size_t nbytes)
nbytes -= bsz;
msg->stream.data.offset += bsz;
- if (msg->stream.data.offset == msg->stream.data.len) {
+ /* If we stream msg is fully consumed, then remove
+ * it from the queue.
+ *
+ * Exception: if this is the second time around the
+ * loop, and the msg indicated an EOF, we must leave
+ * it on the queue so a subsequent read sees the
+ * ret == 0 EOF condition
+ */
+ if (msg->stream.data.offset == msg->stream.data.len &&
+ (!isEOF || got == 0)) {
virFDStreamMsgQueuePop(fdst, fdst->fd, "pipe");
virFDStreamMsgFree(msg);
}
- if (nbytes > 0) {
+ /* If we didn't just see EOF and can read more into
+ * 'bytes' then retry the loop
+ */
+ if (nbytes > 0 && !isEOF) {
goto more;
}
ret = got;
--
2.53.0
On Tue, Feb 24, 2026 at 06:26:04PM +0000, Daniel P. Berrangé via Devel wrote:
> From: Daniel P. Berrangé <berrange@redhat.com>
>
> A recent commit caused the virFDStreamRead method to loop reading data
> until the provided buffer is full. Unfortunately the EOF handling was
> not quiet correct.
>
> * When seeing a virFDStreamMsg with length zero, it would still
> loop trying to read more and then get an error that the thread
> has quit.
>
> * When seeing a virFDStreamMsg with length zero on subsequent
> iterations, it would discard this message, which would in turn
> prevent the caller from ever seeing the 'ret == 0' return value
> indicating EOF. The caller would then try to read again and get
> an error about the stream being closed.
>
> Fixes: e23fd0b7fd36c41e6db49df4f4962762d3ef6ab0
> Reported-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> src/util/virfdstream.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c
> index cd2ede4501..9fe70be0b9 100644
> --- a/src/util/virfdstream.c
> +++ b/src/util/virfdstream.c
> @@ -907,6 +907,7 @@ static int virFDStreamRead(virStreamPtr st, char *bytes, size_t nbytes)
> virFDStreamMsg *msg = NULL;
> size_t got = 0;
> size_t bsz = 0;
> + bool isEOF;
>
> more:
> while (!(msg = fdst->msg)) {
> @@ -945,6 +946,7 @@ static int virFDStreamRead(virStreamPtr st, char *bytes, size_t nbytes)
> goto cleanup;
> }
>
> + isEOF = msg->stream.data.len == 0;
> bsz = msg->stream.data.len - msg->stream.data.offset;
> if (nbytes < bsz)
> bsz = nbytes;
> @@ -956,12 +958,24 @@ static int virFDStreamRead(virStreamPtr st, char *bytes, size_t nbytes)
> nbytes -= bsz;
>
> msg->stream.data.offset += bsz;
> - if (msg->stream.data.offset == msg->stream.data.len) {
> + /* If we stream msg is fully consumed, then remove
s/we/the/
> + * it from the queue.
> + *
> + * Exception: if this is the second time around the
> + * loop, and the msg indicated an EOF, we must leave
> + * it on the queue so a subsequent read sees the
> + * ret == 0 EOF condition
> + */
> + if (msg->stream.data.offset == msg->stream.data.len &&
> + (!isEOF || got == 0)) {
> virFDStreamMsgQueuePop(fdst, fdst->fd, "pipe");
> virFDStreamMsgFree(msg);
> }
>
> - if (nbytes > 0) {
> + /* If we didn't just see EOF and can read more into
> + * 'bytes' then retry the loop
> + */
> + if (nbytes > 0 && !isEOF) {
> goto more;
> }
> ret = got;
> --
> 2.53.0
>
On a Tuesday in 2026, Daniel P. Berrangé via Devel wrote: >From: Daniel P. Berrangé <berrange@redhat.com> > >A recent commit caused the virFDStreamRead method to loop reading data >until the provided buffer is full. Unfortunately the EOF handling was >not quiet correct. > *quite Jano > * When seeing a virFDStreamMsg with length zero, it would still > loop trying to read more and then get an error that the thread > has quit. > > * When seeing a virFDStreamMsg with length zero on subsequent > iterations, it would discard this message, which would in turn > prevent the caller from ever seeing the 'ret == 0' return value > indicating EOF. The caller would then try to read again and get > an error about the stream being closed. > >Fixes: e23fd0b7fd36c41e6db49df4f4962762d3ef6ab0 >Reported-by: Roman Bogorodskiy <bogorodskiy@gmail.com> >Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
© 2016 - 2026 Red Hat, Inc.