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

[Michael Roth]

From: Marc-André Lureau 

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

Reviewed-by: Eric Blake 
* don't confuse {write,read}() with f{write,read}() in
  commit msg (Laszlo)
Signed-off-by: Michael Roth 
---
 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, _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

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

[Eric Blake]

On 11/25/2015 03:47 PM, Michael Roth wrote:
> From: Marc-André Lureau 
> 
> 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
> 
> Reviewed-by: Eric Blake 
> * don't confuse {write,read}() with f{write,read}() in
>   commit msg (Laszlo)
> Signed-off-by: Michael Roth 
> ---

Looks like you lost Marc-Andre's S-o-b :(

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature