[PATCH v5 0/2] In memory QEMUFile

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH v5 0/2] In memory QEMUFile

Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[hidden email]>

  This patch-pair adds the QEMUSizedBuffer based in-memory QEMUFile
written by Stefan Berger and Joel Schopp.  I've made some
fixes and modified the existing test-vmstate to use it for some test cases.

  While there's nothing other than test cases using it yet, I think
it's worth going in by itself, since I'm using it in two separate
patchsets (postcopy and visitor/BER) and Sanidhya uses it in
the periodic vmstate test world.  In addition both microcheckpointing and
COLO have similar but independent implementations (although they both
have some extra-gotcha's so it might not be possible to reuse it), and
there was another implementation of the same thing in the Yabusame Postcopy
world.  Thus it seems best to put in, if only to stop people writing yet
another implementation.

Dave

v5:
  Fixes from Eric's comments; including a memory leak in an error path

v4:
  Fix very silly mistake in qsb_grow ENOMEM check

v3:
  Mostly addressing comments from Eric and Gonglei

  rewrote qsb_grow to always use _try_ for memory allocation
    also made it use MAX_CHUNK_SIZE as needed
  made QSB_MAX_CHUNK_SIZE 16x rather than 10x CHUNK_SIZE

  reworked qsb_get_buffer to always take an allocated buffer

  qsb_create:
     Cope with 0 length but passed buffer
     Rework so it can fail on memory allocation failures

  qsb_write_at
     return -EINVAL for invalid position
  qsb_clone
     Cope with qsb_write_at failing

  Typo: 'of of' in comment
        'withing'
  Reword qsb_clone 'exact copy'->'deep copy'

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

 include/migration/qemu-file.h |  28 +++
 include/qemu/typedefs.h       |   1 +
 qemu-file.c                   | 456 ++++++++++++++++++++++++++++++++++++++++++
 tests/Makefile                |   2 +-
 tests/test-vmstate.c          |  74 +++----
 5 files changed, 524 insertions(+), 37 deletions(-)

--
1.9.3


Reply | Threaded
Open this post in threaded view
|

[PATCH v5 1/2] QEMUSizedBuffer based QEMUFile

Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[hidden email]>

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 <[hidden email]>
Signed-off-by: Joel Schopp <[hidden email]>

For fixes/tweeks I've done:
Signed-off-by: Dr. David Alan Gilbert <[hidden email]>

Reviewed-by: Eric Blake <[hidden email]>
---
 include/migration/qemu-file.h |  28 +++
 include/qemu/typedefs.h       |   1 +
 qemu-file.c                   | 456 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 485 insertions(+)

diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index c90f529..6ef8ebc 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,21 @@ 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 */
+};
+
+typedef struct QEMUSizedBuffer QEMUSizedBuffer;
+
 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 +123,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 5f20b0e..db1153a 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -60,6 +60,7 @@ typedef struct PCIEAERLog PCIEAERLog;
 typedef struct PCIEAERErr PCIEAERErr;
 typedef struct PCIEPort PCIEPort;
 typedef struct PCIESlot PCIESlot;
+typedef struct QEMUSizedBuffer QEMUSizedBuffer;
 typedef struct MSIMessage MSIMessage;
 typedef struct SerialState SerialState;
 typedef struct PCMCIACardState PCMCIACardState;
diff --git a/qemu-file.c b/qemu-file.c
index a8e3912..ccc516c 100644
--- a/qemu-file.c
+++ b/qemu-file.c
@@ -878,3 +878,459 @@ 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_malloc_n(qsb->n_iov + needed_chunks,
+                                 sizeof(struct iovec));
+        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;
+}
--
1.9.3


Reply | Threaded
Open this post in threaded view
|

[PATCH v5 2/2] Tests: QEMUSizedBuffer/QEMUBuffer

Dr. David Alan Gilbert (git)
In reply to this post by Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[hidden email]>

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 <[hidden email]>
Reviewed-by: Eric Blake <[hidden email]>
---
 tests/Makefile       |  2 +-
 tests/test-vmstate.c | 74 +++++++++++++++++++++++++++-------------------------
 2 files changed, 39 insertions(+), 37 deletions(-)

diff --git a/tests/Makefile b/tests/Makefile
index f5de29c..0ad7ac5 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));
--
1.9.3


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v5 0/2] In memory QEMUFile

Eric Blake
In reply to this post by Dr. David Alan Gilbert (git)
On 09/29/2014 03:41 AM, Dr. David Alan Gilbert (git) wrote:

> From: "Dr. David Alan Gilbert" <[hidden email]>
>
>   This patch-pair adds the QEMUSizedBuffer based in-memory QEMUFile
> written by Stefan Berger and Joel Schopp.  I've made some
> fixes and modified the existing test-vmstate to use it for some test cases.
>
>   While there's nothing other than test cases using it yet, I think
> it's worth going in by itself, since I'm using it in two separate
> patchsets (postcopy and visitor/BER) and Sanidhya uses it in
> the periodic vmstate test world.  In addition both microcheckpointing and
> COLO have similar but independent implementations (although they both
> have some extra-gotcha's so it might not be possible to reuse it), and
> there was another implementation of the same thing in the Yabusame Postcopy
> world.  Thus it seems best to put in, if only to stop people writing yet
> another implementation.
>
> Dave
>
> v5:
>   Fixes from Eric's comments; including a memory leak in an error path
Looks correct to me, so my R-b is valid.

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


signature.asc (551 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v5 1/2] QEMUSizedBuffer based QEMUFile

Hailiang Zhang
In reply to this post by Dr. David Alan Gilbert (git)
On 2014/9/29 17:41, Dr. David Alan Gilbert (git) wrote:

> From: "Dr. David Alan Gilbert" <[hidden email]>
>
> 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 <[hidden email]>
> Signed-off-by: Joel Schopp <[hidden email]>
>
> For fixes/tweeks I've done:
> Signed-off-by: Dr. David Alan Gilbert <[hidden email]>
>
> Reviewed-by: Eric Blake <[hidden email]>
> ---
>   include/migration/qemu-file.h |  28 +++
>   include/qemu/typedefs.h       |   1 +
>   qemu-file.c                   | 456 ++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 485 insertions(+)
>
> diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
> index c90f529..6ef8ebc 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,21 @@ 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 */
> +};
> +
> +typedef struct QEMUSizedBuffer QEMUSizedBuffer;
> +
>   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 +123,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 5f20b0e..db1153a 100644
> --- a/include/qemu/typedefs.h
> +++ b/include/qemu/typedefs.h
> @@ -60,6 +60,7 @@ typedef struct PCIEAERLog PCIEAERLog;
>   typedef struct PCIEAERErr PCIEAERErr;
>   typedef struct PCIEPort PCIEPort;
>   typedef struct PCIESlot PCIESlot;
> +typedef struct QEMUSizedBuffer QEMUSizedBuffer;
>   typedef struct MSIMessage MSIMessage;
>   typedef struct SerialState SerialState;
>   typedef struct PCMCIACardState PCMCIACardState;
> diff --git a/qemu-file.c b/qemu-file.c
> index a8e3912..ccc516c 100644
> --- a/qemu-file.c
> +++ b/qemu-file.c
> @@ -878,3 +878,459 @@ 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_malloc_n(qsb->n_iov + needed_chunks,
> +                                 sizeof(struct iovec));

It seems that *g_try_malloc_n* was supported since glib2-2.24 version,
But it don't check this when do *configure* before compile...;)

> +        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;
> +}
>



Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v5 1/2] QEMUSizedBuffer based QEMUFile

Dr. David Alan Gilbert (git)
* zhanghailiang ([hidden email]) wrote:
> On 2014/9/29 17:41, Dr. David Alan Gilbert (git) wrote:

> >+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_malloc_n(qsb->n_iov + needed_chunks,
> >+                                 sizeof(struct iovec));
>
> It seems that *g_try_malloc_n* was supported since glib2-2.24 version,
> But it don't check this when do *configure* before compile...;)

OK, that's a shame - it was a nice easy function to use :-)
I'll fix it.

Dave

--
Dr. David Alan Gilbert / [hidden email] / Manchester, UK

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v5 1/2] QEMUSizedBuffer based QEMUFile

Markus Armbruster
"Dr. David Alan Gilbert" <[hidden email]> writes:

> * zhanghailiang ([hidden email]) wrote:
>> On 2014/9/29 17:41, Dr. David Alan Gilbert (git) wrote:
>
>> >+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_malloc_n(qsb->n_iov + needed_chunks,
>> >+                                 sizeof(struct iovec));
>>
>> It seems that *g_try_malloc_n* was supported since glib2-2.24 version,
>> But it don't check this when do *configure* before compile...;)
>
> OK, that's a shame - it was a nice easy function to use :-)
> I'll fix it.

See also commit 02c4f26.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v5 1/2] QEMUSizedBuffer based QEMUFile

Dr. David Alan Gilbert (git)
In reply to this post by Hailiang Zhang
* zhanghailiang ([hidden email]) wrote:

> >+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_malloc_n(qsb->n_iov + needed_chunks,
> >+                                 sizeof(struct iovec));
>
> It seems that *g_try_malloc_n* was supported since glib2-2.24 version,
> But it don't check this when do *configure* before compile...;)

        new_iov = g_try_new(struct iovec, qsb->n_iov + needed_chunks);

seems to work for me; let me know if you hit any others.

Dave



>
> >+        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;
> >+}
> >
>
>
--
Dr. David Alan Gilbert / [hidden email] / Manchester, UK

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v5 1/2] QEMUSizedBuffer based QEMUFile

Hailiang Zhang
On 2014/10/8 17:08, Dr. David Alan Gilbert wrote:

> * zhanghailiang ([hidden email]) wrote:
>
>>> +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_malloc_n(qsb->n_iov + needed_chunks,
>>> +                                 sizeof(struct iovec));
>>
>> It seems that *g_try_malloc_n* was supported since glib2-2.24 version,
>> But it don't check this when do *configure* before compile...;)
>
>          new_iov = g_try_new(struct iovec, qsb->n_iov + needed_chunks);
>
> seems to work for me; let me know if you hit any others.
>

;), This also works for me. Thanks.

> Dave
>
>
>
>>
>>> +        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;
>>> +}
>>>
>>
>>
> --
> Dr. David Alan Gilbert / [hidden email] / Manchester, UK
>
> .
>