[Qemu-devel] [PATCH 1/1] Make qemu_peek_buffer loop until it gets it's data

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <address@hidden>

Make qemu_peek_buffer repatedly call fill_buffer until it gets
all the data it requires, or until there is an error.

  At the moment, qemu_peek_buffer will try one qemu_fill_buffer if there
  isn't enough data waiting, however the kernel is entitled to return
  just a few bytes, and still leave qemu_peek_buffer with less bytes
  than it needed.  I've seen this fail in a dev world, and I think it
  could theoretically fail in the peeking of the subsection headers in
  the current world.

Ditto for qemu_peek_byte (which can only be affected due to it's
offset).

Simplify qemu_get_buffer since it can now rely on qemu_peek_buffer to
loop.

Use size_t rather than int for size parameters, (and result for
those functions that never return -errno).

Fail vmstate_subsection_load if idstr couldn't be peek'd

Signed-off-by: Dr. David Alan Gilbert <address@hidden>
---
 include/migration/qemu-file.h |  6 ++--
 qemu-file.c                   | 64 +++++++++++++++++++++++++++----------------
 vmstate.c                     |  2 +-
 3 files changed, 44 insertions(+), 28 deletions(-)

diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index a191fb6..cc45c87 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -121,11 +121,11 @@ static inline void qemu_put_ubyte(QEMUFile *f, unsigned 
int v)
 void qemu_put_be16(QEMUFile *f, unsigned int v);
 void qemu_put_be32(QEMUFile *f, unsigned int v);
 void qemu_put_be64(QEMUFile *f, uint64_t v);
-int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset);
-int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size);
+size_t qemu_peek_buffer(QEMUFile *f, uint8_t *buf, size_t size, size_t offset);
+size_t qemu_get_buffer(QEMUFile *f, uint8_t *buf, size_t size);
 int qemu_peek_byte(QEMUFile *f, int offset);
 int qemu_get_byte(QEMUFile *f);
-void qemu_file_skip(QEMUFile *f, int size);
+void qemu_file_skip(QEMUFile *f, size_t size);
 void qemu_update_position(QEMUFile *f, size_t size);
 
 static inline unsigned int qemu_get_ubyte(QEMUFile *f)
diff --git a/qemu-file.c b/qemu-file.c
index e5ec798..eb388a7 100644
--- a/qemu-file.c
+++ b/qemu-file.c
@@ -529,7 +529,11 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t 
block_offset,
     return RAM_SAVE_CONTROL_NOT_SUPP;
 }
 
-static void qemu_fill_buffer(QEMUFile *f)
+/*
+ * Attempt to fill the buffer from the underlying file
+ * Returns the number of bytes read, or -ve value for an error.
+ */
+static int qemu_fill_buffer(QEMUFile *f)
 {
     int len;
     int pending;
@@ -553,6 +557,8 @@ static void qemu_fill_buffer(QEMUFile *f)
     } else if (len != -EAGAIN) {
         qemu_file_set_error(f, len);
     }
+
+    return len;
 }
 
 int qemu_get_fd(QEMUFile *f)
@@ -676,24 +682,41 @@ void qemu_put_byte(QEMUFile *f, int v)
     }
 }
 
-void qemu_file_skip(QEMUFile *f, int size)
+void qemu_file_skip(QEMUFile *f, size_t size)
 {
     if (f->buf_index + size <= f->buf_size) {
         f->buf_index += size;
     }
 }
 
-int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset)
+/*
+ * Read 'size' bytes from file (at 'offset') into buf without moving the
+ * pointer.
+ *
+ * If the underlying fd blocks, then it will return size bytes
+ * unless there was an error, in which case it will return as many as it
+ * managed to read.
+ */
+size_t qemu_peek_buffer(QEMUFile *f, uint8_t *buf, size_t size, size_t offset)
 {
     int pending;
     int index;
 
     assert(!qemu_file_is_writable(f));
+    assert(offset < IO_BUF_SIZE);
+    assert(size + offset < IO_BUF_SIZE);
 
+    /* The 1st byte to read from */
     index = f->buf_index + offset;
+    /* The number of available bytes starting at index */
     pending = f->buf_size - index;
-    if (pending < size) {
-        qemu_fill_buffer(f);
+    while (pending < size) {
+        int received = qemu_fill_buffer(f);
+
+        if (received <= 0) {
+            break;
+        }
+
         index = f->buf_index + offset;
         pending = f->buf_size - index;
     }
@@ -709,24 +732,15 @@ int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, 
size_t offset)
     return size;
 }
 
-int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size)
+size_t qemu_get_buffer(QEMUFile *f, uint8_t *buf, size_t size)
 {
-    int pending = size;
-    int done = 0;
+    size_t res;
 
-    while (pending > 0) {
-        int res;
+    res = qemu_peek_buffer(f, buf, size, 0);
 
-        res = qemu_peek_buffer(f, buf, pending, 0);
-        if (res == 0) {
-            return done;
-        }
-        qemu_file_skip(f, res);
-        buf += res;
-        pending -= res;
-        done += res;
-    }
-    return done;
+    qemu_file_skip(f, res);
+
+    return res;
 }
 
 int qemu_peek_byte(QEMUFile *f, int offset)
@@ -735,12 +749,14 @@ int qemu_peek_byte(QEMUFile *f, int offset)
 
     assert(!qemu_file_is_writable(f));
 
-    if (index >= f->buf_size) {
-        qemu_fill_buffer(f);
-        index = f->buf_index + offset;
-        if (index >= f->buf_size) {
+    while (index >= f->buf_size) {
+        int received = qemu_fill_buffer(f);
+
+        if (received <= 0) {
             return 0;
         }
+
+        index = f->buf_index + offset;
     }
     return f->buf[index];
 }
diff --git a/vmstate.c b/vmstate.c
index d1f5eb0..b8e6e31 100644
--- a/vmstate.c
+++ b/vmstate.c
@@ -170,7 +170,7 @@ static int vmstate_subsection_load(QEMUFile *f, const 
VMStateDescription *vmsd,
         }
         size = qemu_peek_buffer(f, (uint8_t *)idstr, len, 2);
         if (size != len) {
-            return 0;
+            return -EIO;
         }
         idstr[size] = 0;
 
-- 
1.8.5.3