[Qemu-devel] [PULL v2 for-2.5 3/6] qga: flush explicitly when needed

[Michael Roth]

From: Marc-André Lureau <address@hidden>

According to the specification:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/fopen.html

"the application shall ensure that output is not directly followed by
input without an intervening call to fflush() or to a file positioning
function (fseek(), fsetpos(), or rewind()), and input is not directly
followed by output without an intervening call to a file positioning
function, unless the input operation encounters end-of-file."

Without this change, an fwrite() followed by an fread() may lose the
previously written content, as shown in the following test.

Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=1210246

Signed-off-by: Marc-André Lureau <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
* don't confuse {write,read}() with f{write,read}() in
  commit msg (Laszlo)
Signed-off-by: Michael Roth <address@hidden>
---
 qga/commands-posix.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 0ebd473..cf1d7ec 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -216,9 +216,16 @@ void qmp_guest_set_time(bool has_time, int64_t time_ns, 
Error **errp)
     }
 }
 
+typedef enum {
+    RW_STATE_NEW,
+    RW_STATE_READING,
+    RW_STATE_WRITING,
+} RwState;
+
 typedef struct GuestFileHandle {
     uint64_t id;
     FILE *fh;
+    RwState state;
     QTAILQ_ENTRY(GuestFileHandle) next;
 } GuestFileHandle;
 
@@ -460,6 +467,17 @@ struct GuestFileRead *qmp_guest_file_read(int64_t handle, 
bool has_count,
     }
 
     fh = gfh->fh;
+
+    /* explicitly flush when switching from writing to reading */
+    if (gfh->state == RW_STATE_WRITING) {
+        int ret = fflush(fh);
+        if (ret == EOF) {
+            error_setg_errno(errp, errno, "failed to flush file");
+            return NULL;
+        }
+        gfh->state = RW_STATE_NEW;
+    }
+
     buf = g_malloc0(count+1);
     read_count = fread(buf, 1, count, fh);
     if (ferror(fh)) {
@@ -473,6 +491,7 @@ struct GuestFileRead *qmp_guest_file_read(int64_t handle, 
bool has_count,
         if (read_count) {
             read_data->buf_b64 = g_base64_encode(buf, read_count);
         }
+        gfh->state = RW_STATE_READING;
     }
     g_free(buf);
     clearerr(fh);
@@ -496,6 +515,16 @@ GuestFileWrite *qmp_guest_file_write(int64_t handle, const 
char *buf_b64,
     }
 
     fh = gfh->fh;
+
+    if (gfh->state == RW_STATE_READING) {
+        int ret = fseek(fh, 0, SEEK_CUR);
+        if (ret == -1) {
+            error_setg_errno(errp, errno, "failed to seek file");
+            return NULL;
+        }
+        gfh->state = RW_STATE_NEW;
+    }
+
     buf = g_base64_decode(buf_b64, &buf_len);
 
     if (!has_count) {
@@ -515,6 +544,7 @@ GuestFileWrite *qmp_guest_file_write(int64_t handle, const 
char *buf_b64,
         write_data = g_new0(GuestFileWrite, 1);
         write_data->count = write_count;
         write_data->eof = feof(fh);
+        gfh->state = RW_STATE_WRITING;
     }
     g_free(buf);
     clearerr(fh);
@@ -538,10 +568,15 @@ struct GuestFileSeek *qmp_guest_file_seek(int64_t handle, 
int64_t offset,
     ret = fseek(fh, offset, whence);
     if (ret == -1) {
         error_setg_errno(errp, errno, "failed to seek file");
+        if (errno == ESPIPE) {
+            /* file is non-seekable, stdio shouldn't be buffering anyways */
+            gfh->state = RW_STATE_NEW;
+        }
     } else {
         seek_data = g_new0(GuestFileSeek, 1);
         seek_data->position = ftell(fh);
         seek_data->eof = feof(fh);
+        gfh->state = RW_STATE_NEW;
     }
     clearerr(fh);
 
@@ -562,6 +597,8 @@ void qmp_guest_file_flush(int64_t handle, Error **errp)
     ret = fflush(fh);
     if (ret == EOF) {
         error_setg_errno(errp, errno, "failed to flush file");
+    } else {
+        gfh->state = RW_STATE_NEW;
     }
 }
 
-- 
1.9.1