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
+         * 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

Reply via email to