[Qemu-devel] [PULL 0/6] Migration pull request

[Qemu-devel] [PULL 0/6] Migration pull request

[Juan Quintela]

Hi

- fix for mutex initialzation (dave)
- different page sizes fixes (dave)
- better text (dave)
- Migration 32bits fixes (myself)

Please, apply.

The following changes since commit ed6c64489ef11d9ac5fb4b4c89d455a4f1ae8083:

  target-arm: Update PC before calling gen_helper_check_breakpoints() (2015-11-12 16:16:02 +0000)

are available in the git repository at:

  git://github.com/juanquintela/qemu.git tags/migration/20151112

for you to fetch changes up to 389775d1f67b2c8f44f9473b1e5363735972e389:

  migration_init: Fix lock initialisation/make it explicit (2015-11-12 17:55:27 +0100)

----------------------------------------------------------------
migration/next for 20151112

----------------------------------------------------------------
Dr. David Alan Gilbert (4):
      Finish non-postcopiable iterative devices before package
      Postcopy: Fix TP!=HP zero case
      migrate-start-postcopy: Improve text
      migration_init: Fix lock initialisation/make it explicit

Juan Quintela (2):
      migration: print ram_addr_t as RAM_ADDR_FMT not %zx
      migration: Make 32bit linux compile with RDMA

 hmp-commands.hx         |  4 +++-
 include/sysemu/sysemu.h |  2 +-
 migration/migration.c   | 63 ++++++++++++++++++++++++-------------------------
 migration/ram.c         |  7 +++---
 migration/rdma.c        |  2 +-
 migration/savevm.c      | 10 ++++++--
 qapi-schema.json        |  4 +++-
 7 files changed, 51 insertions(+), 41 deletions(-)

[Qemu-devel] [PULL 1/6] migration: print ram_addr_t as RAM_ADDR_FMT not %zx

[Juan Quintela]

Not all the wold is 64bits (yet).

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/ram.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 62cf42b..d8d5a50 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1184,7 +1184,8 @@ int ram_save_queue_pages(MigrationState *ms, const char *rbname,
     }
     trace_ram_save_queue_pages(ramblock->idstr, start, len);
     if (start+len > ramblock->used_length) {
-        error_report("%s request overrun start=%zx len=%zx blocklen=%zx",
+        error_report("%s request overrun start=" RAM_ADDR_FMT " len="
+                     RAM_ADDR_FMT " blocklen=" RAM_ADDR_FMT,
                      __func__, start, len, ramblock->used_length);
         goto err;
     }
@@ -1845,7 +1846,7 @@ int ram_discard_range(MigrationIncomingState *mis,
         ret = postcopy_ram_discard_range(mis, host_startaddr, length);
     } else {
         error_report("ram_discard_range: Overrun block '%s' (%" PRIu64
-                     "/%zu/%zu)",
+                     "/%zx/" RAM_ADDR_FMT")",
                      block_name, start, length, rb->used_length);
     }

-- 
2.5.0

[Qemu-devel] [PULL 2/6] migration: Make 32bit linux compile with RDMA

[Juan Quintela]

Rest of the file already use that trick. 64bit offsets make no sense in
32bit archs, but that is ram_addr_t for you.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/rdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index 553fbd7..dcabb91 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -577,7 +577,7 @@ static int rdma_add_block(RDMAContext *rdma, const char *block_name,
     block->is_ram_block = local->init ? false : true;

     if (rdma->blockmap) {
-        g_hash_table_insert(rdma->blockmap, (void *) block_offset, block);
+        g_hash_table_insert(rdma->blockmap, (void *)(uintptr_t)block_offset, block);
     }

     trace_rdma_add_block(block_name, local->nb_blocks,
-- 
2.5.0

[Qemu-devel] [PULL 3/6] Finish non-postcopiable iterative devices before package

[Juan Quintela]

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

Where we have iterable, but non-postcopiable devices (e.g. htab
or block migration), complete them before forming the 'package'
but with the CPUs stopped.  This stops them filling up the package.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 include/sysemu/sysemu.h |  2 +-
 migration/migration.c   | 10 ++++++++--
 migration/savevm.c      | 10 ++++++++--
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index f992494..3bb8897 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -112,7 +112,7 @@ void qemu_savevm_state_header(QEMUFile *f);
 int qemu_savevm_state_iterate(QEMUFile *f, bool postcopy);
 void qemu_savevm_state_cleanup(void);
 void qemu_savevm_state_complete_postcopy(QEMUFile *f);
-void qemu_savevm_state_complete_precopy(QEMUFile *f);
+void qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only);
 void qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size,
                                uint64_t *res_non_postcopiable,
                                uint64_t *res_postcopiable);
diff --git a/migration/migration.c b/migration/migration.c
index c5c977e..5df490a 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1429,6 +1429,12 @@ static int postcopy_start(MigrationState *ms, bool *old_vm_running)
     }

     /*
+     * Cause any non-postcopiable, but iterative devices to
+     * send out their final data.
+     */
+    qemu_savevm_state_complete_precopy(ms->file, true);
+
+    /*
      * in Finish migrate and with the io-lock held everything should
      * be quiet, but we've potentially still got dirty pages and we
      * need to tell the destination to throw any pages it's already received
@@ -1471,7 +1477,7 @@ static int postcopy_start(MigrationState *ms, bool *old_vm_running)
      */
     qemu_savevm_send_postcopy_listen(fb);

-    qemu_savevm_state_complete_precopy(fb);
+    qemu_savevm_state_complete_precopy(fb, false);
     qemu_savevm_send_ping(fb, 3);

     qemu_savevm_send_postcopy_run(fb);
@@ -1538,7 +1544,7 @@ static void migration_completion(MigrationState *s, int current_active_state,
             ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
             if (ret >= 0) {
                 qemu_file_set_rate_limit(s->file, INT64_MAX);
-                qemu_savevm_state_complete_precopy(s->file);
+                qemu_savevm_state_complete_precopy(s->file, false);
             }
         }
         qemu_mutex_unlock_iothread();
diff --git a/migration/savevm.c b/migration/savevm.c
index be52314..d90e228 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1026,7 +1026,7 @@ void qemu_savevm_state_complete_postcopy(QEMUFile *f)
     qemu_fflush(f);
 }

-void qemu_savevm_state_complete_precopy(QEMUFile *f)
+void qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only)
 {
     QJSON *vmdesc;
     int vmdesc_len;
@@ -1041,9 +1041,11 @@ void qemu_savevm_state_complete_precopy(QEMUFile *f)
     QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
         if (!se->ops ||
             (in_postcopy && se->ops->save_live_complete_postcopy) ||
+            (in_postcopy && !iterable_only) ||
             !se->ops->save_live_complete_precopy) {
             continue;
         }
+
         if (se->ops && se->ops->is_active) {
             if (!se->ops->is_active(se->opaque)) {
                 continue;
@@ -1062,6 +1064,10 @@ void qemu_savevm_state_complete_precopy(QEMUFile *f)
         }
     }

+    if (iterable_only) {
+        return;
+    }
+
     vmdesc = qjson_new();
     json_prop_int(vmdesc, "page_size", TARGET_PAGE_SIZE);
     json_start_array(vmdesc, "devices");
@@ -1176,7 +1182,7 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)

     ret = qemu_file_get_error(f);
     if (ret == 0) {
-        qemu_savevm_state_complete_precopy(f);
+        qemu_savevm_state_complete_precopy(f, false);
         ret = qemu_file_get_error(f);
     }
     qemu_savevm_state_cleanup();
-- 
2.5.0

[Qemu-devel] [PULL 4/6] Postcopy: Fix TP!=HP zero case

[Juan Quintela]

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

Where the target page size is different from the host page
we special case it, but I messed up on the zero case check.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/ram.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/ram.c b/migration/ram.c
index d8d5a50..7f32696 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2274,6 +2274,7 @@ static int ram_load_postcopy(QEMUFile *f)
     /* Temporary page that is later 'placed' */
     void *postcopy_host_page = postcopy_get_tmp_page(mis);
     void *last_host = NULL;
+    bool all_zero = false;

     while (!ret && !(flags & RAM_SAVE_FLAG_EOS)) {
         ram_addr_t addr;
@@ -2281,7 +2282,6 @@ static int ram_load_postcopy(QEMUFile *f)
         void *page_buffer = NULL;
         void *place_source = NULL;
         uint8_t ch;
-        bool all_zero = false;

         addr = qemu_get_be64(f);
         flags = addr & ~TARGET_PAGE_MASK;
-- 
2.5.0

[Qemu-devel] [PULL 5/6] migrate-start-postcopy: Improve text

[Juan Quintela]

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

Improve the text in both the qapi-schema and hmp help to point out
you need to set the postcopy-ram capability prior to issuing
migrate-start-postcopy.

Also fix the text of the migrate_start_postcopy error that
deals with capabilities.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Acked-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hmp-commands.hx       | 4 +++-
 migration/migration.c | 2 +-
 qapi-schema.json      | 4 +++-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index f3de407..bb52e4d 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1025,7 +1025,9 @@ ETEXI
         .name       = "migrate_start_postcopy",
         .args_type  = "",
         .params     = "",
-        .help       = "Switch migration to postcopy mode",
+        .help       = "Followup to a migration command to switch the migration"
+                      " to postcopy mode. The x-postcopy-ram capability must "
+                      "be set before the original migration command.",
         .mhandler.cmd = hmp_migrate_start_postcopy,
     },

diff --git a/migration/migration.c b/migration/migration.c
index 5df490a..9bd2ce7 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -768,7 +768,7 @@ void qmp_migrate_start_postcopy(Error **errp)
     MigrationState *s = migrate_get_current();

     if (!migrate_postcopy_ram()) {
-        error_setg(errp, "Enable postcopy with migration_set_capability before"
+        error_setg(errp, "Enable postcopy with migrate_set_capability before"
                          " the start of migration");
         return;
     }
diff --git a/qapi-schema.json b/qapi-schema.json
index c3f95ab..a1514ca 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -706,7 +706,9 @@
 ##
 # @migrate-start-postcopy
 #
-# Switch migration to postcopy mode
+# Followup to a migration command to switch the migration to postcopy mode.
+# The x-postcopy-ram capability must be set before the original migration
+# command.
 #
 # Since: 2.5
 { 'command': 'migrate-start-postcopy' }
-- 
2.5.0

[Qemu-devel] [PULL 6/6] migration_init: Fix lock initialisation/make it explicit

[Juan Quintela]

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

Peter reported a lock error on MacOS after my a82d593b
patch.

migrate_get_current does one-time initialisation of
a bunch of variables.
migrate_init does reinitialisation even on a 2nd
migrate after a cancel.

The problem here was that I'd initialised the mutex
in migrate_get_current, and the memset in migrate_init
corrupted it.

Remove the memset and replace it by explicit initialisation
of fields that need initialising; this also turns out to be simpler
than the old code that had to preserve some fields.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Fixes: a82d593b
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/migration.c | 51 ++++++++++++++++++++++-----------------------------
 1 file changed, 22 insertions(+), 29 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 9bd2ce7..7e4e27b 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -902,38 +902,31 @@ bool migration_in_postcopy(MigrationState *s)
 MigrationState *migrate_init(const MigrationParams *params)
 {
     MigrationState *s = migrate_get_current();
-    int64_t bandwidth_limit = s->bandwidth_limit;
-    bool enabled_capabilities[MIGRATION_CAPABILITY_MAX];
-    int64_t xbzrle_cache_size = s->xbzrle_cache_size;
-    int compress_level = s->parameters[MIGRATION_PARAMETER_COMPRESS_LEVEL];
-    int compress_thread_count =
-            s->parameters[MIGRATION_PARAMETER_COMPRESS_THREADS];
-    int decompress_thread_count =
-            s->parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS];
-    int x_cpu_throttle_initial =
-            s->parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INITIAL];
-    int x_cpu_throttle_increment =
-            s->parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INCREMENT];

-    memcpy(enabled_capabilities, s->enabled_capabilities,
-           sizeof(enabled_capabilities));
-
-    memset(s, 0, sizeof(*s));
+    /*
+     * Reinitialise all migration state, except
+     * parameters/capabilities that the user set, and
+     * locks.
+     */
+    s->bytes_xfer = 0;
+    s->xfer_limit = 0;
+    s->cleanup_bh = 0;
+    s->file = NULL;
+    s->state = MIGRATION_STATUS_NONE;
     s->params = *params;
-    memcpy(s->enabled_capabilities, enabled_capabilities,
-           sizeof(enabled_capabilities));
-    s->xbzrle_cache_size = xbzrle_cache_size;
+    s->rp_state.from_dst_file = NULL;
+    s->rp_state.error = false;
+    s->mbps = 0.0;
+    s->downtime = 0;
+    s->expected_downtime = 0;
+    s->dirty_pages_rate = 0;
+    s->dirty_bytes_rate = 0;
+    s->setup_time = 0;
+    s->dirty_sync_count = 0;
+    s->start_postcopy = false;
+    s->migration_thread_running = false;
+    s->last_req_rb = NULL;

-    s->parameters[MIGRATION_PARAMETER_COMPRESS_LEVEL] = compress_level;
-    s->parameters[MIGRATION_PARAMETER_COMPRESS_THREADS] =
-               compress_thread_count;
-    s->parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS] =
-               decompress_thread_count;
-    s->parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INITIAL] =
-                x_cpu_throttle_initial;
-    s->parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INCREMENT] =
-                x_cpu_throttle_increment;
-    s->bandwidth_limit = bandwidth_limit;
     migrate_set_state(s, MIGRATION_STATUS_NONE, MIGRATION_STATUS_SETUP);

     QSIMPLEQ_INIT(&s->src_page_requests);
-- 
2.5.0

Re: [Qemu-devel] [PULL 0/6] Migration pull request

[Peter Maydell]

On 12 November 2015 at 17:20, Juan Quintela <quintela@redhat.com> wrote:
> Hi
>
> - fix for mutex initialzation (dave)
> - different page sizes fixes (dave)
> - better text (dave)
> - Migration 32bits fixes (myself)
>
> Please, apply.
>
> The following changes since commit ed6c64489ef11d9ac5fb4b4c89d455a4f1ae8083:
>
>   target-arm: Update PC before calling gen_helper_check_breakpoints() (2015-11-12 16:16:02 +0000)
>
> are available in the git repository at:
>
>   git://github.com/juanquintela/qemu.git tags/migration/20151112
>
> for you to fetch changes up to 389775d1f67b2c8f44f9473b1e5363735972e389:
>
>   migration_init: Fix lock initialisation/make it explicit (2015-11-12 17:55:27 +0100)
>
> ----------------------------------------------------------------
> migration/next for 20151112
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM