[PATCH] fdstream: don't report error on EOF if we read some data
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
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
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
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
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 :|
participants (4)
-
Daniel P. Berrangé -
Erik Huelsmann -
Michal Prívozník -
Roman Bogorodskiy