qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 00/10] Migration pull request
@ 2014-10-15  8:24 Juan Quintela
  2014-10-15  8:24 ` [Qemu-devel] [PULL 01/10] QEMUSizedBuffer based QEMUFile Juan Quintela
                   ` (10 more replies)
  0 siblings, 11 replies; 12+ messages in thread
From: Juan Quintela @ 2014-10-15  8:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: Juan Quintela

From: Juan Quintela <quintela@trasno.org>

Hi

This patches have been on the list for a while:

- ram flags from Peter Lieven
- qemu-file cleanups from Eduardo (just code movement)
- QEMUSizedBuffer from David (they have already have 6 versions)
- VBUFFER (new macro, can't break old things) (Alexey)
- invalidate state (Alexey)

Please, apply.

The following changes since commit b1d28ec6a7dbdaadda39d29322f0de694aeb0b74:

  Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20141010' into staging (2014-10-10 14:55:29 +0100)

are available in the git repository at:

  git://github.com/juanquintela/qemu.git migration/20141015

for you to fetch changes up to 5b0e9dd46fbda5152566a4a26fd96bc0d0452bf7:

  migration: catch unknown flag combinations in ram_load (2014-10-14 11:24:20 +0200)

----------------------------------------------------------------
migration/next for 20141015

----------------------------------------------------------------
Alexey Kardashevskiy (2):
      block/migration: Disable cache invalidate for incoming migration
      vmstate: Allow dynamic allocation for VBUFFER during migration

Dr. David Alan Gilbert (2):
      QEMUSizedBuffer based QEMUFile
      Tests: QEMUSizedBuffer/QEMUBuffer

Eduardo Habkost (5):
      qemu-file: Add copyright header to qemu-file.c
      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

Peter Lieven (1):
      migration: catch unknown flag combinations in ram_load

 Makefile.objs                 |   2 +-
 arch_init.c                   |  62 ++--
 block.c                       |  18 +-
 include/migration/qemu-file.h |  27 ++
 include/migration/vmstate.h   |  11 +
 include/qemu/typedefs.h       |   1 +
 migration.c                   |   1 -
 nbd.c                         |   6 +
 qemu-file-stdio.c             | 194 ++++++++++
 qemu-file-unix.c              | 223 +++++++++++
 qemu-file.c                   | 843 ++++++++++++++++++++++++------------------
 tests/Makefile                |   4 +-
 tests/test-vmstate.c          |  74 ++--
 vmstate.c                     |  13 +-
 14 files changed, 1029 insertions(+), 450 deletions(-)
 create mode 100644 qemu-file-stdio.c
 create mode 100644 qemu-file-unix.c

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 01/10] QEMUSizedBuffer based QEMUFile
  2014-10-15  8:24 [Qemu-devel] [PULL 00/10] Migration pull request Juan Quintela
@ 2014-10-15  8:24 ` Juan Quintela
  2014-10-15  8:24 ` [Qemu-devel] [PULL 02/10] Tests: QEMUSizedBuffer/QEMUBuffer Juan Quintela
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Juan Quintela @ 2014-10-15  8:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: Joel Schopp, Dr. David Alan Gilbert, Stefan Berger

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

This is based on Stefan and Joel's patch that creates a QEMUFile that goes
to a memory buffer; from:

http://lists.gnu.org/archive/html/qemu-devel/2013-03/msg05036.html

Using the QEMUFile interface, this patch adds support functions for
operating on in-memory sized buffers that can be written to or read from.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Signed-off-by: Joel Schopp <jschopp@linux.vnet.ibm.com>

For fixes/tweeks I've done:
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 include/migration/qemu-file.h |  26 +++
 include/qemu/typedefs.h       |   1 +
 qemu-file.c                   | 455 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 482 insertions(+)

diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index c90f529..70ff347 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -25,6 +25,8 @@
 #define QEMU_FILE_H 1
 #include "exec/cpu-common.h"

+#include <stdint.h>
+
 /* This function writes a chunk of data to a file at the given position.
  * The pos argument can be ignored if the file is only being used for
  * streaming.  The handler should try to write all of the data it can.
@@ -94,11 +96,19 @@ typedef struct QEMUFileOps {
     QEMURamSaveFunc *save_page;
 } QEMUFileOps;

+struct QEMUSizedBuffer {
+    struct iovec *iov;
+    size_t n_iov;
+    size_t size; /* total allocated size in all iov's */
+    size_t used; /* number of used bytes */
+};
+
 QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops);
 QEMUFile *qemu_fopen(const char *filename, const char *mode);
 QEMUFile *qemu_fdopen(int fd, const char *mode);
 QEMUFile *qemu_fopen_socket(int fd, const char *mode);
 QEMUFile *qemu_popen_cmd(const char *command, const char *mode);
+QEMUFile *qemu_bufopen(const char *mode, QEMUSizedBuffer *input);
 int qemu_get_fd(QEMUFile *f);
 int qemu_fclose(QEMUFile *f);
 int64_t qemu_ftell(QEMUFile *f);
@@ -111,6 +121,22 @@ 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);

+QEMUSizedBuffer *qsb_create(const uint8_t *buffer, size_t len);
+QEMUSizedBuffer *qsb_clone(const QEMUSizedBuffer *);
+void qsb_free(QEMUSizedBuffer *);
+size_t qsb_set_length(QEMUSizedBuffer *qsb, size_t length);
+size_t qsb_get_length(const QEMUSizedBuffer *qsb);
+ssize_t qsb_get_buffer(const QEMUSizedBuffer *, off_t start, size_t count,
+                       uint8_t *buf);
+ssize_t qsb_write_at(QEMUSizedBuffer *qsb, const uint8_t *buf,
+                     off_t pos, size_t count);
+
+
+/*
+ * For use on files opened with qemu_bufopen
+ */
+const QEMUSizedBuffer *qemu_buf_get(QEMUFile *f);
+
 static inline void qemu_put_ubyte(QEMUFile *f, unsigned int v)
 {
     qemu_put_byte(f, (int)v);
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 04df51b..168a408 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -71,6 +71,7 @@ typedef struct SSIBus SSIBus;
 typedef struct EventNotifier EventNotifier;
 typedef struct VirtIODevice VirtIODevice;
 typedef struct QEMUSGList QEMUSGList;
+typedef struct QEMUSizedBuffer QEMUSizedBuffer;
 typedef struct SHPCDevice SHPCDevice;
 typedef struct FWCfgState FWCfgState;
 typedef struct PcGuestInfo PcGuestInfo;
diff --git a/qemu-file.c b/qemu-file.c
index a8e3912..cdda745 100644
--- a/qemu-file.c
+++ b/qemu-file.c
@@ -878,3 +878,458 @@ uint64_t qemu_get_be64(QEMUFile *f)
     v |= qemu_get_be32(f);
     return v;
 }
+
+#define QSB_CHUNK_SIZE      (1 << 10)
+#define QSB_MAX_CHUNK_SIZE  (16 * QSB_CHUNK_SIZE)
+
+/**
+ * Create a QEMUSizedBuffer
+ * This type of buffer uses scatter-gather lists internally and
+ * can grow to any size. Any data array in the scatter-gather list
+ * can hold different amount of bytes.
+ *
+ * @buffer: Optional buffer to copy into the QSB
+ * @len: size of initial buffer; if @buffer is given, buffer must
+ *       hold at least len bytes
+ *
+ * Returns a pointer to a QEMUSizedBuffer or NULL on allocation failure
+ */
+QEMUSizedBuffer *qsb_create(const uint8_t *buffer, size_t len)
+{
+    QEMUSizedBuffer *qsb;
+    size_t alloc_len, num_chunks, i, to_copy;
+    size_t chunk_size = (len > QSB_MAX_CHUNK_SIZE)
+                        ? QSB_MAX_CHUNK_SIZE
+                        : QSB_CHUNK_SIZE;
+
+    num_chunks = DIV_ROUND_UP(len ? len : QSB_CHUNK_SIZE, chunk_size);
+    alloc_len = num_chunks * chunk_size;
+
+    qsb = g_try_new0(QEMUSizedBuffer, 1);
+    if (!qsb) {
+        return NULL;
+    }
+
+    qsb->iov = g_try_new0(struct iovec, num_chunks);
+    if (!qsb->iov) {
+        g_free(qsb);
+        return NULL;
+    }
+
+    qsb->n_iov = num_chunks;
+
+    for (i = 0; i < num_chunks; i++) {
+        qsb->iov[i].iov_base = g_try_malloc0(chunk_size);
+        if (!qsb->iov[i].iov_base) {
+            /* qsb_free is safe since g_free can cope with NULL */
+            qsb_free(qsb);
+            return NULL;
+        }
+
+        qsb->iov[i].iov_len = chunk_size;
+        if (buffer) {
+            to_copy = (len - qsb->used) > chunk_size
+                      ? chunk_size : (len - qsb->used);
+            memcpy(qsb->iov[i].iov_base, &buffer[qsb->used], to_copy);
+            qsb->used += to_copy;
+        }
+    }
+
+    qsb->size = alloc_len;
+
+    return qsb;
+}
+
+/**
+ * Free the QEMUSizedBuffer
+ *
+ * @qsb: The QEMUSizedBuffer to free
+ */
+void qsb_free(QEMUSizedBuffer *qsb)
+{
+    size_t i;
+
+    if (!qsb) {
+        return;
+    }
+
+    for (i = 0; i < qsb->n_iov; i++) {
+        g_free(qsb->iov[i].iov_base);
+    }
+    g_free(qsb->iov);
+    g_free(qsb);
+}
+
+/**
+ * Get the number of used bytes in the QEMUSizedBuffer
+ *
+ * @qsb: A QEMUSizedBuffer
+ *
+ * Returns the number of bytes currently used in this buffer
+ */
+size_t qsb_get_length(const QEMUSizedBuffer *qsb)
+{
+    return qsb->used;
+}
+
+/**
+ * Set the length of the buffer; the primary usage of this
+ * function is to truncate the number of used bytes in the buffer.
+ * The size will not be extended beyond the current number of
+ * allocated bytes in the QEMUSizedBuffer.
+ *
+ * @qsb: A QEMUSizedBuffer
+ * @new_len: The new length of bytes in the buffer
+ *
+ * Returns the number of bytes the buffer was truncated or extended
+ * to.
+ */
+size_t qsb_set_length(QEMUSizedBuffer *qsb, size_t new_len)
+{
+    if (new_len <= qsb->size) {
+        qsb->used = new_len;
+    } else {
+        qsb->used = qsb->size;
+    }
+    return qsb->used;
+}
+
+/**
+ * Get the iovec that holds the data for a given position @pos.
+ *
+ * @qsb: A QEMUSizedBuffer
+ * @pos: The index of a byte in the buffer
+ * @d_off: Pointer to an offset that this function will indicate
+ *         at what position within the returned iovec the byte
+ *         is to be found
+ *
+ * Returns the index of the iovec that holds the byte at the given
+ * index @pos in the byte stream; a negative number if the iovec
+ * for the given position @pos does not exist.
+ */
+static ssize_t qsb_get_iovec(const QEMUSizedBuffer *qsb,
+                             off_t pos, off_t *d_off)
+{
+    ssize_t i;
+    off_t curr = 0;
+
+    if (pos > qsb->used) {
+        return -1;
+    }
+
+    for (i = 0; i < qsb->n_iov; i++) {
+        if (curr + qsb->iov[i].iov_len > pos) {
+            *d_off = pos - curr;
+            return i;
+        }
+        curr += qsb->iov[i].iov_len;
+    }
+    return -1;
+}
+
+/*
+ * Convert the QEMUSizedBuffer into a flat buffer.
+ *
+ * Note: If at all possible, try to avoid this function since it
+ *       may unnecessarily copy memory around.
+ *
+ * @qsb: pointer to QEMUSizedBuffer
+ * @start: offset to start at
+ * @count: number of bytes to copy
+ * @buf: a pointer to a buffer to write into (at least @count bytes)
+ *
+ * Returns the number of bytes copied into the output buffer
+ */
+ssize_t qsb_get_buffer(const QEMUSizedBuffer *qsb, off_t start,
+                       size_t count, uint8_t *buffer)
+{
+    const struct iovec *iov;
+    size_t to_copy, all_copy;
+    ssize_t index;
+    off_t s_off;
+    off_t d_off = 0;
+    char *s;
+
+    if (start > qsb->used) {
+        return 0;
+    }
+
+    all_copy = qsb->used - start;
+    if (all_copy > count) {
+        all_copy = count;
+    } else {
+        count = all_copy;
+    }
+
+    index = qsb_get_iovec(qsb, start, &s_off);
+    if (index < 0) {
+        return 0;
+    }
+
+    while (all_copy > 0) {
+        iov = &qsb->iov[index];
+
+        s = iov->iov_base;
+
+        to_copy = iov->iov_len - s_off;
+        if (to_copy > all_copy) {
+            to_copy = all_copy;
+        }
+        memcpy(&buffer[d_off], &s[s_off], to_copy);
+
+        d_off += to_copy;
+        all_copy -= to_copy;
+
+        s_off = 0;
+        index++;
+    }
+
+    return count;
+}
+
+/**
+ * Grow the QEMUSizedBuffer to the given size and allocate
+ * memory for it.
+ *
+ * @qsb: A QEMUSizedBuffer
+ * @new_size: The new size of the buffer
+ *
+ * Return:
+ *    a negative error code in case of memory allocation failure
+ * or
+ *    the new size of the buffer. The returned size may be greater or equal
+ *    to @new_size.
+ */
+static ssize_t qsb_grow(QEMUSizedBuffer *qsb, size_t new_size)
+{
+    size_t needed_chunks, i;
+
+    if (qsb->size < new_size) {
+        struct iovec *new_iov;
+        size_t size_diff = new_size - qsb->size;
+        size_t chunk_size = (size_diff > QSB_MAX_CHUNK_SIZE)
+                             ? QSB_MAX_CHUNK_SIZE : QSB_CHUNK_SIZE;
+
+        needed_chunks = DIV_ROUND_UP(size_diff, chunk_size);
+
+        new_iov = g_try_new(struct iovec, qsb->n_iov + needed_chunks);
+        if (new_iov == NULL) {
+            return -ENOMEM;
+        }
+
+        /* Allocate new chunks as needed into new_iov */
+        for (i = qsb->n_iov; i < qsb->n_iov + needed_chunks; i++) {
+            new_iov[i].iov_base = g_try_malloc0(chunk_size);
+            new_iov[i].iov_len = chunk_size;
+            if (!new_iov[i].iov_base) {
+                size_t j;
+
+                /* Free previously allocated new chunks */
+                for (j = qsb->n_iov; j < i; j++) {
+                    g_free(new_iov[j].iov_base);
+                }
+                g_free(new_iov);
+
+                return -ENOMEM;
+            }
+        }
+
+        /*
+         * Now we can't get any allocation errors, copy over to new iov
+         * and switch.
+         */
+        for (i = 0; i < qsb->n_iov; i++) {
+            new_iov[i] = qsb->iov[i];
+        }
+
+        qsb->n_iov += needed_chunks;
+        g_free(qsb->iov);
+        qsb->iov = new_iov;
+        qsb->size += (needed_chunks * chunk_size);
+    }
+
+    return qsb->size;
+}
+
+/**
+ * Write into the QEMUSizedBuffer at a given position and a given
+ * number of bytes. This function will automatically grow the
+ * QEMUSizedBuffer.
+ *
+ * @qsb: A QEMUSizedBuffer
+ * @source: A byte array to copy data from
+ * @pos: The position within the @qsb to write data to
+ * @size: The number of bytes to copy into the @qsb
+ *
+ * Returns @size or a negative error code in case of memory allocation failure,
+ *           or with an invalid 'pos'
+ */
+ssize_t qsb_write_at(QEMUSizedBuffer *qsb, const uint8_t *source,
+                     off_t pos, size_t count)
+{
+    ssize_t rc = qsb_grow(qsb, pos + count);
+    size_t to_copy;
+    size_t all_copy = count;
+    const struct iovec *iov;
+    ssize_t index;
+    char *dest;
+    off_t d_off, s_off = 0;
+
+    if (rc < 0) {
+        return rc;
+    }
+
+    if (pos + count > qsb->used) {
+        qsb->used = pos + count;
+    }
+
+    index = qsb_get_iovec(qsb, pos, &d_off);
+    if (index < 0) {
+        return -EINVAL;
+    }
+
+    while (all_copy > 0) {
+        iov = &qsb->iov[index];
+
+        dest = iov->iov_base;
+
+        to_copy = iov->iov_len - d_off;
+        if (to_copy > all_copy) {
+            to_copy = all_copy;
+        }
+
+        memcpy(&dest[d_off], &source[s_off], to_copy);
+
+        s_off += to_copy;
+        all_copy -= to_copy;
+
+        d_off = 0;
+        index++;
+    }
+
+    return count;
+}
+
+/**
+ * Create a deep copy of the given QEMUSizedBuffer.
+ *
+ * @qsb: A QEMUSizedBuffer
+ *
+ * Returns a clone of @qsb or NULL on allocation failure
+ */
+QEMUSizedBuffer *qsb_clone(const QEMUSizedBuffer *qsb)
+{
+    QEMUSizedBuffer *out = qsb_create(NULL, qsb_get_length(qsb));
+    size_t i;
+    ssize_t res;
+    off_t pos = 0;
+
+    if (!out) {
+        return NULL;
+    }
+
+    for (i = 0; i < qsb->n_iov; i++) {
+        res =  qsb_write_at(out, qsb->iov[i].iov_base,
+                            pos, qsb->iov[i].iov_len);
+        if (res < 0) {
+            qsb_free(out);
+            return NULL;
+        }
+        pos += res;
+    }
+
+    return out;
+}
+
+typedef struct QEMUBuffer {
+    QEMUSizedBuffer *qsb;
+    QEMUFile *file;
+} QEMUBuffer;
+
+static int buf_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
+{
+    QEMUBuffer *s = opaque;
+    ssize_t len = qsb_get_length(s->qsb) - pos;
+
+    if (len <= 0) {
+        return 0;
+    }
+
+    if (len > size) {
+        len = size;
+    }
+    return qsb_get_buffer(s->qsb, pos, len, buf);
+}
+
+static int buf_put_buffer(void *opaque, const uint8_t *buf,
+                          int64_t pos, int size)
+{
+    QEMUBuffer *s = opaque;
+
+    return qsb_write_at(s->qsb, buf, pos, size);
+}
+
+static int buf_close(void *opaque)
+{
+    QEMUBuffer *s = opaque;
+
+    qsb_free(s->qsb);
+
+    g_free(s);
+
+    return 0;
+}
+
+const QEMUSizedBuffer *qemu_buf_get(QEMUFile *f)
+{
+    QEMUBuffer *p;
+
+    qemu_fflush(f);
+
+    p = f->opaque;
+
+    return p->qsb;
+}
+
+static const QEMUFileOps buf_read_ops = {
+    .get_buffer = buf_get_buffer,
+    .close =      buf_close,
+};
+
+static const QEMUFileOps buf_write_ops = {
+    .put_buffer = buf_put_buffer,
+    .close =      buf_close,
+};
+
+QEMUFile *qemu_bufopen(const char *mode, QEMUSizedBuffer *input)
+{
+    QEMUBuffer *s;
+
+    if (mode == NULL || (mode[0] != 'r' && mode[0] != 'w') ||
+        mode[1] != '\0') {
+        error_report("qemu_bufopen: Argument validity check failed");
+        return NULL;
+    }
+
+    s = g_malloc0(sizeof(QEMUBuffer));
+    if (mode[0] == 'r') {
+        s->qsb = input;
+    }
+
+    if (s->qsb == NULL) {
+        s->qsb = qsb_create(NULL, 0);
+    }
+    if (!s->qsb) {
+        g_free(s);
+        error_report("qemu_bufopen: qsb_create failed");
+        return NULL;
+    }
+
+
+    if (mode[0] == 'r') {
+        s->file = qemu_fopen_ops(s, &buf_read_ops);
+    } else {
+        s->file = qemu_fopen_ops(s, &buf_write_ops);
+    }
+    return s->file;
+}
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 02/10] Tests: QEMUSizedBuffer/QEMUBuffer
  2014-10-15  8:24 [Qemu-devel] [PULL 00/10] Migration pull request Juan Quintela
  2014-10-15  8:24 ` [Qemu-devel] [PULL 01/10] QEMUSizedBuffer based QEMUFile Juan Quintela
@ 2014-10-15  8:24 ` Juan Quintela
  2014-10-15  8:24 ` [Qemu-devel] [PULL 03/10] block/migration: Disable cache invalidate for incoming migration Juan Quintela
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Juan Quintela @ 2014-10-15  8:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: Dr. David Alan Gilbert

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Modify some of tests/test-vmstate.c to use the in memory file based
on QEMUSizedBuffer to provide basic testing of QEMUSizedBuffer and
the associated memory backed QEMUFile type.

Only some of the tests are changed so that the fd backed QEMUFile is
still tested.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 tests/Makefile       |  2 +-
 tests/test-vmstate.c | 74 +++++++++++++++++++++++++++-------------------------
 2 files changed, 39 insertions(+), 37 deletions(-)

diff --git a/tests/Makefile b/tests/Makefile
index ffa8312..31e867f 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -259,7 +259,7 @@ tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
 	libqemuutil.a libqemustub.a
 tests/test-vmstate$(EXESUF): tests/test-vmstate.o \
 	vmstate.o qemu-file.o \
-	libqemuutil.a
+	libqemuutil.a libqemustub.a

 tests/test-qapi-types.c tests/test-qapi-types.h :\
 $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py
diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c
index d72c64c..5e0fd13 100644
--- a/tests/test-vmstate.c
+++ b/tests/test-vmstate.c
@@ -43,6 +43,12 @@ void yield_until_fd_readable(int fd)
     select(fd + 1, &fds, NULL, NULL, NULL);
 }

+/*
+ * Some tests use 'open_test_file' to work on a real fd, some use
+ * an in memory file (QEMUSizedBuffer+qemu_bufopen); we could pick one
+ * but this way we test both.
+ */
+
 /* Duplicate temp_fd and seek to the beginning of the file */
 static QEMUFile *open_test_file(bool write)
 {
@@ -54,6 +60,30 @@ static QEMUFile *open_test_file(bool write)
     return qemu_fdopen(fd, write ? "wb" : "rb");
 }

+/* Open a read-only qemu-file from an existing memory block */
+static QEMUFile *open_mem_file_read(const void *data, size_t len)
+{
+    /* The qsb gets freed by qemu_fclose */
+    QEMUSizedBuffer *qsb = qsb_create(data, len);
+    g_assert(qsb);
+
+    return qemu_bufopen("r", qsb);
+}
+
+/*
+ * Check that the contents of the memory-buffered file f match
+ * the given size/data.
+ */
+static void check_mem_file(QEMUFile *f, void *data, size_t size)
+{
+    uint8_t *result = g_malloc(size);
+    const QEMUSizedBuffer *qsb = qemu_buf_get(f);
+    g_assert_cmpint(qsb_get_length(qsb), ==, size);
+    g_assert_cmpint(qsb_get_buffer(qsb, 0, size, result), ==, size);
+    g_assert_cmpint(memcmp(result, data, size), ==, 0);
+    g_free(result);
+}
+
 #define SUCCESS(val) \
     g_assert_cmpint((val), ==, 0)

@@ -371,14 +401,12 @@ static const VMStateDescription vmstate_skipping = {

 static void test_save_noskip(void)
 {
-    QEMUFile *fsave = open_test_file(true);
+    QEMUFile *fsave = qemu_bufopen("w", NULL);
     TestStruct obj = { .a = 1, .b = 2, .c = 3, .d = 4, .e = 5, .f = 6,
                        .skip_c_e = false };
     vmstate_save_state(fsave, &vmstate_skipping, &obj);
     g_assert(!qemu_file_get_error(fsave));
-    qemu_fclose(fsave);

-    QEMUFile *loading = open_test_file(false);
     uint8_t expected[] = {
         0, 0, 0, 1,             /* a */
         0, 0, 0, 2,             /* b */
@@ -387,52 +415,31 @@ static void test_save_noskip(void)
         0, 0, 0, 5,             /* e */
         0, 0, 0, 0, 0, 0, 0, 6, /* f */
     };
-    uint8_t result[sizeof(expected)];
-    g_assert_cmpint(qemu_get_buffer(loading, result, sizeof(result)), ==,
-                    sizeof(result));
-    g_assert(!qemu_file_get_error(loading));
-    g_assert_cmpint(memcmp(result, expected, sizeof(result)), ==, 0);
-
-    /* Must reach EOF */
-    qemu_get_byte(loading);
-    g_assert_cmpint(qemu_file_get_error(loading), ==, -EIO);
-
-    qemu_fclose(loading);
+    check_mem_file(fsave, expected, sizeof(expected));
+    qemu_fclose(fsave);
 }

 static void test_save_skip(void)
 {
-    QEMUFile *fsave = open_test_file(true);
+    QEMUFile *fsave = qemu_bufopen("w", NULL);
     TestStruct obj = { .a = 1, .b = 2, .c = 3, .d = 4, .e = 5, .f = 6,
                        .skip_c_e = true };
     vmstate_save_state(fsave, &vmstate_skipping, &obj);
     g_assert(!qemu_file_get_error(fsave));
-    qemu_fclose(fsave);

-    QEMUFile *loading = open_test_file(false);
     uint8_t expected[] = {
         0, 0, 0, 1,             /* a */
         0, 0, 0, 2,             /* b */
         0, 0, 0, 0, 0, 0, 0, 4, /* d */
         0, 0, 0, 0, 0, 0, 0, 6, /* f */
     };
-    uint8_t result[sizeof(expected)];
-    g_assert_cmpint(qemu_get_buffer(loading, result, sizeof(result)), ==,
-                    sizeof(result));
-    g_assert(!qemu_file_get_error(loading));
-    g_assert_cmpint(memcmp(result, expected, sizeof(result)), ==, 0);
-
-
-    /* Must reach EOF */
-    qemu_get_byte(loading);
-    g_assert_cmpint(qemu_file_get_error(loading), ==, -EIO);
+    check_mem_file(fsave, expected, sizeof(expected));

-    qemu_fclose(loading);
+    qemu_fclose(fsave);
 }

 static void test_load_noskip(void)
 {
-    QEMUFile *fsave = open_test_file(true);
     uint8_t buf[] = {
         0, 0, 0, 10,             /* a */
         0, 0, 0, 20,             /* b */
@@ -442,10 +449,8 @@ static void test_load_noskip(void)
         0, 0, 0, 0, 0, 0, 0, 60, /* f */
         QEMU_VM_EOF, /* just to ensure we won't get EOF reported prematurely */
     };
-    qemu_put_buffer(fsave, buf, sizeof(buf));
-    qemu_fclose(fsave);

-    QEMUFile *loading = open_test_file(false);
+    QEMUFile *loading = open_mem_file_read(buf, sizeof(buf));
     TestStruct obj = { .skip_c_e = false };
     vmstate_load_state(loading, &vmstate_skipping, &obj, 2);
     g_assert(!qemu_file_get_error(loading));
@@ -460,7 +465,6 @@ static void test_load_noskip(void)

 static void test_load_skip(void)
 {
-    QEMUFile *fsave = open_test_file(true);
     uint8_t buf[] = {
         0, 0, 0, 10,             /* a */
         0, 0, 0, 20,             /* b */
@@ -468,10 +472,8 @@ static void test_load_skip(void)
         0, 0, 0, 0, 0, 0, 0, 60, /* f */
         QEMU_VM_EOF, /* just to ensure we won't get EOF reported prematurely */
     };
-    qemu_put_buffer(fsave, buf, sizeof(buf));
-    qemu_fclose(fsave);

-    QEMUFile *loading = open_test_file(false);
+    QEMUFile *loading = open_mem_file_read(buf, sizeof(buf));
     TestStruct obj = { .skip_c_e = true, .c = 300, .e = 500 };
     vmstate_load_state(loading, &vmstate_skipping, &obj, 2);
     g_assert(!qemu_file_get_error(loading));
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 03/10] block/migration: Disable cache invalidate for incoming migration
  2014-10-15  8:24 [Qemu-devel] [PULL 00/10] Migration pull request Juan Quintela
  2014-10-15  8:24 ` [Qemu-devel] [PULL 01/10] QEMUSizedBuffer based QEMUFile Juan Quintela
  2014-10-15  8:24 ` [Qemu-devel] [PULL 02/10] Tests: QEMUSizedBuffer/QEMUBuffer Juan Quintela
@ 2014-10-15  8:24 ` Juan Quintela
  2014-10-15  8:24 ` [Qemu-devel] [PULL 04/10] vmstate: Allow dynamic allocation for VBUFFER during migration Juan Quintela
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Juan Quintela @ 2014-10-15  8:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy

From: Alexey Kardashevskiy <aik@ozlabs.ru>

When migrated using libvirt with "--copy-storage-all", at the end of
migration there is race between NBD mirroring task trying to do flush
and migration completion, both end up invalidating cache. Since qcow2
driver does not handle this situation very well, random crashes happen.

This disables the BDRV_O_INCOMING flag for the block device being migrated
once the cache has been invalidated.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

--

fixed parens by hand
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 block.c     | 18 +++++-------------
 migration.c |  1 -
 nbd.c       |  6 ++++++
 3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/block.c b/block.c
index d3aebeb..27533f3 100644
--- a/block.c
+++ b/block.c
@@ -5043,6 +5043,11 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
         return;
     }

+    if (!(bs->open_flags & BDRV_O_INCOMING)) {
+        return;
+    }
+    bs->open_flags &= ~BDRV_O_INCOMING;
+
     if (bs->drv->bdrv_invalidate_cache) {
         bs->drv->bdrv_invalidate_cache(bs, &local_err);
     } else if (bs->file) {
@@ -5078,19 +5083,6 @@ void bdrv_invalidate_cache_all(Error **errp)
     }
 }

-void bdrv_clear_incoming_migration_all(void)
-{
-    BlockDriverState *bs;
-
-    QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
-        AioContext *aio_context = bdrv_get_aio_context(bs);
-
-        aio_context_acquire(aio_context);
-        bs->open_flags = bs->open_flags & ~(BDRV_O_INCOMING);
-        aio_context_release(aio_context);
-    }
-}
-
 int bdrv_flush(BlockDriverState *bs)
 {
     Coroutine *co;
diff --git a/migration.c b/migration.c
index 8d675b3..c49a05a 100644
--- a/migration.c
+++ b/migration.c
@@ -103,7 +103,6 @@ static void process_incoming_migration_co(void *opaque)
     }
     qemu_announce_self();

-    bdrv_clear_incoming_migration_all();
     /* Make sure all file formats flush their mutable metadata */
     bdrv_invalidate_cache_all(&local_err);
     if (local_err) {
diff --git a/nbd.c b/nbd.c
index e9b539b..a7bce45 100644
--- a/nbd.c
+++ b/nbd.c
@@ -972,6 +972,12 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset,
     exp->ctx = bdrv_get_aio_context(bs);
     bdrv_ref(bs);
     bdrv_add_aio_context_notifier(bs, bs_aio_attached, bs_aio_detach, exp);
+    /*
+     * NBD exports are used for non-shared storage migration.  Make sure
+     * that BDRV_O_INCOMING is cleared and the image is ready for write
+     * access since the export could be available before migration handover.
+     */
+    bdrv_invalidate_cache(bs, NULL);
     return exp;
 }

-- 
2.1.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 04/10] vmstate: Allow dynamic allocation for VBUFFER during migration
  2014-10-15  8:24 [Qemu-devel] [PULL 00/10] Migration pull request Juan Quintela
                   ` (2 preceding siblings ...)
  2014-10-15  8:24 ` [Qemu-devel] [PULL 03/10] block/migration: Disable cache invalidate for incoming migration Juan Quintela
@ 2014-10-15  8:24 ` Juan Quintela
  2014-10-15  8:24 ` [Qemu-devel] [PULL 05/10] qemu-file: Add copyright header to qemu-file.c Juan Quintela
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Juan Quintela @ 2014-10-15  8:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy

From: Alexey Kardashevskiy <aik@ozlabs.ru>

This extends use of VMS_ALLOC flag from arrays to VBUFFER as well.

This defines VMSTATE_VBUFFER_ALLOC_UINT32 which makes use of VMS_ALLOC
and uses uint32_t type for a size.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 include/migration/vmstate.h | 11 +++++++++++
 vmstate.c                   | 13 ++++++++++---
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 9a001bd..e45fc49 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -484,6 +484,17 @@ extern const VMStateInfo vmstate_info_bitmap;
     .start        = (_start),                                        \
 }

+#define VMSTATE_VBUFFER_ALLOC_UINT32(_field, _state, _version, _test, _start, _field_size) { \
+    .name         = (stringify(_field)),                             \
+    .version_id   = (_version),                                      \
+    .field_exists = (_test),                                         \
+    .size_offset  = vmstate_offset_value(_state, _field_size, uint32_t),\
+    .info         = &vmstate_info_buffer,                            \
+    .flags        = VMS_VBUFFER|VMS_POINTER|VMS_ALLOC,               \
+    .offset       = offsetof(_state, _field),                        \
+    .start        = (_start),                                        \
+}
+
 #define VMSTATE_BUFFER_UNSAFE_INFO(_field, _state, _version, _info, _size) { \
     .name       = (stringify(_field)),                               \
     .version_id = (_version),                                        \
diff --git a/vmstate.c b/vmstate.c
index ef2f87b..3dde574 100644
--- a/vmstate.c
+++ b/vmstate.c
@@ -49,9 +49,16 @@ static void *vmstate_base_addr(void *opaque, VMStateField *field, bool alloc)

     if (field->flags & VMS_POINTER) {
         if (alloc && (field->flags & VMS_ALLOC)) {
-            int n_elems = vmstate_n_elems(opaque, field);
-            if (n_elems) {
-                gsize size = n_elems * field->size;
+            gsize size = 0;
+            if (field->flags & VMS_VBUFFER) {
+                size = vmstate_size(opaque, field);
+            } else {
+                int n_elems = vmstate_n_elems(opaque, field);
+                if (n_elems) {
+                    size = n_elems * field->size;
+                }
+            }
+            if (size) {
                 *((void **)base_addr + field->start) = g_malloc(size);
             }
         }
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 05/10] qemu-file: Add copyright header to qemu-file.c
  2014-10-15  8:24 [Qemu-devel] [PULL 00/10] Migration pull request Juan Quintela
                   ` (3 preceding siblings ...)
  2014-10-15  8:24 ` [Qemu-devel] [PULL 04/10] vmstate: Allow dynamic allocation for VBUFFER during migration Juan Quintela
@ 2014-10-15  8:24 ` Juan Quintela
  2014-10-15  8:25 ` [Qemu-devel] [PULL 06/10] qemu-file: Make qemu_file_is_writable() non-static Juan Quintela
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Juan Quintela @ 2014-10-15  8:24 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alexey Kardashevskiy, Markus Armbruster, Eduardo Habkost,
	Dr. David Alan Gilbert

From: Eduardo Habkost <ehabkost@redhat.com>

The person who created qemu-file.c (me, on commit
093c455a8c6d8f715eabd8c8d346f08f17d686ec) didn't add a copyright/license
header to the file, even though the whole code was copied from savevm.c
(which had a copyright/license header).

To correct this, copy the copyright information and license from
savevm.c, that's where the original code came from.

Luckily, very few changes were made on qemu-file.c after it was created.
All the authors who touched the code are being CCed, so they can confirm
if they are OK with the copyright/license information being added.

Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Alexey Kardashevskiy <aik@ozlabs.ru>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Juan Quintela <quintela@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 qemu-file.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/qemu-file.c b/qemu-file.c
index cdda745..a5bbe7e 100644
--- a/qemu-file.c
+++ b/qemu-file.c
@@ -1,3 +1,26 @@
+/*
+ * 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"
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 06/10] qemu-file: Make qemu_file_is_writable() non-static
  2014-10-15  8:24 [Qemu-devel] [PULL 00/10] Migration pull request Juan Quintela
                   ` (4 preceding siblings ...)
  2014-10-15  8:24 ` [Qemu-devel] [PULL 05/10] qemu-file: Add copyright header to qemu-file.c Juan Quintela
@ 2014-10-15  8:25 ` Juan Quintela
  2014-10-15  8:25 ` [Qemu-devel] [PULL 07/10] qemu-file: Use qemu_file_is_writable() on stdio_fclose() Juan Quintela
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Juan Quintela @ 2014-10-15  8:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Eduardo Habkost

From: Eduardo Habkost <ehabkost@redhat.com>

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>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Juan Quintela <quintela@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 70ff347..401676b 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -120,6 +120,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);

 QEMUSizedBuffer *qsb_create(const uint8_t *buffer, size_t len);
 QEMUSizedBuffer *qsb_clone(const QEMUSizedBuffer *);
diff --git a/qemu-file.c b/qemu-file.c
index a5bbe7e..c303b61 100644
--- a/qemu-file.c
+++ b/qemu-file.c
@@ -456,7 +456,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;
 }
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 07/10] qemu-file: Use qemu_file_is_writable() on stdio_fclose()
  2014-10-15  8:24 [Qemu-devel] [PULL 00/10] Migration pull request Juan Quintela
                   ` (5 preceding siblings ...)
  2014-10-15  8:25 ` [Qemu-devel] [PULL 06/10] qemu-file: Make qemu_file_is_writable() non-static Juan Quintela
@ 2014-10-15  8:25 ` Juan Quintela
  2014-10-15  8:25 ` [Qemu-devel] [PULL 08/10] qemu-file: Move unix and socket implementations to qemu-file-unix.c Juan Quintela
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Juan Quintela @ 2014-10-15  8:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Eduardo Habkost

From: Eduardo Habkost <ehabkost@redhat.com>

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>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 qemu-file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-file.c b/qemu-file.c
index c303b61..9cb1223 100644
--- a/qemu-file.c
+++ b/qemu-file.c
@@ -175,7 +175,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;

-- 
2.1.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 08/10] qemu-file: Move unix and socket implementations to qemu-file-unix.c
  2014-10-15  8:24 [Qemu-devel] [PULL 00/10] Migration pull request Juan Quintela
                   ` (6 preceding siblings ...)
  2014-10-15  8:25 ` [Qemu-devel] [PULL 07/10] qemu-file: Use qemu_file_is_writable() on stdio_fclose() Juan Quintela
@ 2014-10-15  8:25 ` Juan Quintela
  2014-10-15  8:25 ` [Qemu-devel] [PULL 09/10] qemu-file: Move stdio implementation to qemu-file-stdio.c Juan Quintela
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Juan Quintela @ 2014-10-15  8:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Eduardo Habkost

From: Eduardo Habkost <ehabkost@redhat.com>

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>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Juan Quintela <quintela@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 add8375..957e56c 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 9cb1223..09488a0 100644
--- a/qemu-file.c
+++ b/qemu-file.c
@@ -56,63 +56,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;
@@ -250,125 +193,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 ||
@@ -381,25 +205,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 31e867f..ba8c6b5 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -258,7 +258,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 libqemustub.a

 tests/test-qapi-types.c tests/test-qapi-types.h :\
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 09/10] qemu-file: Move stdio implementation to qemu-file-stdio.c
  2014-10-15  8:24 [Qemu-devel] [PULL 00/10] Migration pull request Juan Quintela
                   ` (7 preceding siblings ...)
  2014-10-15  8:25 ` [Qemu-devel] [PULL 08/10] qemu-file: Move unix and socket implementations to qemu-file-unix.c Juan Quintela
@ 2014-10-15  8:25 ` Juan Quintela
  2014-10-15  8:25 ` [Qemu-devel] [PULL 10/10] migration: catch unknown flag combinations in ram_load Juan Quintela
  2014-10-15 12:37 ` [Qemu-devel] [PULL 00/10] Migration pull request Peter Maydell
  10 siblings, 0 replies; 12+ messages in thread
From: Juan Quintela @ 2014-10-15  8:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Eduardo Habkost, Markus Armbruster

From: Eduardo Habkost <ehabkost@redhat.com>

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>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Juan Quintela <quintela@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 957e56c..18fd35c 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 09488a0..f938e36 100644
--- a/qemu-file.c
+++ b/qemu-file.c
@@ -51,148 +51,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 ||
@@ -205,32 +63,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;
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 10/10] migration: catch unknown flag combinations in ram_load
  2014-10-15  8:24 [Qemu-devel] [PULL 00/10] Migration pull request Juan Quintela
                   ` (8 preceding siblings ...)
  2014-10-15  8:25 ` [Qemu-devel] [PULL 09/10] qemu-file: Move stdio implementation to qemu-file-stdio.c Juan Quintela
@ 2014-10-15  8:25 ` Juan Quintela
  2014-10-15 12:37 ` [Qemu-devel] [PULL 00/10] Migration pull request Peter Maydell
  10 siblings, 0 replies; 12+ messages in thread
From: Juan Quintela @ 2014-10-15  8:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Lieven

From: Peter Lieven <pl@kamp.de>

this patch extends commit db80fac by not only checking
for unknown flags, but also filtering out unknown flag
combinations.

Suggested-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Peter Lieven <pl@kamp.de>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 arch_init.c | 62 +++++++++++++++++++++++++++++++------------------------------
 1 file changed, 32 insertions(+), 30 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 9b3e25d..88a5ba0 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -1040,8 +1040,7 @@ void ram_handle_compressed(void *host, uint8_t ch, uint64_t size)

 static int ram_load(QEMUFile *f, void *opaque, int version_id)
 {
-    ram_addr_t addr;
-    int flags, ret = 0;
+    int flags = 0, ret = 0;
     static uint64_t seq_iter;

     seq_iter++;
@@ -1050,21 +1049,24 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
         ret = -EINVAL;
     }

-    while (!ret) {
-        addr = qemu_get_be64(f);
+    while (!ret && !(flags & RAM_SAVE_FLAG_EOS)) {
+        ram_addr_t addr, total_ram_bytes;
+        void *host;
+        uint8_t ch;

+        addr = qemu_get_be64(f);
         flags = addr & ~TARGET_PAGE_MASK;
         addr &= TARGET_PAGE_MASK;

-        if (flags & RAM_SAVE_FLAG_MEM_SIZE) {
+        switch (flags & ~RAM_SAVE_FLAG_CONTINUE) {
+        case RAM_SAVE_FLAG_MEM_SIZE:
             /* Synchronize RAM block list */
-            char id[256];
-            ram_addr_t length;
-            ram_addr_t total_ram_bytes = addr;
-
-            while (total_ram_bytes) {
+            total_ram_bytes = addr;
+            while (!ret && total_ram_bytes) {
                 RAMBlock *block;
                 uint8_t len;
+                char id[256];
+                ram_addr_t length;

                 len = qemu_get_byte(f);
                 qemu_get_buffer(f, (uint8_t *)id, len);
@@ -1088,16 +1090,11 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
                                  "accept migration", id);
                     ret = -EINVAL;
                 }
-                if (ret) {
-                    break;
-                }

                 total_ram_bytes -= length;
             }
-        } else if (flags & RAM_SAVE_FLAG_COMPRESS) {
-            void *host;
-            uint8_t ch;
-
+            break;
+        case RAM_SAVE_FLAG_COMPRESS:
             host = host_from_stream_offset(f, addr, flags);
             if (!host) {
                 error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
@@ -1107,9 +1104,8 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)

             ch = qemu_get_byte(f);
             ram_handle_compressed(host, ch, TARGET_PAGE_SIZE);
-        } else if (flags & RAM_SAVE_FLAG_PAGE) {
-            void *host;
-
+            break;
+        case RAM_SAVE_FLAG_PAGE:
             host = host_from_stream_offset(f, addr, flags);
             if (!host) {
                 error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
@@ -1118,8 +1114,9 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
             }

             qemu_get_buffer(f, host, TARGET_PAGE_SIZE);
-        } else if (flags & RAM_SAVE_FLAG_XBZRLE) {
-            void *host = host_from_stream_offset(f, addr, flags);
+            break;
+        case RAM_SAVE_FLAG_XBZRLE:
+            host = host_from_stream_offset(f, addr, flags);
             if (!host) {
                 error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
                 ret = -EINVAL;
@@ -1132,17 +1129,22 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
                 ret = -EINVAL;
                 break;
             }
-        } else if (flags & RAM_SAVE_FLAG_HOOK) {
-            ram_control_load_hook(f, flags);
-        } else if (flags & RAM_SAVE_FLAG_EOS) {
-            /* normal exit */
             break;
-        } else {
-            error_report("Unknown migration flags: %#x", flags);
-            ret = -EINVAL;
+        case RAM_SAVE_FLAG_EOS:
+            /* normal exit */
             break;
+        default:
+            if (flags & RAM_SAVE_FLAG_HOOK) {
+                ram_control_load_hook(f, flags);
+            } else {
+                error_report("Unknown combination of migration flags: %#x",
+                             flags);
+                ret = -EINVAL;
+            }
+        }
+        if (!ret) {
+            ret = qemu_file_get_error(f);
         }
-        ret = qemu_file_get_error(f);
     }

     DPRINTF("Completed load of VM with exit code %d seq iteration "
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PULL 00/10] Migration pull request
  2014-10-15  8:24 [Qemu-devel] [PULL 00/10] Migration pull request Juan Quintela
                   ` (9 preceding siblings ...)
  2014-10-15  8:25 ` [Qemu-devel] [PULL 10/10] migration: catch unknown flag combinations in ram_load Juan Quintela
@ 2014-10-15 12:37 ` Peter Maydell
  10 siblings, 0 replies; 12+ messages in thread
From: Peter Maydell @ 2014-10-15 12:37 UTC (permalink / raw)
  To: Juan Quintela; +Cc: QEMU Developers, Juan Quintela

On 15 October 2014 10:24, Juan Quintela <quintela@redhat.com> wrote:
> From: Juan Quintela <quintela@trasno.org>
>
> Hi
>
> This patches have been on the list for a while:
>
> - ram flags from Peter Lieven
> - qemu-file cleanups from Eduardo (just code movement)
> - QEMUSizedBuffer from David (they have already have 6 versions)
> - VBUFFER (new macro, can't break old things) (Alexey)
> - invalidate state (Alexey)
>
> Please, apply.
>
> The following changes since commit b1d28ec6a7dbdaadda39d29322f0de694aeb0b74:
>
>   Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20141010' into staging (2014-10-10 14:55:29 +0100)
>
> are available in the git repository at:
>
>   git://github.com/juanquintela/qemu.git migration/20141015
>
> for you to fetch changes up to 5b0e9dd46fbda5152566a4a26fd96bc0d0452bf7:
>
>   migration: catch unknown flag combinations in ram_load (2014-10-14 11:24:20 +0200)
>

Applied, thanks.

-- PMM

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2014-10-15 12:37 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-15  8:24 [Qemu-devel] [PULL 00/10] Migration pull request Juan Quintela
2014-10-15  8:24 ` [Qemu-devel] [PULL 01/10] QEMUSizedBuffer based QEMUFile Juan Quintela
2014-10-15  8:24 ` [Qemu-devel] [PULL 02/10] Tests: QEMUSizedBuffer/QEMUBuffer Juan Quintela
2014-10-15  8:24 ` [Qemu-devel] [PULL 03/10] block/migration: Disable cache invalidate for incoming migration Juan Quintela
2014-10-15  8:24 ` [Qemu-devel] [PULL 04/10] vmstate: Allow dynamic allocation for VBUFFER during migration Juan Quintela
2014-10-15  8:24 ` [Qemu-devel] [PULL 05/10] qemu-file: Add copyright header to qemu-file.c Juan Quintela
2014-10-15  8:25 ` [Qemu-devel] [PULL 06/10] qemu-file: Make qemu_file_is_writable() non-static Juan Quintela
2014-10-15  8:25 ` [Qemu-devel] [PULL 07/10] qemu-file: Use qemu_file_is_writable() on stdio_fclose() Juan Quintela
2014-10-15  8:25 ` [Qemu-devel] [PULL 08/10] qemu-file: Move unix and socket implementations to qemu-file-unix.c Juan Quintela
2014-10-15  8:25 ` [Qemu-devel] [PULL 09/10] qemu-file: Move stdio implementation to qemu-file-stdio.c Juan Quintela
2014-10-15  8:25 ` [Qemu-devel] [PULL 10/10] migration: catch unknown flag combinations in ram_load Juan Quintela
2014-10-15 12:37 ` [Qemu-devel] [PULL 00/10] Migration pull request Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).