1. Patchew
  2. Libvirt
  3. View series

[PATCH] fdstream: don't report error on EOF if we read some data

Daniel P. Berrangé via Devel posted 1 patch 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20260223193931.227063-1-berrange@redhat.com
src/util/virfdstream.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] fdstream: don't report error on EOF if we read some data
Posted by Daniel P. Berrangé via Devel 1 week ago 
From: Daniel P. Berrangé <berrange@redhat.com>

Since e23fd0b7fd36c41e6db49df4f4962762d3ef6ab0 the stream read
code will repeatedly try to read a message from the background
thread until it has filled its buffer. When hitting EOF, there
will be no message left on the queue, and if the read buffer
was non-zero length an error will be raised. This should only
be done if there was no data previously read off the stream.

Fixes: e23fd0b7fd36c41e6db49df4f4962762d3ef6ab0
Reported-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/util/virfdstream.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c
index 4c974ba5d7..efda768ef3 100644
--- a/src/util/virfdstream.c
+++ b/src/util/virfdstream.c
@@ -911,7 +911,7 @@ static int virFDStreamRead(virStreamPtr st, char *bytes, size_t nbytes)
     more:
         while (!(msg = fdst->msg)) {
             if (fdst->threadQuit || fdst->threadErr) {
-                if (nbytes) {
+                if (nbytes && !got) {
                     /* virStreamRecv will virResetLastError possibly set
                      * by virFDStreamEvent */
                     if (fdst->threadErr && !virGetLastError())
-- 
2.53.0
Re: [PATCH] fdstream: don't report error on EOF if we read some data
Posted by Roman Bogorodskiy 6 days, 10 hours ago 
  Daniel P. Berrangé wrote:

> From: Daniel P. Berrangé <berrange@redhat.com>
> 
> Since e23fd0b7fd36c41e6db49df4f4962762d3ef6ab0 the stream read
> code will repeatedly try to read a message from the background
> thread until it has filled its buffer. When hitting EOF, there
> will be no message left on the queue, and if the read buffer
> was non-zero length an error will be raised. This should only
> be done if there was no data previously read off the stream.
> 
> Fixes: e23fd0b7fd36c41e6db49df4f4962762d3ef6ab0
> Reported-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  src/util/virfdstream.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c
> index 4c974ba5d7..efda768ef3 100644
> --- a/src/util/virfdstream.c
> +++ b/src/util/virfdstream.c
> @@ -911,7 +911,7 @@ static int virFDStreamRead(virStreamPtr st, char *bytes, size_t nbytes)
>      more:
>          while (!(msg = fdst->msg)) {
>              if (fdst->threadQuit || fdst->threadErr) {
> -                if (nbytes) {
> +                if (nbytes && !got) {
>                      /* virStreamRecv will virResetLastError possibly set
>                       * by virFDStreamEvent */
>                      if (fdst->threadErr && !virGetLastError())


Thanks for looking into that. I can still reproduce the issue with this
change though.

> -- 
> 2.53.0
>
Re: [PATCH] fdstream: don't report error on EOF if we read some data
Posted by Daniel P. Berrangé via Devel 6 days, 6 hours ago 
On Tue, Feb 24, 2026 at 03:22:55PM +0100, Roman Bogorodskiy wrote:
>   Daniel P. Berrangé wrote:
> 
> > From: Daniel P. Berrangé <berrange@redhat.com>
> > 
> > Since e23fd0b7fd36c41e6db49df4f4962762d3ef6ab0 the stream read
> > code will repeatedly try to read a message from the background
> > thread until it has filled its buffer. When hitting EOF, there
> > will be no message left on the queue, and if the read buffer
> > was non-zero length an error will be raised. This should only
> > be done if there was no data previously read off the stream.
> > 
> > Fixes: e23fd0b7fd36c41e6db49df4f4962762d3ef6ab0
> > Reported-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  src/util/virfdstream.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c
> > index 4c974ba5d7..efda768ef3 100644
> > --- a/src/util/virfdstream.c
> > +++ b/src/util/virfdstream.c
> > @@ -911,7 +911,7 @@ static int virFDStreamRead(virStreamPtr st, char *bytes, size_t nbytes)
> >      more:
> >          while (!(msg = fdst->msg)) {
> >              if (fdst->threadQuit || fdst->threadErr) {
> > -                if (nbytes) {
> > +                if (nbytes && !got) {
> >                      /* virStreamRecv will virResetLastError possibly set
> >                       * by virFDStreamEvent */
> >                      if (fdst->threadErr && !virGetLastError())
> 
> 
> Thanks for looking into that. I can still reproduce the issue with this
> change though.

Yep, I've installed FreeBSD 15 and can reproduce that too now.

My diagnosis that we didn't handle EOF correctly was right, but
this patch is fixing it too late. I've sent another patch which
fixes it earlier in the code flow - we need to prevent iterating
around the loop entirely when seeing EOF, rather than iterating
and then attempting to handle 'threadQuit'.


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 :|
Re: [PATCH] fdstream: don't report error on EOF if we read some data
Posted by Michal Prívozník via Devel 6 days, 11 hours ago 
On 2/23/26 20:39, Daniel P. Berrangé via Devel wrote:
> From: Daniel P. Berrangé <berrange@redhat.com>
> 
> Since e23fd0b7fd36c41e6db49df4f4962762d3ef6ab0 the stream read
> code will repeatedly try to read a message from the background
> thread until it has filled its buffer. When hitting EOF, there
> will be no message left on the queue, and if the read buffer
> was non-zero length an error will be raised. This should only
> be done if there was no data previously read off the stream.
> 
> Fixes: e23fd0b7fd36c41e6db49df4f4962762d3ef6ab0
> Reported-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  src/util/virfdstream.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal
Re: [PATCH] fdstream: don't report error on EOF if we read some data
Posted by Erik Huelsmann 1 week ago 
On Mon, Feb 23, 2026 at 8:39 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> From: Daniel P. Berrangé <berrange@redhat.com>
>
> Since e23fd0b7fd36c41e6db49df4f4962762d3ef6ab0 the stream read
> code will repeatedly try to read a message from the background
> thread until it has filled its buffer. When hitting EOF, there
> will be no message left on the queue, and if the read buffer
> was non-zero length an error will be raised. This should only
> be done if there was no data previously read off the stream.
>
> Fixes: e23fd0b7fd36c41e6db49df4f4962762d3ef6ab0
> Reported-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Reviewed-by: Erik Huelsmann <ehuels@gmail.com>

> ---
>  src/util/virfdstream.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c
> index 4c974ba5d7..efda768ef3 100644
> --- a/src/util/virfdstream.c
> +++ b/src/util/virfdstream.c
> @@ -911,7 +911,7 @@ static int virFDStreamRead(virStreamPtr st, char *bytes, size_t nbytes)
>      more:
>          while (!(msg = fdst->msg)) {
>              if (fdst->threadQuit || fdst->threadErr) {
> -                if (nbytes) {
> +                if (nbytes && !got) {
>                      /* virStreamRecv will virResetLastError possibly set
>                       * by virFDStreamEvent */
>                      if (fdst->threadErr && !virGetLastError())
> --
> 2.53.0
>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.