On Tue, Feb 24, 2026 at 06:26:04PM +0000, Daniel P. Berrangé via Devel wrote: > From: Daniel P. Berrangé <[email protected]> > > 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 <[email protected]> > Signed-off-by: Daniel P. Berrangé <[email protected]> > --- > 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
>
signature.asc
Description: PGP signature
