All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [RFC PATCH v3 0/6] VMState testing
@ 2014-08-09  6:26 Sanidhya Kashyap
  2014-08-09  6:26 ` [Qemu-devel] [RFC PATCH v3 1/6] QEMUSizedBuffer/QEMUFile Sanidhya Kashyap
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Sanidhya Kashyap @ 2014-08-09  6:26 UTC (permalink / raw)
  To: qemu list; +Cc: Sanidhya Kashyap, Dr. David Alan Gilbert, Juan Quintela

Hi,

The following patch introduce a mechanism to test the correctness of the
vmstate's information. This is achieved by saving the device states'
information to a memory buffer and then clearing the states, followed by
loading the data from the saved memory buffer.

v2 --> v3

* Made the devices list generic by removing the qdevified format. But, it only
  supports the qdevified devices.
* Extended the SaveStateEntry for the VMstate testing purpose.
* Replaced DPRINTF with trace_##name functions.
* Squashed all of the hmp and qmp patches into one.
* Rectified some mistakes in the documentation of the hmp and qmp interface
  explanations (the ones which I have introduced).
* Now, I am relying only on the devices that have been qdevified. I am not using
  the qemu_system_reset() function for resetting the devices.

v1 --> v2:

* Added a list containing all the devices that have been qdevified and gets
  registered with the SaveStateEntry.
* Have provided a way to use either qemu_system_reset functionality or my
  own version of qdev entries untill all the devices have been qdevified.
  This gives us the privilege to test any device we want.
* Rename some of the variables. I am very bad at naming convention, thanks to
  community, specially Eric, I try to improve it with every version.
* On Eric's advice, I have separated all of the qmp and hmp interface patches.
* Changed the DPRINTF statements as required.


Dr. David Alan Gilbert (1):
  QEMUSizedBuffer/QEMUFile

Sanidhya Kashyap (5):
  VMState test: get information about the registered devices
  VMstate test: basic VMState testing mechanism
  VMState test: querying the vmstate testing process
  VMState test: update period of vmstate testing process
  VMState test: cancel mechanism for an already running vmstate testing
    process

 hmp-commands.hx               |  49 +++++
 hmp.c                         |  90 +++++++++
 hmp.h                         |   5 +
 include/migration/qemu-file.h |  27 +++
 monitor.c                     |  14 ++
 qapi-schema.json              | 106 +++++++++++
 qemu-file.c                   | 411 ++++++++++++++++++++++++++++++++++++++++++
 qmp-commands.hx               | 145 +++++++++++++++
 savevm.c                      | 332 ++++++++++++++++++++++++++++++++++
 trace-events                  |   2 +
 10 files changed, 1181 insertions(+)

-- 
1.9.3

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

* [Qemu-devel] [RFC PATCH v3 1/6] QEMUSizedBuffer/QEMUFile
  2014-08-09  6:26 [Qemu-devel] [RFC PATCH v3 0/6] VMState testing Sanidhya Kashyap
@ 2014-08-09  6:26 ` Sanidhya Kashyap
  2014-08-11  2:49   ` Gonglei (Arei)
  2014-08-11 16:38   ` Eric Blake
  2014-08-09  6:26 ` [Qemu-devel] [RFC PATCH v3 2/6] VMState test: get information about the registered devices Sanidhya Kashyap
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 14+ messages in thread
From: Sanidhya Kashyap @ 2014-08-09  6:26 UTC (permalink / raw)
  To: qemu list
  Cc: Sanidhya Kashyap, Joel Schopp, Stefan Berger,
	Dr. David Alan Gilbert, Juan Quintela

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

Stefan Berger's to create 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>
Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
---
 include/migration/qemu-file.h |  27 +++
 qemu-file.c                   | 411 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 438 insertions(+)

diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index c90f529..14e1f4d 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,31 @@ 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;
+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);
+
+
 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 +133,11 @@ 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);
 
+/*
+ * 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/qemu-file.c b/qemu-file.c
index a8e3912..9fd1675 100644
--- a/qemu-file.c
+++ b/qemu-file.c
@@ -878,3 +878,414 @@ 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  (10 * 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
+ */
+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;
+
+    if (len == 0) {
+        /* we want to allocate at least one chunk */
+        len = QSB_CHUNK_SIZE;
+    }
+
+    num_chunks = DIV_ROUND_UP(len, chunk_size);
+    alloc_len = num_chunks * chunk_size;
+
+    qsb = g_new0(QEMUSizedBuffer, 1);
+    qsb->iov = g_new0(struct iovec, num_chunks);
+    qsb->n_iov = num_chunks;
+
+    for (i = 0; i < num_chunks; i++) {
+        qsb->iov[i].iov_base = g_malloc0(chunk_size);
+        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 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 trucated 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 an optional buffer to write into; the pointer may
+ *       point to NULL in which case the buffer will be allocated;
+ *       if buffer is provided, it must be large enough to hold @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 **buf)
+{
+    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;
+    }
+
+    if (*buf == NULL) {
+        *buf = g_malloc(all_copy);
+    }
+    buffer = *buf;
+
+    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 allocated
+ * memory for it.
+ *
+ * @qsb: A QEMUSizedBuffer
+ * @new_size: The new size of the buffer
+ *
+ * Returns an error code in case of memory allocation failure
+ * or the new size of the buffer otherwise. 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;
+    size_t chunk_size = QSB_CHUNK_SIZE;
+
+    if (qsb->size < new_size) {
+        needed_chunks = DIV_ROUND_UP(new_size - qsb->size,
+                                     chunk_size);
+
+        qsb->iov = g_realloc_n(qsb->iov, qsb->n_iov + needed_chunks,
+                               sizeof(struct iovec));
+        if (qsb->iov == NULL) {
+            return -ENOMEM;
+        }
+
+        for (i = qsb->n_iov; i < qsb->n_iov + needed_chunks; i++) {
+            qsb->iov[i].iov_base = g_malloc0(chunk_size);
+            qsb->iov[i].iov_len = chunk_size;
+        }
+
+        qsb->n_iov += needed_chunks;
+        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 withing the @qsb to write data to
+ * @size: The number of bytes to copy into the @qsb
+ *
+ * Returns an error code in case of memory allocation failure,
+ * @size otherwise.
+ */
+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 0;
+    }
+
+    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 an exact copy of the given QEMUSizedBuffer.
+ *
+ * @qsb : A QEMUSizedBuffer
+ *
+ * Returns a clone of @qsb
+ */
+QEMUSizedBuffer *qsb_clone(const QEMUSizedBuffer *qsb)
+{
+    QEMUSizedBuffer *out = qsb_create(NULL, qsb_get_length(qsb));
+    size_t i;
+    off_t pos = 0;
+
+    for (i = 0; i < qsb->n_iov; i++) {
+        pos += qsb_write_at(out, qsb->iov[i].iov_base,
+                            pos, qsb->iov[i].iov_len);
+    }
+
+    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 = (QEMUBuffer *)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) {
+        fprintf(stderr, "qemu_bufopen: Argument validity check failed\n");
+        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 (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

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

* [Qemu-devel] [RFC PATCH v3 2/6] VMState test: get information about the registered devices
  2014-08-09  6:26 [Qemu-devel] [RFC PATCH v3 0/6] VMState testing Sanidhya Kashyap
  2014-08-09  6:26 ` [Qemu-devel] [RFC PATCH v3 1/6] QEMUSizedBuffer/QEMUFile Sanidhya Kashyap
@ 2014-08-09  6:26 ` Sanidhya Kashyap
  2014-08-11 16:24   ` Eric Blake
  2014-08-09  6:26 ` [Qemu-devel] [RFC PATCH v3 3/6] VMstate test: basic VMState testing mechanism Sanidhya Kashyap
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Sanidhya Kashyap @ 2014-08-09  6:26 UTC (permalink / raw)
  To: qemu list; +Cc: Sanidhya Kashyap, Dr. David Alan Gilbert, Juan Quintela

Added both qmp and hmp interface to get the information about the devices that
have been qdevified and are registered with the SaveVMHandlers. I have not used 
any format to print the device information for the hmp interface. It would be
great if anyone can give me some pointers about this about the printing format
if there is something.

The qmp command to extract the information about the devices is
qmp_query_devices which provides the list of device names and their respective
version. The hmp command name is 'info devices' which lists the same information
as provided by the qmp interface.

Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
---
 hmp-commands.hx  |  2 ++
 hmp.c            | 23 +++++++++++++++++++++++
 hmp.h            |  1 +
 monitor.c        |  7 +++++++
 qapi-schema.json | 26 ++++++++++++++++++++++++++
 qmp-commands.hx  | 43 +++++++++++++++++++++++++++++++++++++++++++
 savevm.c         | 45 +++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 147 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index d0943b1..a221459 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1780,6 +1780,8 @@ show qdev device model list
 show roms
 @item info tpm
 show the TPM device
+@item info devices
+show the devices registered with migration capability
 @end table
 ETEXI
 
diff --git a/hmp.c b/hmp.c
index 4d1838e..6e1802a 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1714,3 +1714,26 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
 
     monitor_printf(mon, "\n");
 }
+
+void hmp_info_devices(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+    QemuDeviceList *device_list = qmp_query_devices(&err);
+
+    if (device_list) {
+        monitor_printf(mon, "devices:\n");
+        while (device_list) {
+            monitor_printf(mon, "device-name: %s, ",
+                           device_list->value->device_name);
+            monitor_printf(mon, "version: %ld\n",
+                           device_list->value->version);
+            device_list = device_list->next;
+        }
+    }
+
+    if (err) {
+        hmp_handle_error(mon, &err);
+    }
+
+    qapi_free_QemuDeviceList(device_list);
+}
diff --git a/hmp.h b/hmp.h
index 4fd3c4a..9d6b577 100644
--- a/hmp.h
+++ b/hmp.h
@@ -38,6 +38,7 @@ void hmp_info_balloon(Monitor *mon, const QDict *qdict);
 void hmp_info_pci(Monitor *mon, const QDict *qdict);
 void hmp_info_block_jobs(Monitor *mon, const QDict *qdict);
 void hmp_info_tpm(Monitor *mon, const QDict *qdict);
+void hmp_info_devices(Monitor *mon, const QDict *qdict);
 void hmp_quit(Monitor *mon, const QDict *qdict);
 void hmp_stop(Monitor *mon, const QDict *qdict);
 void hmp_system_reset(Monitor *mon, const QDict *qdict);
diff --git a/monitor.c b/monitor.c
index 5bc70a6..2431686 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2918,6 +2918,13 @@ static mon_cmd_t info_cmds[] = {
         .mhandler.cmd = hmp_info_memdev,
     },
     {
+        .name       = "devices",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show registered devices",
+        .mhandler.cmd = hmp_info_devices,
+    },
+    {
         .name       = NULL,
     },
 };
diff --git a/qapi-schema.json b/qapi-schema.json
index b11aad2..74d26fe 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3480,3 +3480,29 @@
 # Since: 2.1
 ##
 { 'command': 'rtc-reset-reinjection' }
+
+##
+# @QemuDevice
+#
+# device that is registered with SaveVMHandlers
+#
+# @device-name: name of the device that is registered with
+# SaveVMHandlers (in vmsd structure).
+#
+# @version: version of the device.
+#
+# Since 2.2
+##
+{ 'type': 'QemuDevice',
+  'data': { 'device-name':  'str',
+            '*version':     'int' } }
+
+##
+# @query-devices
+#
+# returns the list of QemuDevice
+#
+# Since 2.2
+##
+{ 'command': 'query-devices',
+  'returns': [ 'QemuDevice' ] }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 4be4765..e489197 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3755,3 +3755,46 @@ Example:
 <- { "return": {} }
 
 EQMP
+
+    {
+        .name       = "query-devices",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_query_devices,
+    },
+
+SQMP
+query-devices
+-------------
+
+Shows registered devices along with the version
+
+Example (1):
+
+-> { "execute": "query-devices" }
+<- { "return": [
+       {
+           [ { 'device-name': 'kvm-tpr-opt', 'version': 1 },
+             { 'device-name': 'apic', 'version': 3 },
+             { 'device-name': 'kvmclock', 'version': 1 },
+             { 'device-name': 'fw_cfg', 'version': 2 },
+             { 'device-name': 'I440FX', 'version': 3 },
+             { 'device-name': 'PIIX3', 'version': 3 },
+             { 'device-name': 'i8259', 'version': 1 },
+             { 'device-name': 'i8259', 'version': 1 },
+             { 'device-name': 'ioapic', 'version': 3 },
+             { 'device-name': 'cirrus_vga', 'version': 2 },
+             { 'device-name': 'hpet', 'version': 2 },
+             { 'device-name': 'mc146818rtc', 'version': 3 },
+             { 'device-name': 'i8254', 'version': 3 },
+             { 'device-name': 'serial', 'version': 3 },
+             { 'device-name': 'pckbd', 'version': 3 },
+             { 'device-name': 'vmmouse', 'version': 0 },
+             { 'device-name': 'port92', 'version': 1 },
+             { 'device-name': 'fdc', 'version': 2 },
+             { 'device-name': 'e1000', 'version': 2 },
+             { 'device-name': 'ide', 'version': 3 },
+             { 'device-name': 'piix4_pm', 'version': 3 } ]
+       }
+     ]
+   }
+EQMP
diff --git a/savevm.c b/savevm.c
index e19ae0a..764ca71 100644
--- a/savevm.c
+++ b/savevm.c
@@ -233,6 +233,7 @@ typedef struct SaveStateEntry {
     void *opaque;
     CompatEntry *compat;
     int is_ram;
+    DeviceState *dev;
 } SaveStateEntry;
 
 
@@ -429,6 +430,7 @@ int register_savevm_live(DeviceState *dev,
     se->ops = ops;
     se->opaque = opaque;
     se->vmsd = NULL;
+    se->dev = dev;
     /* if this is a live_savem then set is_ram */
     if (ops->save_live_setup != NULL) {
         se->is_ram = 1;
@@ -519,6 +521,7 @@ int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
     se->opaque = opaque;
     se->vmsd = vmsd;
     se->alias_id = alias_id;
+    se->dev = dev;
 
     if (dev) {
         char *id = qdev_get_dev_path(dev);
@@ -1137,6 +1140,48 @@ void do_savevm(Monitor *mon, const QDict *qdict)
     }
 }
 
+static QemuDeviceList *create_device_list(const char *name, int version,
+                                           QemuDeviceList *list)
+{
+    QemuDeviceList *temp_list;
+    QemuDeviceList *parent_list = list;
+    int len;
+
+    temp_list = g_malloc0(sizeof(QemuDeviceList));
+    len = strlen(name);
+    temp_list->value = g_malloc0(sizeof(QemuDevice));
+    temp_list->value->device_name = g_malloc0(sizeof(char)*(len+1));
+    strcpy(temp_list->value->device_name, name);
+    temp_list->value->version = version;
+    temp_list->next = NULL;
+
+    if (!list) {
+        return temp_list;
+    }
+
+    while (list->next) {
+        list = list->next;
+    }
+    list->next = temp_list;
+
+    return parent_list;
+}
+
+QemuDeviceList *qmp_query_devices(Error **errp)
+{
+    QemuDeviceList *device_list = NULL;
+    SaveStateEntry *se;
+
+    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
+        if (se->dev) {
+            device_list = create_device_list(se->vmsd->name, se->version_id,
+                                             device_list);
+        }
+    }
+
+    return device_list;
+}
+
 void qmp_xen_save_devices_state(const char *filename, Error **errp)
 {
     QEMUFile *f;
-- 
1.9.3

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

* [Qemu-devel] [RFC PATCH v3 3/6] VMstate test: basic VMState testing mechanism
  2014-08-09  6:26 [Qemu-devel] [RFC PATCH v3 0/6] VMState testing Sanidhya Kashyap
  2014-08-09  6:26 ` [Qemu-devel] [RFC PATCH v3 1/6] QEMUSizedBuffer/QEMUFile Sanidhya Kashyap
  2014-08-09  6:26 ` [Qemu-devel] [RFC PATCH v3 2/6] VMState test: get information about the registered devices Sanidhya Kashyap
@ 2014-08-09  6:26 ` Sanidhya Kashyap
  2014-08-11 16:32   ` Eric Blake
  2014-08-09  6:26 ` [Qemu-devel] [RFC PATCH v3 4/6] VMState test: querying the vmstate testing process Sanidhya Kashyap
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Sanidhya Kashyap @ 2014-08-09  6:26 UTC (permalink / raw)
  To: qemu list; +Cc: Sanidhya Kashyap, Dr. David Alan Gilbert, Juan Quintela

This patch implements the basic way of testing the VMStates' information
whether it is correct or not while saving and loading the states. The qmp
interface - test-vmstates can take three parameters as an input to test
the device states. Now, one can check for any of the devices that have been
registered with the SaveVMHandlers aka the migration protocol. Similarly,
the hmp interface (test_vmstates) has only two input parameters - iterations and
period.

I have removed the following from the patch on previous comments:
* replaed DPRINTF with trace_##name.
* removed the noqdev and completely removed the support for resetting of devices
based on qemu_system_reset()

As Juan pointed out that there might be a memory leak as I did not close the
QEMUFile pointer. But, it is not required as that pointer is directly referenced
by the QEMUBuffer that has been implemented by David. So, IMHO the case pointed
out by Juan does not exist.


Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
---
 hmp-commands.hx  |  16 ++++
 hmp.c            |  17 ++++
 hmp.h            |   1 +
 qapi-schema.json |  22 ++++++
 qmp-commands.hx  |  33 ++++++++
 savevm.c         | 233 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 trace-events     |   2 +
 7 files changed, 324 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index a221459..e16e1ac 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1790,6 +1790,22 @@ STEXI
 show available trace events and their state
 ETEXI
 
+    {
+        .name       = "test_vmstates",
+        .args_type  = "iterations:i?,period:i?",
+        .params     = "iterations period",
+        .help       = "test the vmstates by dumping and loading form memory\n\t\t\t"
+                      "iterations: (optional) number of times, the vmstates will be tested\n\t\t\t"
+                      "period: (optional) sleep interval in milliseconds between each iteration",
+        .mhandler.cmd = hmp_test_vmstates,
+    },
+STEXI
+@item test_vmstates
+@findex test_vmstates
+dumps and reads the device state's data from the memory for testing purpose
+ETEXI
+
+
 STEXI
 @end table
 ETEXI
diff --git a/hmp.c b/hmp.c
index 6e1802a..ca7664e 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1737,3 +1737,20 @@ void hmp_info_devices(Monitor *mon, const QDict *qdict)
 
     qapi_free_QemuDeviceList(device_list);
 }
+void hmp_test_vmstates(Monitor *mon, const QDict *qdict)
+{
+    int has_iterations = qdict_haskey(qdict, "iterations");
+    int64_t iterations = qdict_get_try_int(qdict, "iterations", 10);
+    int has_period = qdict_haskey(qdict, "period");
+    int64_t period = qdict_get_try_int(qdict, "period", 100);
+
+    Error *err = NULL;
+
+    qmp_test_vmstates(has_iterations, iterations, has_period, period,
+                      false, NULL, &err);
+
+    if (err) {
+        monitor_printf(mon, "test-vmstates: %s\n", error_get_pretty(err));
+        error_free(err);
+    }
+}
diff --git a/hmp.h b/hmp.h
index 9d6b577..56fb865 100644
--- a/hmp.h
+++ b/hmp.h
@@ -95,6 +95,7 @@ void hmp_cpu_add(Monitor *mon, const QDict *qdict);
 void hmp_object_add(Monitor *mon, const QDict *qdict);
 void hmp_object_del(Monitor *mon, const QDict *qdict);
 void hmp_info_memdev(Monitor *mon, const QDict *qdict);
+void hmp_test_vmstates(Monitor *mon, const QDict *qdict);
 void object_add_completion(ReadLineState *rs, int nb_args, const char *str);
 void object_del_completion(ReadLineState *rs, int nb_args, const char *str);
 void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
diff --git a/qapi-schema.json b/qapi-schema.json
index 74d26fe..4c810d7 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3506,3 +3506,25 @@
 ##
 { 'command': 'query-devices',
   'returns': [ 'QemuDevice' ] }
+
+##
+# @test-vmstates
+#
+# tests the vmstates' value by dumping and loading in memory
+#
+# @iterations: (optional) The total iterations for vmstate testing.
+# The min and max defind value is 10 and 100 respectively.
+#
+# @period: (optional) sleep interval between iteration (in milliseconds).
+# The default interval is 100 milliseconds with min and max being
+# 1 and 10000 respectively.
+#
+# @devices: (optional) helps in resetting particular device(s) that
+# have been registered with SaveStateEntry.
+#
+# Since 2.2
+##
+{ 'command': 'test-vmstates',
+  'data': {'*iterations':  'int',
+           '*period':      'int',
+           '*devices':   [ 'QemuDevice' ] } }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index e489197..ae2bdca 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3798,3 +3798,36 @@ Example (1):
      ]
    }
 EQMP
+
+    {
+        .name       = "test-vmstates",
+        .args_type  = "iterations:i?,period:i?,devices:O?",
+        .mhandler.cmd_new = qmp_marshal_input_test_vmstates,
+    },
+
+SQMP
+test-vmstates
+-------------
+
+Tests the vmstates' entry by dumping and loading in/from memory
+
+Arguments:
+
+- "iterations": (optional) The total iterations for vmstate testing.
+                 The min and max defined value is 10 and 100 respectively.
+
+- "period": (optional) sleep interval between iteration (in milliseconds).
+            The default interval is 100 milliseconds with min and max being
+            1 and 10000 respectively.
+
+- "devices": (optional) helps in resetting particular device(s)
+             that have been registered with SaveStateEntry.
+
+Example:
+
+-> { "execute": "test-vmstates",
+     "arguments": {
+        "iterations": 10,
+        "period": 100, } }
+<- { "return": {} }
+EQMP
diff --git a/savevm.c b/savevm.c
index 764ca71..cd67c5d 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1182,6 +1182,239 @@ QemuDeviceList *qmp_query_devices(Error **errp)
     return device_list;
 }
 
+/*
+ * VMState testing
+ */
+
+#define TEST_VMSTATE_MIN_TIMES 10
+#define TEST_VMSTATE_MAX_TIMES 1000
+
+#define TEST_VMSTATE_MIN_INTERVAL_MS 1
+#define TEST_VMSTATE_DEFAULT_INTERVAL_MS 100
+#define TEST_VMSTATE_MAX_INTERVAL_MS 10000
+
+typedef struct VMStateLogState VMStateLogState;
+typedef struct DeviceInfoEntry DeviceInfoEntry;
+
+struct DeviceInfoEntry {
+    QemuDevice *device;
+    DeviceState *dev;
+    QTAILQ_ENTRY(DeviceInfoEntry) entry;
+};
+
+struct VMStateLogState {
+    int64_t current_iteration;
+    int64_t iterations;
+    int64_t period;
+    bool active_state;
+    QEMUTimer *timer;
+    QTAILQ_HEAD(device_list, DeviceInfoEntry) device_list;
+};
+
+static VMStateLogState *test_vmstates_get_current_state(void)
+{
+    static VMStateLogState current_state = {
+        .active_state = false,
+    };
+
+    return &current_state;
+}
+
+static inline void test_vmstates_clear_device_list(VMStateLogState *v)
+{
+    DeviceInfoEntry *de, *new_de;
+    QTAILQ_FOREACH_SAFE(de, &v->device_list, entry, new_de) {
+        QTAILQ_REMOVE(&v->device_list, de, entry);
+    }
+}
+
+static void test_vmstates_reset_devices(VMStateLogState *v)
+{
+    SaveStateEntry *se;
+    DeviceInfoEntry *de;
+
+    if (QTAILQ_EMPTY(&v->device_list)) {
+        QTAILQ_FOREACH(se, &savevm_handlers, entry) {
+            if (se->dev) {
+                trace_test_vmstates_reset_devices(se->vmsd->name);
+                device_reset(se->dev);
+            }
+        }
+    } else {
+        QTAILQ_FOREACH(de, &v->device_list, entry) {
+            trace_test_vmstates_reset_devices(de->device->device_name);
+            device_reset(de->dev);
+        }
+    }
+}
+
+static inline bool test_vmstates_check_device_name(VMStateLogState *v,
+                                                   QemuDeviceList *devices,
+                                                   Error **errp)
+{
+    SaveStateEntry *se;
+    bool device_present;
+    QTAILQ_INIT(&v->device_list);
+
+    /* now, checking against each one */
+    while (devices->next) {
+        DeviceInfoEntry *new_de;
+        device_present = false;
+        QTAILQ_FOREACH(se, &savevm_handlers, entry) {
+            if (!strcmp(se->vmsd->name, devices->value->device_name)) {
+                device_present = true;
+                break;
+            }
+        }
+        if (!device_present) {
+            test_vmstates_clear_device_list(v);
+            error_setg(errp, "Incorrect device name - %s\n",
+                       devices->value->device_name);
+            return false;
+        } else {
+            new_de = g_malloc0(sizeof(DeviceInfoEntry));
+            new_de->device = devices->value;
+            new_de->dev = se->dev;
+            QTAILQ_INSERT_TAIL(&v->device_list, new_de, entry);
+        }
+    }
+    return true;
+}
+
+static void test_vmstates_cb(void *opaque)
+{
+    VMStateLogState *v = opaque;
+    int saved_vm_running = runstate_is_running();
+    const QEMUSizedBuffer *qsb;
+    QEMUFile *f;
+    int ret;
+    int64_t save_vmstates_duration, load_vmstates_duration;
+    int64_t start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+
+    /* executing the steps for a single time with the help of timer */
+    if (++(v->current_iteration) <= v->iterations) {
+        saved_vm_running = runstate_is_running();
+
+        /* stopping the VM before dumping the vmstates */
+        vm_stop(RUN_STATE_SAVE_VM);
+
+        f = qemu_bufopen("w", NULL);
+        if (!f) {
+            goto testing_end;
+        }
+
+        cpu_synchronize_all_states();
+
+        /* saving the vmsates to memory buffer */
+        ret = qemu_save_device_state(f);
+        if (ret < 0) {
+            goto testing_end;
+        }
+        save_vmstates_duration = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) -
+                                 start_time;
+        trace_test_vmstates_cb(v->current_iteration, save_vmstates_duration,
+                               "save time (ms)");
+
+        qsb = qemu_buf_get(f);
+
+        /* clearing the states of the guest */
+        test_vmstates_reset_devices(v);
+
+        start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+        f = qemu_bufopen("r", (QEMUSizedBuffer *)qsb);
+        if (!f) {
+            goto testing_end;
+        }
+
+        /* loading the device states from the saved buffer */
+        ret = qemu_loadvm_state(f);
+        qemu_fclose(f);
+        if (ret < 0) {
+            goto testing_end;
+        }
+        load_vmstates_duration = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) -
+                                 start_time;
+        trace_test_vmstates_cb(v->current_iteration, load_vmstates_duration,
+                               "load time (ms)");
+
+        if (saved_vm_running) {
+            vm_start();
+        }
+        timer_mod(v->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
+                                              v->period);
+        return;
+    }
+
+ testing_end:
+    if (saved_vm_running) {
+        vm_start();
+    }
+    timer_del(v->timer);
+    timer_free(v->timer);
+    test_vmstates_clear_device_list(v);
+    v->active_state = false;
+    return;
+}
+
+void qmp_test_vmstates(bool has_iterations, int64_t iterations,
+                       bool has_period, int64_t period, bool has_devices,
+                       QemuDeviceList *devices, Error **errp)
+{
+    VMStateLogState *v = test_vmstates_get_current_state();
+    Error *local_err;
+
+    if (v->active_state) {
+        error_setg(errp, "VMState testing already in progress\n");
+        return;
+    }
+
+    v->active_state = true;
+
+    /* checking the value of total iterations to be in the defined range */
+    if (!has_iterations) {
+        v->iterations = TEST_VMSTATE_MIN_TIMES;
+    } else if (iterations >= TEST_VMSTATE_MIN_TIMES &&
+               iterations <= TEST_VMSTATE_MAX_TIMES) {
+        v->iterations = iterations;
+    } else {
+        error_setg(errp, "iterations value must be in the range [%d, %d]\n",
+                   TEST_VMSTATE_MIN_TIMES, TEST_VMSTATE_MAX_TIMES);
+        v->active_state = false;
+        return;
+    }
+
+    /* checking for the value of period to be in the defined range */
+    if (!has_period) {
+        v->period = TEST_VMSTATE_DEFAULT_INTERVAL_MS;
+    } else if (period >= TEST_VMSTATE_MIN_INTERVAL_MS &&
+               period <= TEST_VMSTATE_MAX_INTERVAL_MS) {
+        v->period = period;
+    } else {
+        error_setg(errp, "sleep interval (period) value must be "
+                   "in the defined range [%d, %d](ms)\n",
+                   TEST_VMSTATE_MIN_INTERVAL_MS, TEST_VMSTATE_MAX_INTERVAL_MS);
+        v->active_state = false;
+        return;
+    }
+
+    if (!has_devices) {
+        QTAILQ_INIT(&v->device_list);
+    } else if (!test_vmstates_check_device_name(v, devices, &local_err)) {
+        if (local_err) {
+            error_propagate(errp, local_err);
+        }
+        return;
+    }
+
+    v->current_iteration = 0;
+    v->timer = timer_new_ms(QEMU_CLOCK_REALTIME, test_vmstates_cb, v);
+    timer_mod(v->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME));
+}
+
+/*
+ * ----------------------------------------------------------------------------
+ */
+
 void qmp_xen_save_devices_state(const char *filename, Error **errp)
 {
     QEMUFile *f;
diff --git a/trace-events b/trace-events
index 11a17a8..bb00661 100644
--- a/trace-events
+++ b/trace-events
@@ -1086,6 +1086,8 @@ vmstate_save(const char *idstr, const char *vmsd_name) "%s, %s"
 vmstate_load(const char *idstr, const char *vmsd_name) "%s, %s"
 vmstate_load_field_error(const char *field, int ret) "field \"%s\" load failed, ret = %d"
 qemu_announce_self_iter(const char *mac) "%s"
+test_vmstates_reset_devices(const char *name) "resetting device: \"%s\""
+test_vmstates_cb(int64_t iteration, int64_t time, const char *info) "iteration: %ld, %s: %ld"
 
 # qemu-file.c
 qemu_file_fclose(void) ""
-- 
1.9.3

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

* [Qemu-devel] [RFC PATCH v3 4/6] VMState test: querying the vmstate testing process
  2014-08-09  6:26 [Qemu-devel] [RFC PATCH v3 0/6] VMState testing Sanidhya Kashyap
                   ` (2 preceding siblings ...)
  2014-08-09  6:26 ` [Qemu-devel] [RFC PATCH v3 3/6] VMstate test: basic VMState testing mechanism Sanidhya Kashyap
@ 2014-08-09  6:26 ` Sanidhya Kashyap
  2014-08-11 16:35   ` Eric Blake
  2014-08-09  6:26 ` [Qemu-devel] [RFC PATCH v3 5/6] VMState test: update period of " Sanidhya Kashyap
  2014-08-09  6:26 ` [Qemu-devel] [RFC PATCH v3 6/6] VMState test: cancel mechanism for an already running " Sanidhya Kashyap
  5 siblings, 1 reply; 14+ messages in thread
From: Sanidhya Kashyap @ 2014-08-09  6:26 UTC (permalink / raw)
  To: qemu list; +Cc: Sanidhya Kashyap, Dr. David Alan Gilbert, Juan Quintela

This patch has been updated to provide the following information:
* Added a new return value in the form of devices' info that provides
the device name as well as the version number. 
* provides the hmp interface - info test_vmstates and qmp interface -
query-test-vmstates to obtain the information about the running 
vmstate testing process. 

Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
---
 hmp-commands.hx  |  4 +++-
 hmp.c            | 31 +++++++++++++++++++++++++++++++
 hmp.h            |  1 +
 monitor.c        |  7 +++++++
 qapi-schema.json | 37 +++++++++++++++++++++++++++++++++++++
 qmp-commands.hx  | 28 ++++++++++++++++++++++++++++
 savevm.c         | 29 +++++++++++++++++++++++++++++
 7 files changed, 136 insertions(+), 1 deletion(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index e16e1ac..1a28e63 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1770,6 +1770,8 @@ show migration status
 show current migration capabilities
 @item info migrate_cache_size
 show current migration XBZRLE cache size
+@item info test_vmstates
+show current vmstates testing process info
 @item info balloon
 show balloon information
 @item info qtree
@@ -1799,13 +1801,13 @@ ETEXI
                       "period: (optional) sleep interval in milliseconds between each iteration",
         .mhandler.cmd = hmp_test_vmstates,
     },
+
 STEXI
 @item test_vmstates
 @findex test_vmstates
 dumps and reads the device state's data from the memory for testing purpose
 ETEXI
 
-
 STEXI
 @end table
 ETEXI
diff --git a/hmp.c b/hmp.c
index ca7664e..5d66975 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1754,3 +1754,34 @@ void hmp_test_vmstates(Monitor *mon, const QDict *qdict)
         error_free(err);
     }
 }
+
+void hmp_info_test_vmstates(Monitor *mon, const QDict *qdict)
+{
+    VMStateLogStateInfo *log_info = qmp_query_test_vmstates(NULL);
+    QemuDeviceList *device_list = log_info->devices;
+    Error *err = NULL;
+
+    if (log_info) {
+        monitor_printf(mon, "current-iteration: %"PRId64 "\n"
+                            "iterations: %"PRId64 "\n"
+                            "period: %"PRId64 "\n", log_info->current_iteration,
+                            log_info->iterations, log_info->period);
+        if (device_list) {
+            monitor_printf(mon, "devices:\n");
+            while (device_list) {
+                monitor_printf(mon, "device_name: %s, ",
+                               device_list->value->device_name);
+                monitor_printf(mon, "version: %ld\n",
+                               device_list->value->version);
+                device_list = device_list->next;
+            }
+        }
+    }
+
+    if (err) {
+        monitor_printf(mon, "test-vmstates: %s\n", error_get_pretty(err));
+        error_free(err);
+    }
+
+    qapi_free_VMStateLogStateInfo(log_info);
+}
diff --git a/hmp.h b/hmp.h
index 56fb865..881964a 100644
--- a/hmp.h
+++ b/hmp.h
@@ -39,6 +39,7 @@ void hmp_info_pci(Monitor *mon, const QDict *qdict);
 void hmp_info_block_jobs(Monitor *mon, const QDict *qdict);
 void hmp_info_tpm(Monitor *mon, const QDict *qdict);
 void hmp_info_devices(Monitor *mon, const QDict *qdict);
+void hmp_info_test_vmstates(Monitor *mon, const QDict *qdict);
 void hmp_quit(Monitor *mon, const QDict *qdict);
 void hmp_stop(Monitor *mon, const QDict *qdict);
 void hmp_system_reset(Monitor *mon, const QDict *qdict);
diff --git a/monitor.c b/monitor.c
index 2431686..66a4d1f 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2925,6 +2925,13 @@ static mon_cmd_t info_cmds[] = {
         .mhandler.cmd = hmp_info_devices,
     },
     {
+        .name       = "test_vmstates",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show the current vmstates testing information",
+        .mhandler.cmd = hmp_info_test_vmstates,
+    },
+    {
         .name       = NULL,
     },
 };
diff --git a/qapi-schema.json b/qapi-schema.json
index 4c810d7..b7408df 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3528,3 +3528,40 @@
   'data': {'*iterations':  'int',
            '*period':      'int',
            '*devices':   [ 'QemuDevice' ] } }
+
+##
+# @VMStateLogStateInfo
+#
+# VMState testing information
+# Tells about the current iteration, the total iterations
+# that have been provided and the sleep interval
+#
+# @current-iteration: shows the current iteration at which
+# the test is in.
+#
+# @iterations: the allocated total iterations for the vmstate
+# testing process.
+#
+# @period: the allowed sleep interval between each iteration
+#          (in milliseconds).
+#
+# @devices: returns the list of devices that are being tested
+#
+# Since 2.2
+##
+{ 'type': 'VMStateLogStateInfo',
+  'data': { 'current-iteration': 'int',
+            'iterations':        'int',
+            'period':            'int',
+            'devices':          [ 'QemuDevice' ] } }
+
+##
+# @query-test-vmstates
+#
+# Get the current parameters value of the vmstate testing process.
+#
+# Returns VMStateLogStateInfo structure.
+#
+# Since 2.2
+##
+{ 'command': 'query-test-vmstates', 'returns': 'VMStateLogStateInfo' }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index ae2bdca..84f8fc8 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3831,3 +3831,31 @@ Example:
         "period": 100, } }
 <- { "return": {} }
 EQMP
+
+    {
+        .name       = "query-test-vmstates",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_query_test_vmstates,
+    },
+
+SQMP
+query-test-vmstates
+-------------------
+
+Get the parameters information
+
+- "current_iteration": the current iteration going on
+- "iterations:" the total number of assigned iterations
+- "period": the sleep interval between the iteration
+- "devices": list of devices that are being tested
+
+Example:
+
+-> { "execute": "query-test-vmstates" }
+<- { "return": {
+            "current_iteration": 3,
+            "iterations": 100,
+            "period": 100,
+            "devices": [ { 'device': 'hpet', 'version': 2 },
+             { 'device': 'mc146818rtc', 'version': 3 } ] } }
+EQMP
diff --git a/savevm.c b/savevm.c
index cd67c5d..2709035 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1411,6 +1411,35 @@ void qmp_test_vmstates(bool has_iterations, int64_t iterations,
     timer_mod(v->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME));
 }
 
+VMStateLogStateInfo *qmp_query_test_vmstates(Error **errp)
+{
+    VMStateLogState *v = test_vmstates_get_current_state();
+    VMStateLogStateInfo *log_info = NULL;
+    DeviceInfoEntry *de;
+
+    if (!v->active_state) {
+        return log_info;
+    }
+
+    log_info = g_malloc0(sizeof(VMStateLogStateInfo));
+
+    log_info->current_iteration = v->current_iteration;
+    log_info->iterations = v->iterations;
+    log_info->period = v->period;
+    log_info->devices = NULL;
+    if (QTAILQ_EMPTY(&v->device_list)) {
+        log_info->devices = qmp_query_devices(NULL);
+    } else {
+        QTAILQ_FOREACH(de, &v->device_list, entry) {
+            log_info->devices = create_device_list(de->device->device_name,
+                                                   de->device->version,
+                                                   log_info->devices);
+        }
+    }
+
+    return log_info;
+}
+
 /*
  * ----------------------------------------------------------------------------
  */
-- 
1.9.3

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

* [Qemu-devel] [RFC PATCH v3 5/6] VMState test: update period of vmstate testing process
  2014-08-09  6:26 [Qemu-devel] [RFC PATCH v3 0/6] VMState testing Sanidhya Kashyap
                   ` (3 preceding siblings ...)
  2014-08-09  6:26 ` [Qemu-devel] [RFC PATCH v3 4/6] VMState test: querying the vmstate testing process Sanidhya Kashyap
@ 2014-08-09  6:26 ` Sanidhya Kashyap
  2014-08-09  6:26 ` [Qemu-devel] [RFC PATCH v3 6/6] VMState test: cancel mechanism for an already running " Sanidhya Kashyap
  5 siblings, 0 replies; 14+ messages in thread
From: Sanidhya Kashyap @ 2014-08-09  6:26 UTC (permalink / raw)
  To: qemu list; +Cc: Sanidhya Kashyap, Dr. David Alan Gilbert, Juan Quintela

Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
---
 hmp-commands.hx  | 15 +++++++++++++++
 hmp.c            | 14 ++++++++++++++
 hmp.h            |  1 +
 qapi-schema.json | 12 ++++++++++++
 qmp-commands.hx  | 21 +++++++++++++++++++++
 savevm.c         | 13 +++++++++++++
 6 files changed, 76 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 1a28e63..f74b935 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1808,6 +1808,21 @@ STEXI
 dumps and reads the device state's data from the memory for testing purpose
 ETEXI
 
+    {
+        .name       = "test_vmstates_set_period",
+        .args_type  = "period:i",
+        .params     = "period",
+        .help       = "set the sleep interval for vmstates testing process\n\t\t\t"
+                      "period: the new sleep interval value to replace the existing",
+        .mhandler.cmd = hmp_test_vmstates_set_period,
+    },
+
+STEXI
+@item test_vmstates_set_period @var{period}
+@findex test_vmstates_set_period
+Set the period to @var{period} (int) for vmstate testing process.
+ETEXI
+
 STEXI
 @end table
 ETEXI
diff --git a/hmp.c b/hmp.c
index 5d66975..d1f3045 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1785,3 +1785,17 @@ void hmp_info_test_vmstates(Monitor *mon, const QDict *qdict)
 
     qapi_free_VMStateLogStateInfo(log_info);
 }
+
+void hmp_test_vmstates_set_period(Monitor *mon, const QDict *qdict)
+{
+    int64_t period = qdict_get_int(qdict, "period");
+    Error *err = NULL;
+
+    qmp_test_vmstates_set_period(period, &err);
+
+    if (err) {
+        monitor_printf(mon, "test-vmstates-set-period: %s\n",
+                       error_get_pretty(err));
+        error_free(err);
+    }
+}
diff --git a/hmp.h b/hmp.h
index 881964a..32221e3 100644
--- a/hmp.h
+++ b/hmp.h
@@ -97,6 +97,7 @@ void hmp_object_add(Monitor *mon, const QDict *qdict);
 void hmp_object_del(Monitor *mon, const QDict *qdict);
 void hmp_info_memdev(Monitor *mon, const QDict *qdict);
 void hmp_test_vmstates(Monitor *mon, const QDict *qdict);
+void hmp_test_vmstates_set_period(Monitor *mon, const QDict *qdict);
 void object_add_completion(ReadLineState *rs, int nb_args, const char *str);
 void object_del_completion(ReadLineState *rs, int nb_args, const char *str);
 void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
diff --git a/qapi-schema.json b/qapi-schema.json
index b7408df..cff9b0f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3565,3 +3565,15 @@
 # Since 2.2
 ##
 { 'command': 'query-test-vmstates', 'returns': 'VMStateLogStateInfo' }
+
+##
+# @test-vmstates-set-period
+#
+# sets the sleep interval between iterations of the vmstate testing process
+#
+# @period: the updated sleep interval value (in milliseconds)
+#
+# Since 2.2
+##
+{ 'command' : 'test-vmstates-set-period',
+  'data'    : { 'period': 'int' } }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 84f8fc8..bde4fc9 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3859,3 +3859,24 @@ Example:
             "devices": [ { 'device': 'hpet', 'version': 2 },
              { 'device': 'mc146818rtc', 'version': 3 } ] } }
 EQMP
+    {
+        .name       = "test-vmstates-set-period",
+        .args_type  = "period:i",
+        .mhandler.cmd_new = qmp_marshal_input_test_vmstates_set_period,
+    },
+
+SQMP
+test-vmstates-set-period
+------------------------
+
+Update the sleep interval for the remaining iterations
+
+Arguments:
+
+- "period": the updated sleep interval between iterations (json-int)
+
+Example:
+
+-> { "execute": "test-vmstates-set-period", "arguments": { "period": 1024 } }
+<- { "return": {} }
+EQMP
diff --git a/savevm.c b/savevm.c
index 2709035..6ef69a1 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1440,6 +1440,19 @@ VMStateLogStateInfo *qmp_query_test_vmstates(Error **errp)
     return log_info;
 }
 
+void qmp_test_vmstates_set_period(int64_t period, Error **errp)
+{
+    VMStateLogState *v = test_vmstates_get_current_state();
+    if (period < TEST_VMSTATE_MIN_INTERVAL_MS ||
+        period > TEST_VMSTATE_MAX_INTERVAL_MS) {
+        error_setg(errp, "sleep interval (period) value must be "
+                   "in the defined range [%d, %d](ms)\n",
+                   TEST_VMSTATE_MIN_INTERVAL_MS, TEST_VMSTATE_MAX_INTERVAL_MS);
+        return;
+    }
+    v->period = period;
+}
+
 /*
  * ----------------------------------------------------------------------------
  */
-- 
1.9.3

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

* [Qemu-devel] [RFC PATCH v3 6/6] VMState test: cancel mechanism for an already running vmstate testing process
  2014-08-09  6:26 [Qemu-devel] [RFC PATCH v3 0/6] VMState testing Sanidhya Kashyap
                   ` (4 preceding siblings ...)
  2014-08-09  6:26 ` [Qemu-devel] [RFC PATCH v3 5/6] VMState test: update period of " Sanidhya Kashyap
@ 2014-08-09  6:26 ` Sanidhya Kashyap
  5 siblings, 0 replies; 14+ messages in thread
From: Sanidhya Kashyap @ 2014-08-09  6:26 UTC (permalink / raw)
  To: qemu list; +Cc: Sanidhya Kashyap, Dr. David Alan Gilbert, Juan Quintela

Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
---
 hmp-commands.hx  | 14 ++++++++++++++
 hmp.c            |  6 ++++++
 hmp.h            |  1 +
 qapi-schema.json |  9 +++++++++
 qmp-commands.hx  | 20 ++++++++++++++++++++
 savevm.c         | 16 ++++++++++++++--
 6 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index f74b935..eb543f8 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1823,6 +1823,20 @@ STEXI
 Set the period to @var{period} (int) for vmstate testing process.
 ETEXI
 
+	{
+	.name       = "test_vmstates_cancel",
+	.args_type  = "",
+	.params     = "",
+	.help       = "cancel the current vmstates testing process",
+	.mhandler.cmd = hmp_test_vmstates_cancel,
+},
+
+STEXI
+@item test_vmstates_cancel
+@findex test_vmstates_cancel
+Cancel the current vmstates testing process
+ETEXI
+
 STEXI
 @end table
 ETEXI
diff --git a/hmp.c b/hmp.c
index d1f3045..bda1672 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1799,3 +1799,9 @@ void hmp_test_vmstates_set_period(Monitor *mon, const QDict *qdict)
         error_free(err);
     }
 }
+
+void hmp_test_vmstates_cancel(Monitor *mon, const QDict *qdict)
+{
+   qmp_test_vmstates_cancel(NULL);
+}
+
diff --git a/hmp.h b/hmp.h
index 32221e3..d72de64 100644
--- a/hmp.h
+++ b/hmp.h
@@ -98,6 +98,7 @@ void hmp_object_del(Monitor *mon, const QDict *qdict);
 void hmp_info_memdev(Monitor *mon, const QDict *qdict);
 void hmp_test_vmstates(Monitor *mon, const QDict *qdict);
 void hmp_test_vmstates_set_period(Monitor *mon, const QDict *qdict);
+void hmp_test_vmstates_cancel(Monitor *mon, const QDict *qdict);
 void object_add_completion(ReadLineState *rs, int nb_args, const char *str);
 void object_del_completion(ReadLineState *rs, int nb_args, const char *str);
 void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
diff --git a/qapi-schema.json b/qapi-schema.json
index cff9b0f..a1161a9 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3577,3 +3577,12 @@
 ##
 { 'command' : 'test-vmstates-set-period',
   'data'    : { 'period': 'int' } }
+
+##
+# @test-vmstates-cancel
+#
+# cancel the testing vmstates process
+#
+# Since 2.2
+##
+{ 'command': 'test-vmstates-cancel' }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index bde4fc9..2e06611 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3880,3 +3880,23 @@ Example:
 -> { "execute": "test-vmstates-set-period", "arguments": { "period": 1024 } }
 <- { "return": {} }
 EQMP
+
+	{
+        .name       = "test-vmstates-cancel",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_test_vmstates_cancel,
+    },
+
+SQMP
+test-vmstates-cancel
+--------------------
+
+Cancel the current vmstate testing process.
+
+Arguments: None.
+
+Example:
+
+-> { "execute": "test-vmstates-cancel" }
+<- { "return": {} }
+EQMP
diff --git a/savevm.c b/savevm.c
index 6ef69a1..48b3579 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1340,8 +1340,12 @@ static void test_vmstates_cb(void *opaque)
         if (saved_vm_running) {
             vm_start();
         }
-        timer_mod(v->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
-                                              v->period);
+       if (v->active_state) {
+            timer_mod(v->timer, v->period +
+                      qemu_clock_get_ms(QEMU_CLOCK_REALTIME));
+        } else {
+            goto testing_end;
+        }
         return;
     }
 
@@ -1453,6 +1457,14 @@ void qmp_test_vmstates_set_period(int64_t period, Error **errp)
     v->period = period;
 }
 
+void qmp_test_vmstates_cancel(Error **errp)
+{
+    VMStateLogState *v = test_vmstates_get_current_state();
+    if (v->active_state) {
+        v->active_state = false;
+    }
+}
+
 /*
  * ----------------------------------------------------------------------------
  */
-- 
1.9.3

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

* Re: [Qemu-devel] [RFC PATCH v3 1/6] QEMUSizedBuffer/QEMUFile
  2014-08-09  6:26 ` [Qemu-devel] [RFC PATCH v3 1/6] QEMUSizedBuffer/QEMUFile Sanidhya Kashyap
@ 2014-08-11  2:49   ` Gonglei (Arei)
  2014-08-11 18:47     ` Dr. David Alan Gilbert
  2014-08-11 16:38   ` Eric Blake
  1 sibling, 1 reply; 14+ messages in thread
From: Gonglei (Arei) @ 2014-08-11  2:49 UTC (permalink / raw)
  To: Sanidhya Kashyap, qemu list
  Cc: Joel Schopp, Juan Quintela, Dr. David Alan Gilbert, Stefan Berger

> -----Original Message-----
> From: qemu-devel-bounces+arei.gonglei=huawei.com@nongnu.org
> [mailto:qemu-devel-bounces+arei.gonglei=huawei.com@nongnu.org] On
> Behalf Of Sanidhya Kashyap
> Sent: Saturday, August 09, 2014 2:27 PM
> To: qemu list
> Cc: Sanidhya Kashyap; Joel Schopp; Stefan Berger; Dr. David Alan Gilbert; Juan
> Quintela
> Subject: [Qemu-devel] [RFC PATCH v3 1/6] QEMUSizedBuffer/QEMUFile
> 
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Stefan Berger's to create 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>
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
> ---
>  include/migration/qemu-file.h |  27 +++
>  qemu-file.c                   | 411
> ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 438 insertions(+)
> 
> diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
> index c90f529..14e1f4d 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,31 @@ 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;
> +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);
> +
> +
>  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 +133,11 @@ 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);
> 
> +/*
> + * 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/qemu-file.c b/qemu-file.c
> index a8e3912..9fd1675 100644
> --- a/qemu-file.c
> +++ b/qemu-file.c
> @@ -878,3 +878,414 @@ 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  (10 * 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
> + */
> +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;
> +
> +    if (len == 0) {
> +        /* we want to allocate at least one chunk */
> +        len = QSB_CHUNK_SIZE;
> +    }
> +
> +    num_chunks = DIV_ROUND_UP(len, chunk_size);
> +    alloc_len = num_chunks * chunk_size;
> +
> +    qsb = g_new0(QEMUSizedBuffer, 1);
> +    qsb->iov = g_new0(struct iovec, num_chunks);
> +    qsb->n_iov = num_chunks;
> +
> +    for (i = 0; i < num_chunks; i++) {
> +        qsb->iov[i].iov_base = g_malloc0(chunk_size);
> +        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 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 trucated 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 an optional buffer to write into; the pointer may
> + *       point to NULL in which case the buffer will be allocated;
> + *       if buffer is provided, it must be large enough to hold @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 **buf)
> +{
> +    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;
> +    }
> +
> +    if (*buf == NULL) {
> +        *buf = g_malloc(all_copy);
> +    }
> +    buffer = *buf;
> +
> +    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 allocated
> + * memory for it.
> + *
> + * @qsb: A QEMUSizedBuffer
> + * @new_size: The new size of the buffer
> + *
> + * Returns an error code in case of memory allocation failure
> + * or the new size of the buffer otherwise. 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;
> +    size_t chunk_size = QSB_CHUNK_SIZE;
> +
> +    if (qsb->size < new_size) {
> +        needed_chunks = DIV_ROUND_UP(new_size - qsb->size,
> +                                     chunk_size);
> +
> +        qsb->iov = g_realloc_n(qsb->iov, qsb->n_iov + needed_chunks,
> +                               sizeof(struct iovec));
> +        if (qsb->iov == NULL) {
> +            return -ENOMEM;
> +        }

OK...

> +
> +        for (i = qsb->n_iov; i < qsb->n_iov + needed_chunks; i++) {
> +            qsb->iov[i].iov_base = g_malloc0(chunk_size);
> +            qsb->iov[i].iov_len = chunk_size;
> +        }
> +
> +        qsb->n_iov += needed_chunks;
> +        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 withing the @qsb to write data to
> + * @size: The number of bytes to copy into the @qsb
> + *
> + * Returns an error code in case of memory allocation failure,
> + * @size otherwise.
> + */
> +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;
> +    }
> +

OK...

> +    if (pos + count > qsb->used) {
> +        qsb->used = pos + count;
> +    }
> +
> +    index = qsb_get_iovec(qsb, pos, &d_off);
> +    if (index < 0) {
> +        return 0;
> +    }
> +
> +    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 an exact copy of the given QEMUSizedBuffer.
> + *
> + * @qsb : A QEMUSizedBuffer
> + *
> + * Returns a clone of @qsb
> + */
> +QEMUSizedBuffer *qsb_clone(const QEMUSizedBuffer *qsb)
> +{
> +    QEMUSizedBuffer *out = qsb_create(NULL, qsb_get_length(qsb));
> +    size_t i;
> +    off_t pos = 0;
> +
> +    for (i = 0; i < qsb->n_iov; i++) {
> +        pos += qsb_write_at(out, qsb->iov[i].iov_base,
> +                            pos, qsb->iov[i].iov_len);

If qsb_write_at() return -ENOMEM, just simply add it to pos ?

Best regards,
-Gonglei

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

* Re: [Qemu-devel] [RFC PATCH v3 2/6] VMState test: get information about the registered devices
  2014-08-09  6:26 ` [Qemu-devel] [RFC PATCH v3 2/6] VMState test: get information about the registered devices Sanidhya Kashyap
@ 2014-08-11 16:24   ` Eric Blake
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Blake @ 2014-08-11 16:24 UTC (permalink / raw)
  To: Sanidhya Kashyap, qemu list; +Cc: Dr. David Alan Gilbert, Juan Quintela

[-- Attachment #1: Type: text/plain, Size: 2740 bytes --]

On 08/09/2014 12:26 AM, Sanidhya Kashyap wrote:
> Added both qmp and hmp interface to get the information about the devices that
> have been qdevified and are registered with the SaveVMHandlers. I have not used 
> any format to print the device information for the hmp interface. It would be
> great if anyone can give me some pointers about this about the printing format
> if there is something.
> 
> The qmp command to extract the information about the devices is
> qmp_query_devices which provides the list of device names and their respective
> version. The hmp command name is 'info devices' which lists the same information
> as provided by the qmp interface.
> 
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
> ---
>  hmp-commands.hx  |  2 ++
>  hmp.c            | 23 +++++++++++++++++++++++
>  hmp.h            |  1 +
>  monitor.c        |  7 +++++++
>  qapi-schema.json | 26 ++++++++++++++++++++++++++
>  qmp-commands.hx  | 43 +++++++++++++++++++++++++++++++++++++++++++
>  savevm.c         | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  7 files changed, 147 insertions(+)
> 
> +++ b/qapi-schema.json
> @@ -3480,3 +3480,29 @@
>  # Since: 2.1
>  ##
>  { 'command': 'rtc-reset-reinjection' }
> +
> +##
> +# @QemuDevice
> +#
> +# device that is registered with SaveVMHandlers

This sounds like an internal implementation detail; is there a better
description more suited to what this struct represents from the API
point of view, maybe as simple as: "Information about a device"


> +{ 'command': 'query-devices',
> +  'returns': [ 'QemuDevice' ] }

Looks okay here...

> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 4be4765..e489197 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx

> +
> +Example (1):

s/ (1)// - unless you have a second example, you don't need to label the
first example with a 1 suffix

> +
> +-> { "execute": "query-devices" }
> +<- { "return": [
> +       {
> +           [ { 'device-name': 'kvm-tpr-opt', 'version': 1 },

... but this doesn't match your command.  You have too many layers.
Also, QMP wire format uses ONLY ", never '.  It should look like:

{ "return": [ { "device-name": "kvm-tpr-opt", "version": 1 },
              ...
            ] }

In fact, rather than trying to write it by hand, actually compile your
code and test the actual QMP output and paste that in - then you'll know
you got it right.


> +static QemuDeviceList *create_device_list(const char *name, int version,
> +                                           QemuDeviceList *list)

Indentation is off.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

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

* Re: [Qemu-devel] [RFC PATCH v3 3/6] VMstate test: basic VMState testing mechanism
  2014-08-09  6:26 ` [Qemu-devel] [RFC PATCH v3 3/6] VMstate test: basic VMState testing mechanism Sanidhya Kashyap
@ 2014-08-11 16:32   ` Eric Blake
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Blake @ 2014-08-11 16:32 UTC (permalink / raw)
  To: Sanidhya Kashyap, qemu list; +Cc: Dr. David Alan Gilbert, Juan Quintela

[-- Attachment #1: Type: text/plain, Size: 4273 bytes --]

On 08/09/2014 12:26 AM, Sanidhya Kashyap wrote:
> This patch implements the basic way of testing the VMStates' information
> whether it is correct or not while saving and loading the states. The qmp
> interface - test-vmstates can take three parameters as an input to test
> the device states. Now, one can check for any of the devices that have been
> registered with the SaveVMHandlers aka the migration protocol. Similarly,
> the hmp interface (test_vmstates) has only two input parameters - iterations and
> period.
> 

This paragraph:

vvvvvvv
> I have removed the following from the patch on previous comments:
> * replaed DPRINTF with trace_##name.
> * removed the noqdev and completely removed the support for resetting of devices
> based on qemu_system_reset()
^^^^^^^

should not be part of the commit message proper, but...

> 
> As Juan pointed out that there might be a memory leak as I did not close the
> QEMUFile pointer. But, it is not required as that pointer is directly referenced
> by the QEMUBuffer that has been implemented by David. So, IMHO the case pointed
> out by Juan does not exist.
> 
> 
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
> ---

...listed here, as a changelog to guide reviewers.  Remember, anything
before the --- should stand alone as what you would read in qemu.git,
without regards to how many revisions the patch went through; and
anything after the --- is useful for reviewers but intentionally
stripped by the maintainer when using 'git am' as fluff that doesn't
help explain the commit in isolation.

>  hmp-commands.hx  |  16 ++++
>  hmp.c            |  17 ++++
>  hmp.h            |   1 +
>  qapi-schema.json |  22 ++++++
>  qmp-commands.hx  |  33 ++++++++
>  savevm.c         | 233 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  trace-events     |   2 +
>  7 files changed, 324 insertions(+)
> 

> +++ b/qapi-schema.json
> @@ -3506,3 +3506,25 @@
>  ##
>  { 'command': 'query-devices',
>    'returns': [ 'QemuDevice' ] }
> +
> +##
> +# @test-vmstates
> +#
> +# tests the vmstates' value by dumping and loading in memory
> +#
> +# @iterations: (optional) The total iterations for vmstate testing.

For consistency, and in case we ever start generating documentation from
the .json file,
s/(optional)/#optional/

> +# The min and max defind value is 10 and 100 respectively.

s/defind value is/defined values are/

> +#
> +# @period: (optional) sleep interval between iteration (in milliseconds).

s/(optional)/#optional/

> +# The default interval is 100 milliseconds with min and max being
> +# 1 and 10000 respectively.
> +#
> +# @devices: (optional) helps in resetting particular device(s) that
> +# have been registered with SaveStateEntry.
> +#
> +# Since 2.2
> +##
> +{ 'command': 'test-vmstates',
> +  'data': {'*iterations':  'int',
> +           '*period':      'int',
> +           '*devices':   [ 'QemuDevice' ] } }

> +Example:
> +
> +-> { "execute": "test-vmstates",
> +     "arguments": {
> +        "iterations": 10,
> +        "period": 100, } }

That is not valid JSON.  You cannot have a trailing , inside {}.  Also,
it might be worth an example of the proper use of the optional "devices"
parameter.


> +
> +static inline bool test_vmstates_check_device_name(VMStateLogState *v,

The compiler is good enough about inlining static functions that you
seldom need to use 'inline'.  That, and 'static inline' has changed
semantics over the years of gcc, so you really want to avoid it unless
you know what you are doing.


> +static void test_vmstates_cb(void *opaque)
> +{
> +    VMStateLogState *v = opaque;
> +    int saved_vm_running = runstate_is_running();
> +    const QEMUSizedBuffer *qsb;

> +
> +        qsb = qemu_buf_get(f);
> +
> +        /* clearing the states of the guest */
> +        test_vmstates_reset_devices(v);
> +
> +        start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> +        f = qemu_bufopen("r", (QEMUSizedBuffer *)qsb);

Ewww.  Why are you casting away const?  Make qsb the correct type to
begin with.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

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

* Re: [Qemu-devel] [RFC PATCH v3 4/6] VMState test: querying the vmstate testing process
  2014-08-09  6:26 ` [Qemu-devel] [RFC PATCH v3 4/6] VMState test: querying the vmstate testing process Sanidhya Kashyap
@ 2014-08-11 16:35   ` Eric Blake
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Blake @ 2014-08-11 16:35 UTC (permalink / raw)
  To: Sanidhya Kashyap, qemu list; +Cc: Dr. David Alan Gilbert, Juan Quintela

[-- Attachment #1: Type: text/plain, Size: 2795 bytes --]

On 08/09/2014 12:26 AM, Sanidhya Kashyap wrote:
> This patch has been updated to provide the following information:
> * Added a new return value in the form of devices' info that provides
> the device name as well as the version number. 
> * provides the hmp interface - info test_vmstates and qmp interface -
> query-test-vmstates to obtain the information about the running 
> vmstate testing process. 

Patch changelog information belongs...

> 
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
> ---

...after the --- separator.

>  hmp-commands.hx  |  4 +++-
>  hmp.c            | 31 +++++++++++++++++++++++++++++++
>  hmp.h            |  1 +
>  monitor.c        |  7 +++++++
>  qapi-schema.json | 37 +++++++++++++++++++++++++++++++++++++
>  qmp-commands.hx  | 28 ++++++++++++++++++++++++++++
>  savevm.c         | 29 +++++++++++++++++++++++++++++
>  7 files changed, 136 insertions(+), 1 deletion(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx

> @@ -1799,13 +1801,13 @@ ETEXI
>                        "period: (optional) sleep interval in milliseconds between each iteration",
>          .mhandler.cmd = hmp_test_vmstates,
>      },
> +
>  STEXI
>  @item test_vmstates
>  @findex test_vmstates
>  dumps and reads the device state's data from the memory for testing purpose
>  ETEXI
>  
> -
>  STEXI

Spurious whitespace changes?

> +++ b/qapi-schema.json
> @@ -3528,3 +3528,40 @@
>    'data': {'*iterations':  'int',
>             '*period':      'int',
>             '*devices':   [ 'QemuDevice' ] } }
> +
> +##
> +# @VMStateLogStateInfo

Having State in the name twice sounds redundant.  Would 'VMStateLogInfo'
be sufficient?


> +{ 'type': 'VMStateLogStateInfo',
> +  'data': { 'current-iteration': 'int',
> +            'iterations':        'int',
> +            'period':            'int',
> +            'devices':          [ 'QemuDevice' ] } }
> +
> +##
> +# @query-test-vmstates
> +#
> +# Get the current parameters value of the vmstate testing process.
> +#
> +# Returns VMStateLogStateInfo structure.
> +#
> +# Since 2.2
> +##
> +{ 'command': 'query-test-vmstates', 'returns': 'VMStateLogStateInfo' }

> +Example:
> +
> +-> { "execute": "query-test-vmstates" }
> +<- { "return": {
> +            "current_iteration": 3,

Doesn't match the spelling documented in the .json.

> +            "iterations": 100,
> +            "period": 100,
> +            "devices": [ { 'device': 'hpet', 'version': 2 },

QMP uses ", not '.  Please paste an actual example (possibly trimmed, if
the list of devices is otherwise too huge) rather than trying to write
it by hand.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

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

* Re: [Qemu-devel] [RFC PATCH v3 1/6] QEMUSizedBuffer/QEMUFile
  2014-08-09  6:26 ` [Qemu-devel] [RFC PATCH v3 1/6] QEMUSizedBuffer/QEMUFile Sanidhya Kashyap
  2014-08-11  2:49   ` Gonglei (Arei)
@ 2014-08-11 16:38   ` Eric Blake
  1 sibling, 0 replies; 14+ messages in thread
From: Eric Blake @ 2014-08-11 16:38 UTC (permalink / raw)
  To: Sanidhya Kashyap, qemu list
  Cc: Joel Schopp, Juan Quintela, Dr. David Alan Gilbert, Stefan Berger

[-- Attachment #1: Type: text/plain, Size: 1087 bytes --]

On 08/09/2014 12:26 AM, Sanidhya Kashyap wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Stefan Berger's to create 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>
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
> ---
>  include/migration/qemu-file.h |  27 +++
>  qemu-file.c                   | 411 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 438 insertions(+)

I'd rather see us take the version most recently updated by David:

https://lists.gnu.org/archive/html/qemu-devel/2014-08/msg01099.html

then you rebase the rest of your series on top of that.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

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

* Re: [Qemu-devel] [RFC PATCH v3 1/6] QEMUSizedBuffer/QEMUFile
  2014-08-11  2:49   ` Gonglei (Arei)
@ 2014-08-11 18:47     ` Dr. David Alan Gilbert
  2014-08-11 19:16       ` Stefan Berger
  0 siblings, 1 reply; 14+ messages in thread
From: Dr. David Alan Gilbert @ 2014-08-11 18:47 UTC (permalink / raw)
  To: Gonglei (Arei)
  Cc: Joel Schopp, Stefan Berger, qemu list, Juan Quintela, Sanidhya Kashyap

* Gonglei (Arei) (arei.gonglei@huawei.com) wrote:

<snip>

> > +/**
> > + * Grow the QEMUSizedBuffer to the given size and allocated
> > + * memory for it.
> > + *
> > + * @qsb: A QEMUSizedBuffer
> > + * @new_size: The new size of the buffer
> > + *
> > + * Returns an error code in case of memory allocation failure
> > + * or the new size of the buffer otherwise. 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;
> > +    size_t chunk_size = QSB_CHUNK_SIZE;
> > +
> > +    if (qsb->size < new_size) {
> > +        needed_chunks = DIV_ROUND_UP(new_size - qsb->size,
> > +                                     chunk_size);
> > +
> > +        qsb->iov = g_realloc_n(qsb->iov, qsb->n_iov + needed_chunks,
> > +                               sizeof(struct iovec));
> > +        if (qsb->iov == NULL) {
> > +            return -ENOMEM;
> > +        }
> 
> OK...

Is it?  https://developer.gnome.org/glib/stable/glib-Memory-Allocation.html
says that g_realloc_n is 'similar to g_realloc()' except for overflow protection,
g_realloc() doesn't say what it's behaviour on OOM is, but g_try_realloc says
that 'Contrast with g_realloc(), which aborts the program on failure'
So the only case iov will return NULL there is if the size is 0 which it can't
be.  So should that be a g_try_realloc_n ?

> > +
> > +        for (i = qsb->n_iov; i < qsb->n_iov + needed_chunks; i++) {
> > +            qsb->iov[i].iov_base = g_malloc0(chunk_size);
> > +            qsb->iov[i].iov_len = chunk_size;
> > +        }
> > +
> > +        qsb->n_iov += needed_chunks;
> > +        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 withing the @qsb to write data to
> > + * @size: The number of bytes to copy into the @qsb
> > + *
> > + * Returns an error code in case of memory allocation failure,
> > + * @size otherwise.
> > + */
> > +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;
> > +    }
> > +
> 
> OK...
> 
> > +    if (pos + count > qsb->used) {
> > +        qsb->used = pos + count;
> > +    }
> > +
> > +    index = qsb_get_iovec(qsb, pos, &d_off);
> > +    if (index < 0) {
> > +        return 0;
> > +    }
> > +
> > +    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 an exact copy of the given QEMUSizedBuffer.
> > + *
> > + * @qsb : A QEMUSizedBuffer
> > + *
> > + * Returns a clone of @qsb
> > + */
> > +QEMUSizedBuffer *qsb_clone(const QEMUSizedBuffer *qsb)
> > +{
> > +    QEMUSizedBuffer *out = qsb_create(NULL, qsb_get_length(qsb));
> > +    size_t i;
> > +    off_t pos = 0;
> > +
> > +    for (i = 0; i < qsb->n_iov; i++) {
> > +        pos += qsb_write_at(out, qsb->iov[i].iov_base,
> > +                            pos, qsb->iov[i].iov_len);
> 
> If qsb_write_at() return -ENOMEM, just simply add it to pos ?

qsb_create uses g_new0 so it will abort on out of memory;
what should qsb_clone do if qsb_write_at returns -ENOMEM?
(Admittedly anything is better than getting the position wrong).
I guess the choice is to allow it to return NULL, tidying up
and offering the chance sometime in the future of tidying up
the other allocators.

Dave

> 
> Best regards,
> -Gonglei
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

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

* Re: [Qemu-devel] [RFC PATCH v3 1/6] QEMUSizedBuffer/QEMUFile
  2014-08-11 18:47     ` Dr. David Alan Gilbert
@ 2014-08-11 19:16       ` Stefan Berger
  0 siblings, 0 replies; 14+ messages in thread
From: Stefan Berger @ 2014-08-11 19:16 UTC (permalink / raw)
  To: qemu-devel

On 08/11/2014 02:47 PM, Dr. David Alan Gilbert wrote:
> * Gonglei (Arei) (arei.gonglei@huawei.com) wrote:
>
> <snip>
>
>>> +/**
>>> + * Grow the QEMUSizedBuffer to the given size and allocated
>>> + * memory for it.
>>> + *
>>> + * @qsb: A QEMUSizedBuffer
>>> + * @new_size: The new size of the buffer
>>> + *
>>> + * Returns an error code in case of memory allocation failure
>>> + * or the new size of the buffer otherwise. 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;
>>> +    size_t chunk_size = QSB_CHUNK_SIZE;
>>> +
>>> +    if (qsb->size < new_size) {
>>> +        needed_chunks = DIV_ROUND_UP(new_size - qsb->size,
>>> +                                     chunk_size);
>>> +
>>> +        qsb->iov = g_realloc_n(qsb->iov, qsb->n_iov + needed_chunks,
>>> +                               sizeof(struct iovec));
>>> +        if (qsb->iov == NULL) {
>>> +            return -ENOMEM;
>>> +        }
>> OK...
> Is it?  https://developer.gnome.org/glib/stable/glib-Memory-Allocation.html
> says that g_realloc_n is 'similar to g_realloc()' except for overflow protection,
> g_realloc() doesn't say what it's behaviour on OOM is, but g_try_realloc says
> that 'Contrast with g_realloc(), which aborts the program on failure'
> So the only case iov will return NULL there is if the size is 0 which it can't
> be.  So should that be a g_try_realloc_n ?
>
>>> +
>>> +        for (i = qsb->n_iov; i < qsb->n_iov + needed_chunks; i++) {
>>> +            qsb->iov[i].iov_base = g_malloc0(chunk_size);
>>> +            qsb->iov[i].iov_len = chunk_size;
>>> +        }
>>> +
>>> +        qsb->n_iov += needed_chunks;
>>> +        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 withing the @qsb to write data to
>>> + * @size: The number of bytes to copy into the @qsb
>>> + *
>>> + * Returns an error code in case of memory allocation failure,
>>> + * @size otherwise.
>>> + */
>>> +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;
>>> +    }
>>> +
>> OK...
>>
>>> +    if (pos + count > qsb->used) {
>>> +        qsb->used = pos + count;
>>> +    }
>>> +
>>> +    index = qsb_get_iovec(qsb, pos, &d_off);
>>> +    if (index < 0) {
>>> +        return 0;
>>> +    }
>>> +
>>> +    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 an exact copy of the given QEMUSizedBuffer.
>>> + *
>>> + * @qsb : A QEMUSizedBuffer
>>> + *
>>> + * Returns a clone of @qsb
>>> + */
>>> +QEMUSizedBuffer *qsb_clone(const QEMUSizedBuffer *qsb)
>>> +{
>>> +    QEMUSizedBuffer *out = qsb_create(NULL, qsb_get_length(qsb));
>>> +    size_t i;
>>> +    off_t pos = 0;
>>> +
>>> +    for (i = 0; i < qsb->n_iov; i++) {
>>> +        pos += qsb_write_at(out, qsb->iov[i].iov_base,
>>> +                            pos, qsb->iov[i].iov_len);
>> If qsb_write_at() return -ENOMEM, just simply add it to pos ?
> qsb_create uses g_new0 so it will abort on out of memory;
> what should qsb_clone do if qsb_write_at returns -ENOMEM?
> (Admittedly anything is better than getting the position wrong).
> I guess the choice is to allow it to return NULL, tidying up
> and offering the chance sometime in the future of tidying up
> the other allocators.

I remember looking at all the allocation API as well. It seems QEMU 
would terminate upon OOM since g_new0 is used all over the place.

     Stefan

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

end of thread, other threads:[~2014-08-11 19:17 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-09  6:26 [Qemu-devel] [RFC PATCH v3 0/6] VMState testing Sanidhya Kashyap
2014-08-09  6:26 ` [Qemu-devel] [RFC PATCH v3 1/6] QEMUSizedBuffer/QEMUFile Sanidhya Kashyap
2014-08-11  2:49   ` Gonglei (Arei)
2014-08-11 18:47     ` Dr. David Alan Gilbert
2014-08-11 19:16       ` Stefan Berger
2014-08-11 16:38   ` Eric Blake
2014-08-09  6:26 ` [Qemu-devel] [RFC PATCH v3 2/6] VMState test: get information about the registered devices Sanidhya Kashyap
2014-08-11 16:24   ` Eric Blake
2014-08-09  6:26 ` [Qemu-devel] [RFC PATCH v3 3/6] VMstate test: basic VMState testing mechanism Sanidhya Kashyap
2014-08-11 16:32   ` Eric Blake
2014-08-09  6:26 ` [Qemu-devel] [RFC PATCH v3 4/6] VMState test: querying the vmstate testing process Sanidhya Kashyap
2014-08-11 16:35   ` Eric Blake
2014-08-09  6:26 ` [Qemu-devel] [RFC PATCH v3 5/6] VMState test: update period of " Sanidhya Kashyap
2014-08-09  6:26 ` [Qemu-devel] [RFC PATCH v3 6/6] VMState test: cancel mechanism for an already running " Sanidhya Kashyap

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.