[Qemu-devel] [PULL 0/9] migration queue

[Qemu-devel] [PULL 0/9] migration queue

[Juan Quintela]

Hi Anthony

This are the migration patches on the list at the moment, could you apply?

* cleanup patches from Isaku
* change is_zero_page() prototype
* cleanups from Lei Li
* cleanup from Christoffer Dall
* new VMSTATE macro from Alexey

They are quite easy, passed virt-test without trouble.


Thanks, Juan.

The following changes since commit f828a4c8faa118e0ebab3e353ac6840f3b2a0318:

  Merge remote-tracking branch 'stefanha/tracing' into staging (2013-09-23 11:53:22 -0500)

are available in the git repository at:


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

for you to fetch changes up to d613a56f845788412a442c6b5aff88b38244f99a:

  migration: ram_handle_compressed (2013-09-24 13:22:50 +0200)

----------------------------------------------------------------
Alexey Kardashevskiy (1):
      migration: add version supporting macros for struct pointer

Christoffer Dall (1):
      migration: Fix debug print type

Isaku Yamahata (4):
      rdma: clean up of qemu_rdma_cleanup()
      rdma: constify ram_chunk_{index, start, end}
      arch_init: make is_zero_page accept size
      migration: ram_handle_compressed

Lei Li (3):
      savevm: add comments for qemu_file_get_error()
      savevm: fix wrong initialization by ram_control_load_hook
      arch_init: right return for ram_save_iterate

 arch_init.c                 | 33 +++++++++++++++++++--------------
 include/migration/vmstate.h | 17 +++++++++++++++--
 migration-rdma.c            | 17 +++++++++++------
 migration.c                 |  3 ++-
 savevm.c                    |  9 ++++++++-
 5 files changed, 55 insertions(+), 24 deletions(-)

[Qemu-devel] [PATCH 1/9] savevm: add comments for qemu_file_get_error()

[Juan Quintela]

From: Lei Li <lilei@linux.vnet.ibm.com>

Add comments for qemu_file_get_error(), as its return value
is not very clear.

Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 savevm.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/savevm.c b/savevm.c
index 4a3c819..a834c6f 100644
--- a/savevm.c
+++ b/savevm.c
@@ -566,6 +566,13 @@ QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops)
     return f;
 }

+/*
+ * Get last error for stream f
+ *
+ * Return negative error value if there has been an error on previous
+ * operations, return 0 if no error happened.
+ *
+ */
 int qemu_file_get_error(QEMUFile *f)
 {
     return f->last_error;
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/9] savevm: fix wrong initialization by ram_control_load_hook

[Juan Quintela]

From: Lei Li <lilei@linux.vnet.ibm.com>

It should set negative error value rather than 0 in QEMUFile
if there has been an error.

Reviewed-by: Michael R. Hines <mrhines@us.ibm.com>
Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 savevm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/savevm.c b/savevm.c
index a834c6f..2f631d4 100644
--- a/savevm.c
+++ b/savevm.c
@@ -649,7 +649,7 @@ void ram_control_after_iterate(QEMUFile *f, uint64_t flags)

 void ram_control_load_hook(QEMUFile *f, uint64_t flags)
 {
-    int ret = 0;
+    int ret = -EINVAL;

     if (f->ops->hook_ram_load) {
         ret = f->ops->hook_ram_load(f, f->opaque, flags);
-- 
1.8.3.1

[Qemu-devel] [PATCH 3/9] arch_init: right return for ram_save_iterate

[Juan Quintela]

From: Lei Li <lilei@linux.vnet.ibm.com>

qemu_file_rate_limit() never return negative value since the refactor
by Commit 1964a39, this patch gets rid of the negative check for it,
adjust bytes_transferred and return value correspondingly in
ram_save_iterate().

Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 arch_init.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index e47e139..18cd9a1 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -710,15 +710,20 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
      */
     ram_control_after_iterate(f, RAM_CONTROL_ROUND);

+    bytes_transferred += total_sent;
+
+    /*
+     * Do not count these 8 bytes into total_sent, so that we can
+     * return 0 if no page had been dirtied.
+     */
+    qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
+    bytes_transferred += 8;
+
+    ret = qemu_file_get_error(f);
     if (ret < 0) {
-        bytes_transferred += total_sent;
         return ret;
     }

-    qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
-    total_sent += 8;
-    bytes_transferred += total_sent;
-
     return total_sent;
 }

-- 
1.8.3.1

[Qemu-devel] [PATCH 4/9] rdma: clean up of qemu_rdma_cleanup()

[Juan Quintela]

From: Isaku Yamahata <yamahata@private.email.ne.jp>

- It can't be determined by RDMAContext::cm_id != NULL if the connection
  is established or not.
- RDMAContext::cm_id is leaked and not destroyed because it is set to NULL
  too early.
- RDMAContext::qp is created by rdma_create_qp() so that it should be destroyed
  by rdma_destroy_qp(). not ibv_destroy_qp()

Cc: Michael R. Hines <mrhines@us.ibm.com>
Signed-off-by: Isaku Yamahata <yamahata@private.email.ne.jp>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration-rdma.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/migration-rdma.c b/migration-rdma.c
index 05a155b..3679acb 100644
--- a/migration-rdma.c
+++ b/migration-rdma.c
@@ -356,6 +356,7 @@ typedef struct RDMAContext {
      */
     struct rdma_cm_id *cm_id;               /* connection manager ID */
     struct rdma_cm_id *listen_id;
+    bool connected;

     struct ibv_context          *verbs;
     struct rdma_event_channel   *channel;
@@ -2194,7 +2195,7 @@ static void qemu_rdma_cleanup(RDMAContext *rdma)
     struct rdma_cm_event *cm_event;
     int ret, idx;

-    if (rdma->cm_id) {
+    if (rdma->cm_id && rdma->connected) {
         if (rdma->error_state) {
             RDMAControlHeader head = { .len = 0,
                                        .type = RDMA_CONTROL_ERROR,
@@ -2213,7 +2214,7 @@ static void qemu_rdma_cleanup(RDMAContext *rdma)
             }
         }
         DDPRINTF("Disconnected.\n");
-        rdma->cm_id = NULL;
+        rdma->connected = false;
     }

     g_free(rdma->block);
@@ -2235,7 +2236,7 @@ static void qemu_rdma_cleanup(RDMAContext *rdma)
     }

     if (rdma->qp) {
-        ibv_destroy_qp(rdma->qp);
+        rdma_destroy_qp(rdma->cm_id);
         rdma->qp = NULL;
     }
     if (rdma->cq) {
@@ -2372,6 +2373,7 @@ static int qemu_rdma_connect(RDMAContext *rdma, Error **errp)
         rdma->cm_id = NULL;
         goto err_rdma_source_connect;
     }
+    rdma->connected = true;

     memcpy(&cap, cm_event->param.conn.private_data, sizeof(cap));
     network_to_caps(&cap);
@@ -2906,6 +2908,7 @@ static int qemu_rdma_accept(RDMAContext *rdma)
     }

     rdma_ack_cm_event(cm_event);
+    rdma->connected = true;

     ret = qemu_rdma_post_recv_control(rdma, RDMA_WRID_READY);
     if (ret) {
-- 
1.8.3.1

[Qemu-devel] [PATCH 5/9] rdma: constify ram_chunk_{index, start, end}

[Juan Quintela]

From: Isaku Yamahata <yamahata@private.email.ne.jp>

Signed-off-by: Isaku Yamahata <yamahata@private.email.ne.jp>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration-rdma.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/migration-rdma.c b/migration-rdma.c
index 3679acb..f94f3b4 100644
--- a/migration-rdma.c
+++ b/migration-rdma.c
@@ -511,19 +511,21 @@ static int qemu_rdma_exchange_send(RDMAContext *rdma, RDMAControlHeader *head,
                                    int *resp_idx,
                                    int (*callback)(RDMAContext *rdma));

-static inline uint64_t ram_chunk_index(uint8_t *start, uint8_t *host)
+static inline uint64_t ram_chunk_index(const uint8_t *start,
+                                       const uint8_t *host)
 {
     return ((uintptr_t) host - (uintptr_t) start) >> RDMA_REG_CHUNK_SHIFT;
 }

-static inline uint8_t *ram_chunk_start(RDMALocalBlock *rdma_ram_block,
+static inline uint8_t *ram_chunk_start(const RDMALocalBlock *rdma_ram_block,
                                        uint64_t i)
 {
     return (uint8_t *) (((uintptr_t) rdma_ram_block->local_host_addr)
                                     + (i << RDMA_REG_CHUNK_SHIFT));
 }

-static inline uint8_t *ram_chunk_end(RDMALocalBlock *rdma_ram_block, uint64_t i)
+static inline uint8_t *ram_chunk_end(const RDMALocalBlock *rdma_ram_block,
+                                     uint64_t i)
 {
     uint8_t *result = ram_chunk_start(rdma_ram_block, i) +
                                          (1UL << RDMA_REG_CHUNK_SHIFT);
-- 
1.8.3.1

[Qemu-devel] [PATCH 6/9] migration: add version supporting macros for struct pointer

[Juan Quintela]

From: Alexey Kardashevskiy <aik@ozlabs.ru>

This adds version supporting macros VMSTATE_STRUCT_POINTER_TEST_V
and VMSTATE_STRUCT_POINTER_V in addition to the already existing
VMSTATE_STRUCT_POINTER and VMSTATE_STRUCT_POINTER_TEST macros.

Cc: Andreas Färber <afaerber@suse.de>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 include/migration/vmstate.h | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 1c31b5d..9d09e60 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -310,8 +310,18 @@ extern const VMStateInfo vmstate_info_bitmap;
     .offset       = vmstate_offset_value(_state, _field, _type),     \
 }

-#define VMSTATE_STRUCT_POINTER_TEST(_field, _state, _test, _vmsd, _type) { \
+#define VMSTATE_STRUCT_POINTER_V(_field, _state, _version, _vmsd, _type) { \
     .name         = (stringify(_field)),                             \
+    .version_id   = (_version),                                        \
+    .vmsd         = &(_vmsd),                                        \
+    .size         = sizeof(_type),                                   \
+    .flags        = VMS_STRUCT|VMS_POINTER,                          \
+    .offset       = vmstate_offset_value(_state, _field, _type),     \
+}
+
+#define VMSTATE_STRUCT_POINTER_TEST_V(_field, _state, _test, _version, _vmsd, _type) { \
+    .name         = (stringify(_field)),                             \
+    .version_id   = (_version),                                        \
     .field_exists = (_test),                                         \
     .vmsd         = &(_vmsd),                                        \
     .size         = sizeof(_type),                                   \
@@ -497,7 +507,10 @@ extern const VMStateInfo vmstate_info_bitmap;
     VMSTATE_STRUCT_TEST(_field, _state, NULL, _version, _vmsd, _type)

 #define VMSTATE_STRUCT_POINTER(_field, _state, _vmsd, _type)          \
-    VMSTATE_STRUCT_POINTER_TEST(_field, _state, NULL, _vmsd, _type)
+    VMSTATE_STRUCT_POINTER_V(_field, _state, 0, _vmsd, _type)
+
+#define VMSTATE_STRUCT_POINTER_TEST(_field, _state, _test, _vmsd, _type)     \
+    VMSTATE_STRUCT_POINTER_TEST_V(_field, _state, _test, 0, _vmsd, _type)

 #define VMSTATE_STRUCT_ARRAY(_field, _state, _num, _version, _vmsd, _type) \
     VMSTATE_STRUCT_ARRAY_TEST(_field, _state, _num, NULL, _version,   \
-- 
1.8.3.1

[Qemu-devel] [PATCH 7/9] migration: Fix debug print type

[Juan Quintela]

From: Christoffer Dall <christoffer.dall@linaro.org>

The printf args are uint64_t and with -Werr QEMU doesn't compile with
migration debugging turned on unless this is fixed.  Fix it.

Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/migration.c b/migration.c
index 200d404..b4f8462 100644
--- a/migration.c
+++ b/migration.c
@@ -567,7 +567,8 @@ static void *migration_thread(void *opaque)
         if (!qemu_file_rate_limit(s->file)) {
             DPRINTF("iterate\n");
             pending_size = qemu_savevm_state_pending(s->file, max_size);
-            DPRINTF("pending size %lu max %lu\n", pending_size, max_size);
+            DPRINTF("pending size %" PRIu64 " max %" PRIu64 "\n",
+                    pending_size, max_size);
             if (pending_size && pending_size >= max_size) {
                 qemu_savevm_state_iterate(s->file);
             } else {
-- 
1.8.3.1

[Qemu-devel] [PATCH 8/9] arch_init: make is_zero_page accept size

[Juan Quintela]

From: Isaku Yamahata <yamahata@private.email.ne.jp>

Later is_zero_page will be used for non TARGET_PAGE_SIZE
range.
And rename it to is_zero_range as it isn't page size any more.

Signed-off-by: Isaku Yamahata <yamahata@private.email.ne.jp>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 arch_init.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 18cd9a1..c72790f 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -150,10 +150,9 @@ int qemu_read_default_config_files(bool userconfig)
     return 0;
 }

-static inline bool is_zero_page(uint8_t *p)
+static inline bool is_zero_range(uint8_t *p, uint64_t size)
 {
-    return buffer_find_nonzero_offset(p, TARGET_PAGE_SIZE) ==
-        TARGET_PAGE_SIZE;
+    return buffer_find_nonzero_offset(p, size) == size;
 }

 /* struct contains XBZRLE cache and a static page
@@ -497,7 +496,7 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
                         acct_info.dup_pages++;
                     }
                 }
-            } else if (is_zero_page(p)) {
+            } else if (is_zero_range(p, TARGET_PAGE_SIZE)) {
                 acct_info.dup_pages++;
                 bytes_sent = save_block_hdr(f, block, offset, cont,
                                             RAM_SAVE_FLAG_COMPRESS);
@@ -849,7 +848,7 @@ static inline void *host_from_stream_offset(QEMUFile *f,
  */
 void ram_handle_compressed(void *host, uint8_t ch, uint64_t size)
 {
-    if (ch != 0 || !is_zero_page(host)) {
+    if (ch != 0 || !is_zero_range(host, TARGET_PAGE_SIZE)) {
         memset(host, ch, size);
 #ifndef _WIN32
         if (ch == 0 &&
-- 
1.8.3.1

[Qemu-devel] [PATCH 9/9] migration: ram_handle_compressed

[Juan Quintela]

From: Isaku Yamahata <yamahata@private.email.ne.jp>

ram_handle_compressed() should be aware of size > TARGET_PAGE_SIZE.
migration-rdma can call it with larger size.

Signed-off-by: Isaku Yamahata <yamahata@private.email.ne.jp>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 arch_init.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index c72790f..d14457d 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -848,13 +848,14 @@ static inline void *host_from_stream_offset(QEMUFile *f,
  */
 void ram_handle_compressed(void *host, uint8_t ch, uint64_t size)
 {
-    if (ch != 0 || !is_zero_range(host, TARGET_PAGE_SIZE)) {
+    if (ch != 0 || !is_zero_range(host, size)) {
         memset(host, ch, size);
 #ifndef _WIN32
-        if (ch == 0 &&
-            (!kvm_enabled() || kvm_has_sync_mmu()) &&
-            getpagesize() <= TARGET_PAGE_SIZE) {
-            qemu_madvise(host, TARGET_PAGE_SIZE, QEMU_MADV_DONTNEED);
+        if (ch == 0 && (!kvm_enabled() || kvm_has_sync_mmu())) {
+            size = size & ~(getpagesize() - 1);
+            if (size > 0) {
+                qemu_madvise(host, size, QEMU_MADV_DONTNEED);
+            }
         }
 #endif
     }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 8/9] arch_init: make is_zero_page accept size

[Peter Lieven]

On 24.09.2013 14:03, Juan Quintela wrote:
> From: Isaku Yamahata <yamahata@private.email.ne.jp>
>
> Later is_zero_page will be used for non TARGET_PAGE_SIZE
> range.
> And rename it to is_zero_range as it isn't page size any more.
>
> Signed-off-by: Isaku Yamahata <yamahata@private.email.ne.jp>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>   arch_init.c | 9 ++++-----
>   1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index 18cd9a1..c72790f 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -150,10 +150,9 @@ int qemu_read_default_config_files(bool userconfig)
>       return 0;
>   }
>
> -static inline bool is_zero_page(uint8_t *p)
> +static inline bool is_zero_range(uint8_t *p, uint64_t size)
>   {
> -    return buffer_find_nonzero_offset(p, TARGET_PAGE_SIZE) ==
> -        TARGET_PAGE_SIZE;
> +    return buffer_find_nonzero_offset(p, size) == size;
>   }

this is buffer_is_zero(p, size); maybe this function should be used.

Peter