[Qemu-devel] [PATCH 0/4] qemu-file: Move QEMUFileOps implementations to separate files

[Qemu-devel] [PATCH 0/4] qemu-file: Move QEMUFileOps implementations to separate files

[Eduardo Habkost]

With this, code that uses symbols from qemu-file.c don't need to bring extra
dependencies because of the actual QEMUFile operation implementations.

Eduardo Habkost (4):
  qemu-file: Make qemu_file_is_writable() non-static
  qemu-file: Use qemu_file_is_writable() on stdio_fclose()
  qemu-file: Move unix and socket implementations to qemu-file-unix.c
  qemu-file: Move stdio implementation to qemu-file-stdio.c

 Makefile.objs                 |   2 +-
 include/migration/qemu-file.h |   1 +
 qemu-file-stdio.c             | 194 ++++++++++++++++++++++
 qemu-file-unix.c              | 223 ++++++++++++++++++++++++++
 qemu-file.c                   | 365 +-----------------------------------------
 tests/Makefile                |   2 +-
 6 files changed, 421 insertions(+), 366 deletions(-)
 create mode 100644 qemu-file-stdio.c
 create mode 100644 qemu-file-unix.c

-- 
1.9.3

[Qemu-devel] [PATCH 1/4] qemu-file: Make qemu_file_is_writable() non-static

[Eduardo Habkost]

The QEMUFileStdio code will use qemu_file_is_writable() and will be
moved to a separate file.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 include/migration/qemu-file.h | 1 +
 qemu-file.c                   | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index c90f529..1885b41 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -110,6 +110,7 @@ void qemu_put_byte(QEMUFile *f, int v);
  */
 void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, int size);
 bool qemu_file_mode_is_not_valid(const char *mode);
+bool qemu_file_is_writable(QEMUFile *f);
 
 static inline void qemu_put_ubyte(QEMUFile *f, unsigned int v)
 {
diff --git a/qemu-file.c b/qemu-file.c
index a8e3912..6c8a6c9 100644
--- a/qemu-file.c
+++ b/qemu-file.c
@@ -433,7 +433,7 @@ void qemu_file_set_error(QEMUFile *f, int ret)
     }
 }
 
-static inline bool qemu_file_is_writable(QEMUFile *f)
+bool qemu_file_is_writable(QEMUFile *f)
 {
     return f->ops->writev_buffer || f->ops->put_buffer;
 }
-- 
1.9.3

[Qemu-devel] [PATCH 2/4] qemu-file: Use qemu_file_is_writable() on stdio_fclose()

[Eduardo Habkost]

Use the existing function which checks if writev_buffer() or
put_buffer() are set, instead of duplicating it.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 qemu-file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-file.c b/qemu-file.c
index 6c8a6c9..482bda6 100644
--- a/qemu-file.c
+++ b/qemu-file.c
@@ -152,7 +152,7 @@ static int stdio_fclose(void *opaque)
     QEMUFileStdio *s = opaque;
     int ret = 0;
 
-    if (s->file->ops->put_buffer || s->file->ops->writev_buffer) {
+    if (qemu_file_is_writable(s->file)) {
         int fd = fileno(s->stdio_file);
         struct stat st;
 
-- 
1.9.3

[Qemu-devel] [PATCH 3/4] qemu-file: Move unix and socket implementations to qemu-file-unix.c

[Eduardo Habkost]

Separate the QEMUFile interface from the implementation, to reduce
dependencies from code using QEMUFile.

All the code that is being moved to the new file is exactly the same
code that was on savevm.c (moved by commit
093c455a8c6d8f715eabd8c8d346f08f17d686ec), so I am using the copyright
and license header from savevm.c for the new file.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 Makefile.objs    |   2 +-
 qemu-file-unix.c | 223 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-file.c      | 195 ------------------------------------------------
 tests/Makefile   |   2 +-
 4 files changed, 225 insertions(+), 197 deletions(-)
 create mode 100644 qemu-file-unix.c

diff --git a/Makefile.objs b/Makefile.objs
index 97db978..b89134b 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -50,7 +50,7 @@ common-obj-$(CONFIG_LINUX) += fsdev/
 
 common-obj-y += migration.o migration-tcp.o
 common-obj-y += vmstate.o
-common-obj-y += qemu-file.o
+common-obj-y += qemu-file.o qemu-file-unix.o
 common-obj-$(CONFIG_RDMA) += migration-rdma.o
 common-obj-y += qemu-char.o #aio.o
 common-obj-y += block-migration.o
diff --git a/qemu-file-unix.c b/qemu-file-unix.c
new file mode 100644
index 0000000..9682396
--- /dev/null
+++ b/qemu-file-unix.c
@@ -0,0 +1,223 @@
+/*
+ * QEMU System Emulator
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "qemu-common.h"
+#include "qemu/iov.h"
+#include "qemu/sockets.h"
+#include "block/coroutine.h"
+#include "migration/qemu-file.h"
+
+typedef struct QEMUFileSocket {
+    int fd;
+    QEMUFile *file;
+} QEMUFileSocket;
+
+static ssize_t socket_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
+                                    int64_t pos)
+{
+    QEMUFileSocket *s = opaque;
+    ssize_t len;
+    ssize_t size = iov_size(iov, iovcnt);
+
+    len = iov_send(s->fd, iov, iovcnt, 0, size);
+    if (len < size) {
+        len = -socket_error();
+    }
+    return len;
+}
+
+static int socket_get_fd(void *opaque)
+{
+    QEMUFileSocket *s = opaque;
+
+    return s->fd;
+}
+
+static int socket_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
+{
+    QEMUFileSocket *s = opaque;
+    ssize_t len;
+
+    for (;;) {
+        len = qemu_recv(s->fd, buf, size, 0);
+        if (len != -1) {
+            break;
+        }
+        if (socket_error() == EAGAIN) {
+            yield_until_fd_readable(s->fd);
+        } else if (socket_error() != EINTR) {
+            break;
+        }
+    }
+
+    if (len == -1) {
+        len = -socket_error();
+    }
+    return len;
+}
+
+static int socket_close(void *opaque)
+{
+    QEMUFileSocket *s = opaque;
+    closesocket(s->fd);
+    g_free(s);
+    return 0;
+}
+
+static ssize_t unix_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
+                                  int64_t pos)
+{
+    QEMUFileSocket *s = opaque;
+    ssize_t len, offset;
+    ssize_t size = iov_size(iov, iovcnt);
+    ssize_t total = 0;
+
+    assert(iovcnt > 0);
+    offset = 0;
+    while (size > 0) {
+        /* Find the next start position; skip all full-sized vector elements  */
+        while (offset >= iov[0].iov_len) {
+            offset -= iov[0].iov_len;
+            iov++, iovcnt--;
+        }
+
+        /* skip `offset' bytes from the (now) first element, undo it on exit */
+        assert(iovcnt > 0);
+        iov[0].iov_base += offset;
+        iov[0].iov_len -= offset;
+
+        do {
+            len = writev(s->fd, iov, iovcnt);
+        } while (len == -1 && errno == EINTR);
+        if (len == -1) {
+            return -errno;
+        }
+
+        /* Undo the changes above */
+        iov[0].iov_base -= offset;
+        iov[0].iov_len += offset;
+
+        /* Prepare for the next iteration */
+        offset += len;
+        total += len;
+        size -= len;
+    }
+
+    return total;
+}
+
+static int unix_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
+{
+    QEMUFileSocket *s = opaque;
+    ssize_t len;
+
+    for (;;) {
+        len = read(s->fd, buf, size);
+        if (len != -1) {
+            break;
+        }
+        if (errno == EAGAIN) {
+            yield_until_fd_readable(s->fd);
+        } else if (errno != EINTR) {
+            break;
+        }
+    }
+
+    if (len == -1) {
+        len = -errno;
+    }
+    return len;
+}
+
+static int unix_close(void *opaque)
+{
+    QEMUFileSocket *s = opaque;
+    close(s->fd);
+    g_free(s);
+    return 0;
+}
+
+static const QEMUFileOps unix_read_ops = {
+    .get_fd =     socket_get_fd,
+    .get_buffer = unix_get_buffer,
+    .close =      unix_close
+};
+
+static const QEMUFileOps unix_write_ops = {
+    .get_fd =     socket_get_fd,
+    .writev_buffer = unix_writev_buffer,
+    .close =      unix_close
+};
+
+QEMUFile *qemu_fdopen(int fd, const char *mode)
+{
+    QEMUFileSocket *s;
+
+    if (mode == NULL ||
+        (mode[0] != 'r' && mode[0] != 'w') ||
+        mode[1] != 'b' || mode[2] != 0) {
+        fprintf(stderr, "qemu_fdopen: Argument validity check failed\n");
+        return NULL;
+    }
+
+    s = g_malloc0(sizeof(QEMUFileSocket));
+    s->fd = fd;
+
+    if (mode[0] == 'r') {
+        s->file = qemu_fopen_ops(s, &unix_read_ops);
+    } else {
+        s->file = qemu_fopen_ops(s, &unix_write_ops);
+    }
+    return s->file;
+}
+
+static const QEMUFileOps socket_read_ops = {
+    .get_fd =     socket_get_fd,
+    .get_buffer = socket_get_buffer,
+    .close =      socket_close
+};
+
+static const QEMUFileOps socket_write_ops = {
+    .get_fd =     socket_get_fd,
+    .writev_buffer = socket_writev_buffer,
+    .close =      socket_close
+};
+
+QEMUFile *qemu_fopen_socket(int fd, const char *mode)
+{
+    QEMUFileSocket *s;
+
+    if (qemu_file_mode_is_not_valid(mode)) {
+        return NULL;
+    }
+
+    s = g_malloc0(sizeof(QEMUFileSocket));
+    s->fd = fd;
+    if (mode[0] == 'w') {
+        qemu_set_block(s->fd);
+        s->file = qemu_fopen_ops(s, &socket_write_ops);
+    } else {
+        s->file = qemu_fopen_ops(s, &socket_read_ops);
+    }
+    return s->file;
+}
diff --git a/qemu-file.c b/qemu-file.c
index 482bda6..e277d2a 100644
--- a/qemu-file.c
+++ b/qemu-file.c
@@ -33,63 +33,6 @@ typedef struct QEMUFileStdio {
     QEMUFile *file;
 } QEMUFileStdio;
 
-typedef struct QEMUFileSocket {
-    int fd;
-    QEMUFile *file;
-} QEMUFileSocket;
-
-static ssize_t socket_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
-                                    int64_t pos)
-{
-    QEMUFileSocket *s = opaque;
-    ssize_t len;
-    ssize_t size = iov_size(iov, iovcnt);
-
-    len = iov_send(s->fd, iov, iovcnt, 0, size);
-    if (len < size) {
-        len = -socket_error();
-    }
-    return len;
-}
-
-static int socket_get_fd(void *opaque)
-{
-    QEMUFileSocket *s = opaque;
-
-    return s->fd;
-}
-
-static int socket_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
-{
-    QEMUFileSocket *s = opaque;
-    ssize_t len;
-
-    for (;;) {
-        len = qemu_recv(s->fd, buf, size, 0);
-        if (len != -1) {
-            break;
-        }
-        if (socket_error() == EAGAIN) {
-            yield_until_fd_readable(s->fd);
-        } else if (socket_error() != EINTR) {
-            break;
-        }
-    }
-
-    if (len == -1) {
-        len = -socket_error();
-    }
-    return len;
-}
-
-static int socket_close(void *opaque)
-{
-    QEMUFileSocket *s = opaque;
-    closesocket(s->fd);
-    g_free(s);
-    return 0;
-}
-
 static int stdio_get_fd(void *opaque)
 {
     QEMUFileStdio *s = opaque;
@@ -227,125 +170,6 @@ static const QEMUFileOps stdio_file_write_ops = {
     .close =      stdio_fclose
 };
 
-static ssize_t unix_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
-                                  int64_t pos)
-{
-    QEMUFileSocket *s = opaque;
-    ssize_t len, offset;
-    ssize_t size = iov_size(iov, iovcnt);
-    ssize_t total = 0;
-
-    assert(iovcnt > 0);
-    offset = 0;
-    while (size > 0) {
-        /* Find the next start position; skip all full-sized vector elements  */
-        while (offset >= iov[0].iov_len) {
-            offset -= iov[0].iov_len;
-            iov++, iovcnt--;
-        }
-
-        /* skip `offset' bytes from the (now) first element, undo it on exit */
-        assert(iovcnt > 0);
-        iov[0].iov_base += offset;
-        iov[0].iov_len -= offset;
-
-        do {
-            len = writev(s->fd, iov, iovcnt);
-        } while (len == -1 && errno == EINTR);
-        if (len == -1) {
-            return -errno;
-        }
-
-        /* Undo the changes above */
-        iov[0].iov_base -= offset;
-        iov[0].iov_len += offset;
-
-        /* Prepare for the next iteration */
-        offset += len;
-        total += len;
-        size -= len;
-    }
-
-    return total;
-}
-
-static int unix_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
-{
-    QEMUFileSocket *s = opaque;
-    ssize_t len;
-
-    for (;;) {
-        len = read(s->fd, buf, size);
-        if (len != -1) {
-            break;
-        }
-        if (errno == EAGAIN) {
-            yield_until_fd_readable(s->fd);
-        } else if (errno != EINTR) {
-            break;
-        }
-    }
-
-    if (len == -1) {
-        len = -errno;
-    }
-    return len;
-}
-
-static int unix_close(void *opaque)
-{
-    QEMUFileSocket *s = opaque;
-    close(s->fd);
-    g_free(s);
-    return 0;
-}
-
-static const QEMUFileOps unix_read_ops = {
-    .get_fd =     socket_get_fd,
-    .get_buffer = unix_get_buffer,
-    .close =      unix_close
-};
-
-static const QEMUFileOps unix_write_ops = {
-    .get_fd =     socket_get_fd,
-    .writev_buffer = unix_writev_buffer,
-    .close =      unix_close
-};
-
-QEMUFile *qemu_fdopen(int fd, const char *mode)
-{
-    QEMUFileSocket *s;
-
-    if (mode == NULL ||
-        (mode[0] != 'r' && mode[0] != 'w') ||
-        mode[1] != 'b' || mode[2] != 0) {
-        fprintf(stderr, "qemu_fdopen: Argument validity check failed\n");
-        return NULL;
-    }
-
-    s = g_malloc0(sizeof(QEMUFileSocket));
-    s->fd = fd;
-
-    if (mode[0] == 'r') {
-        s->file = qemu_fopen_ops(s, &unix_read_ops);
-    } else {
-        s->file = qemu_fopen_ops(s, &unix_write_ops);
-    }
-    return s->file;
-}
-
-static const QEMUFileOps socket_read_ops = {
-    .get_fd =     socket_get_fd,
-    .get_buffer = socket_get_buffer,
-    .close =      socket_close
-};
-
-static const QEMUFileOps socket_write_ops = {
-    .get_fd =     socket_get_fd,
-    .writev_buffer = socket_writev_buffer,
-    .close =      socket_close
-};
-
 bool qemu_file_mode_is_not_valid(const char *mode)
 {
     if (mode == NULL ||
@@ -358,25 +182,6 @@ bool qemu_file_mode_is_not_valid(const char *mode)
     return false;
 }
 
-QEMUFile *qemu_fopen_socket(int fd, const char *mode)
-{
-    QEMUFileSocket *s;
-
-    if (qemu_file_mode_is_not_valid(mode)) {
-        return NULL;
-    }
-
-    s = g_malloc0(sizeof(QEMUFileSocket));
-    s->fd = fd;
-    if (mode[0] == 'w') {
-        qemu_set_block(s->fd);
-        s->file = qemu_fopen_ops(s, &socket_write_ops);
-    } else {
-        s->file = qemu_fopen_ops(s, &socket_read_ops);
-    }
-    return s->file;
-}
-
 QEMUFile *qemu_fopen(const char *filename, const char *mode)
 {
     QEMUFileStdio *s;
diff --git a/tests/Makefile b/tests/Makefile
index 834279c..83b0ad6 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -259,7 +259,7 @@ tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
 	$(test-qapi-obj-y) \
 	libqemuutil.a libqemustub.a
 tests/test-vmstate$(EXESUF): tests/test-vmstate.o \
-	vmstate.o qemu-file.o \
+	vmstate.o qemu-file.o qemu-file-unix.o \
 	libqemuutil.a
 
 tests/test-qapi-types.c tests/test-qapi-types.h :\
-- 
1.9.3

[Qemu-devel] [PATCH 4/4] qemu-file: Move stdio implementation to qemu-file-stdio.c

[Eduardo Habkost]

Separate the QEMUFile interface from the stdio-specific implementation,
to reduce dependencies from code using QEMUFile.

The code that is being moved is similar to the one that was on savevm.c before
it was moved in commit 093c455a8c6d8f715eabd8c8d346f08f17d686ec, except for
some changes done by Markus, Juan, and myself. So, I am using the copyright and
license header from savevm.c, but CCing Juan and Markus so they can review the
copyright/license header.

Cc: Markus Armbruster <armbru@redhat.com>
Cc: Juan Quintela <quintela@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 Makefile.objs     |   2 +-
 qemu-file-stdio.c | 194 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-file.c       | 168 ----------------------------------------------
 3 files changed, 195 insertions(+), 169 deletions(-)
 create mode 100644 qemu-file-stdio.c

diff --git a/Makefile.objs b/Makefile.objs
index b89134b..22239a3 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -50,7 +50,7 @@ common-obj-$(CONFIG_LINUX) += fsdev/
 
 common-obj-y += migration.o migration-tcp.o
 common-obj-y += vmstate.o
-common-obj-y += qemu-file.o qemu-file-unix.o
+common-obj-y += qemu-file.o qemu-file-unix.o qemu-file-stdio.o
 common-obj-$(CONFIG_RDMA) += migration-rdma.o
 common-obj-y += qemu-char.o #aio.o
 common-obj-y += block-migration.o
diff --git a/qemu-file-stdio.c b/qemu-file-stdio.c
new file mode 100644
index 0000000..285068b
--- /dev/null
+++ b/qemu-file-stdio.c
@@ -0,0 +1,194 @@
+/*
+ * QEMU System Emulator
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "qemu-common.h"
+#include "block/coroutine.h"
+#include "migration/qemu-file.h"
+
+typedef struct QEMUFileStdio {
+    FILE *stdio_file;
+    QEMUFile *file;
+} QEMUFileStdio;
+
+static int stdio_get_fd(void *opaque)
+{
+    QEMUFileStdio *s = opaque;
+
+    return fileno(s->stdio_file);
+}
+
+static int stdio_put_buffer(void *opaque, const uint8_t *buf, int64_t pos,
+                            int size)
+{
+    QEMUFileStdio *s = opaque;
+    int res;
+
+    res = fwrite(buf, 1, size, s->stdio_file);
+
+    if (res != size) {
+        return -errno;
+    }
+    return res;
+}
+
+static int stdio_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
+{
+    QEMUFileStdio *s = opaque;
+    FILE *fp = s->stdio_file;
+    int bytes;
+
+    for (;;) {
+        clearerr(fp);
+        bytes = fread(buf, 1, size, fp);
+        if (bytes != 0 || !ferror(fp)) {
+            break;
+        }
+        if (errno == EAGAIN) {
+            yield_until_fd_readable(fileno(fp));
+        } else if (errno != EINTR) {
+            break;
+        }
+    }
+    return bytes;
+}
+
+static int stdio_pclose(void *opaque)
+{
+    QEMUFileStdio *s = opaque;
+    int ret;
+    ret = pclose(s->stdio_file);
+    if (ret == -1) {
+        ret = -errno;
+    } else if (!WIFEXITED(ret) || WEXITSTATUS(ret) != 0) {
+        /* close succeeded, but non-zero exit code: */
+        ret = -EIO; /* fake errno value */
+    }
+    g_free(s);
+    return ret;
+}
+
+static int stdio_fclose(void *opaque)
+{
+    QEMUFileStdio *s = opaque;
+    int ret = 0;
+
+    if (qemu_file_is_writable(s->file)) {
+        int fd = fileno(s->stdio_file);
+        struct stat st;
+
+        ret = fstat(fd, &st);
+        if (ret == 0 && S_ISREG(st.st_mode)) {
+            /*
+             * If the file handle is a regular file make sure the
+             * data is flushed to disk before signaling success.
+             */
+            ret = fsync(fd);
+            if (ret != 0) {
+                ret = -errno;
+                return ret;
+            }
+        }
+    }
+    if (fclose(s->stdio_file) == EOF) {
+        ret = -errno;
+    }
+    g_free(s);
+    return ret;
+}
+
+static const QEMUFileOps stdio_pipe_read_ops = {
+    .get_fd =     stdio_get_fd,
+    .get_buffer = stdio_get_buffer,
+    .close =      stdio_pclose
+};
+
+static const QEMUFileOps stdio_pipe_write_ops = {
+    .get_fd =     stdio_get_fd,
+    .put_buffer = stdio_put_buffer,
+    .close =      stdio_pclose
+};
+
+QEMUFile *qemu_popen_cmd(const char *command, const char *mode)
+{
+    FILE *stdio_file;
+    QEMUFileStdio *s;
+
+    if (mode == NULL || (mode[0] != 'r' && mode[0] != 'w') || mode[1] != 0) {
+        fprintf(stderr, "qemu_popen: Argument validity check failed\n");
+        return NULL;
+    }
+
+    stdio_file = popen(command, mode);
+    if (stdio_file == NULL) {
+        return NULL;
+    }
+
+    s = g_malloc0(sizeof(QEMUFileStdio));
+
+    s->stdio_file = stdio_file;
+
+    if (mode[0] == 'r') {
+        s->file = qemu_fopen_ops(s, &stdio_pipe_read_ops);
+    } else {
+        s->file = qemu_fopen_ops(s, &stdio_pipe_write_ops);
+    }
+    return s->file;
+}
+
+static const QEMUFileOps stdio_file_read_ops = {
+    .get_fd =     stdio_get_fd,
+    .get_buffer = stdio_get_buffer,
+    .close =      stdio_fclose
+};
+
+static const QEMUFileOps stdio_file_write_ops = {
+    .get_fd =     stdio_get_fd,
+    .put_buffer = stdio_put_buffer,
+    .close =      stdio_fclose
+};
+
+QEMUFile *qemu_fopen(const char *filename, const char *mode)
+{
+    QEMUFileStdio *s;
+
+    if (qemu_file_mode_is_not_valid(mode)) {
+        return NULL;
+    }
+
+    s = g_malloc0(sizeof(QEMUFileStdio));
+
+    s->stdio_file = fopen(filename, mode);
+    if (!s->stdio_file) {
+        goto fail;
+    }
+
+    if (mode[0] == 'w') {
+        s->file = qemu_fopen_ops(s, &stdio_file_write_ops);
+    } else {
+        s->file = qemu_fopen_ops(s, &stdio_file_read_ops);
+    }
+    return s->file;
+fail:
+    g_free(s);
+    return NULL;
+}
diff --git a/qemu-file.c b/qemu-file.c
index e277d2a..67865fe 100644
--- a/qemu-file.c
+++ b/qemu-file.c
@@ -28,148 +28,6 @@ struct QEMUFile {
     int last_error;
 };
 
-typedef struct QEMUFileStdio {
-    FILE *stdio_file;
-    QEMUFile *file;
-} QEMUFileStdio;
-
-static int stdio_get_fd(void *opaque)
-{
-    QEMUFileStdio *s = opaque;
-
-    return fileno(s->stdio_file);
-}
-
-static int stdio_put_buffer(void *opaque, const uint8_t *buf, int64_t pos,
-                            int size)
-{
-    QEMUFileStdio *s = opaque;
-    int res;
-
-    res = fwrite(buf, 1, size, s->stdio_file);
-
-    if (res != size) {
-        return -errno;
-    }
-    return res;
-}
-
-static int stdio_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
-{
-    QEMUFileStdio *s = opaque;
-    FILE *fp = s->stdio_file;
-    int bytes;
-
-    for (;;) {
-        clearerr(fp);
-        bytes = fread(buf, 1, size, fp);
-        if (bytes != 0 || !ferror(fp)) {
-            break;
-        }
-        if (errno == EAGAIN) {
-            yield_until_fd_readable(fileno(fp));
-        } else if (errno != EINTR) {
-            break;
-        }
-    }
-    return bytes;
-}
-
-static int stdio_pclose(void *opaque)
-{
-    QEMUFileStdio *s = opaque;
-    int ret;
-    ret = pclose(s->stdio_file);
-    if (ret == -1) {
-        ret = -errno;
-    } else if (!WIFEXITED(ret) || WEXITSTATUS(ret) != 0) {
-        /* close succeeded, but non-zero exit code: */
-        ret = -EIO; /* fake errno value */
-    }
-    g_free(s);
-    return ret;
-}
-
-static int stdio_fclose(void *opaque)
-{
-    QEMUFileStdio *s = opaque;
-    int ret = 0;
-
-    if (qemu_file_is_writable(s->file)) {
-        int fd = fileno(s->stdio_file);
-        struct stat st;
-
-        ret = fstat(fd, &st);
-        if (ret == 0 && S_ISREG(st.st_mode)) {
-            /*
-             * If the file handle is a regular file make sure the
-             * data is flushed to disk before signaling success.
-             */
-            ret = fsync(fd);
-            if (ret != 0) {
-                ret = -errno;
-                return ret;
-            }
-        }
-    }
-    if (fclose(s->stdio_file) == EOF) {
-        ret = -errno;
-    }
-    g_free(s);
-    return ret;
-}
-
-static const QEMUFileOps stdio_pipe_read_ops = {
-    .get_fd =     stdio_get_fd,
-    .get_buffer = stdio_get_buffer,
-    .close =      stdio_pclose
-};
-
-static const QEMUFileOps stdio_pipe_write_ops = {
-    .get_fd =     stdio_get_fd,
-    .put_buffer = stdio_put_buffer,
-    .close =      stdio_pclose
-};
-
-QEMUFile *qemu_popen_cmd(const char *command, const char *mode)
-{
-    FILE *stdio_file;
-    QEMUFileStdio *s;
-
-    if (mode == NULL || (mode[0] != 'r' && mode[0] != 'w') || mode[1] != 0) {
-        fprintf(stderr, "qemu_popen: Argument validity check failed\n");
-        return NULL;
-    }
-
-    stdio_file = popen(command, mode);
-    if (stdio_file == NULL) {
-        return NULL;
-    }
-
-    s = g_malloc0(sizeof(QEMUFileStdio));
-
-    s->stdio_file = stdio_file;
-
-    if (mode[0] == 'r') {
-        s->file = qemu_fopen_ops(s, &stdio_pipe_read_ops);
-    } else {
-        s->file = qemu_fopen_ops(s, &stdio_pipe_write_ops);
-    }
-    return s->file;
-}
-
-static const QEMUFileOps stdio_file_read_ops = {
-    .get_fd =     stdio_get_fd,
-    .get_buffer = stdio_get_buffer,
-    .close =      stdio_fclose
-};
-
-static const QEMUFileOps stdio_file_write_ops = {
-    .get_fd =     stdio_get_fd,
-    .put_buffer = stdio_put_buffer,
-    .close =      stdio_fclose
-};
-
 bool qemu_file_mode_is_not_valid(const char *mode)
 {
     if (mode == NULL ||
@@ -182,32 +40,6 @@ bool qemu_file_mode_is_not_valid(const char *mode)
     return false;
 }
 
-QEMUFile *qemu_fopen(const char *filename, const char *mode)
-{
-    QEMUFileStdio *s;
-
-    if (qemu_file_mode_is_not_valid(mode)) {
-        return NULL;
-    }
-
-    s = g_malloc0(sizeof(QEMUFileStdio));
-
-    s->stdio_file = fopen(filename, mode);
-    if (!s->stdio_file) {
-        goto fail;
-    }
-
-    if (mode[0] == 'w') {
-        s->file = qemu_fopen_ops(s, &stdio_file_write_ops);
-    } else {
-        s->file = qemu_fopen_ops(s, &stdio_file_read_ops);
-    }
-    return s->file;
-fail:
-    g_free(s);
-    return NULL;
-}
-
 QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops)
 {
     QEMUFile *f;
-- 
1.9.3

Re: [Qemu-devel] [PATCH 0/4] qemu-file: Move QEMUFileOps implementations to separate files

[Markus Armbruster]

Eduardo Habkost <ehabkost@redhat.com> writes:

> With this, code that uses symbols from qemu-file.c don't need to bring extra
> dependencies because of the actual QEMUFile operation implementations.

Each case of omitting the "extra dependencies" should be visible in
makefiles as "prerequisites include qemu-file.o, but not all
qemu-file-*.o".  I can see one: tests/Makefile has just
qemu-file-unix.o, but not qemu-file-stdio.o.  Out of curiosity: will
there be more?

Regardless:

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [PATCH 0/4] qemu-file: Move QEMUFileOps implementations to separate files

[Eduardo Habkost]

On Thu, Oct 02, 2014 at 08:49:12AM +0200, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > With this, code that uses symbols from qemu-file.c don't need to bring extra
> > dependencies because of the actual QEMUFile operation implementations.
> 
> Each case of omitting the "extra dependencies" should be visible in
> makefiles as "prerequisites include qemu-file.o, but not all
> qemu-file-*.o".  I can see one: tests/Makefile has just
> qemu-file-unix.o, but not qemu-file-stdio.o.  Out of curiosity: will
> there be more?

Yes, tests/test-x86-cpu[1] will only need qemu-file.o. That was the
original motivation for this series.

[1] See Subject: [PATCH v2 0/7] Target-specific unit test support, add
    unit tests for target-i386/cpu.c code

-- 
Eduardo

Re: [Qemu-devel] [PATCH 0/4] qemu-file: Move QEMUFileOps implementations to separate files

[Juan Quintela]

Eduardo Habkost <ehabkost@redhat.com> wrote:
> With this, code that uses symbols from qemu-file.c don't need to bring extra
> dependencies because of the actual QEMUFile operation implementations.
>
> Eduardo Habkost (4):
>   qemu-file: Make qemu_file_is_writable() non-static
>   qemu-file: Use qemu_file_is_writable() on stdio_fclose()
>   qemu-file: Move unix and socket implementations to qemu-file-unix.c
>   qemu-file: Move stdio implementation to qemu-file-stdio.c

Reviewed-by: Juan Quintela <quintela@redhat.com>

Applied.  Minor conflicts with David qemu-file on buffer on Makefiles
handled by me.