fdstream: fix EOF handling when reading data

A recent commit caused the virFDStreamRead method to loop reading data
until the provided buffer is full. Unfortunately the EOF handling was
not quite 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>
Tested-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c
index cd2ede450166d4c4f5b6ec12844fcd38ad88d31a..f54514a620b7ca7ea8ae6e9aa0af0b7f2439e66a 100644 (file)
--- 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 the 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;