[Qemu-devel] [PULL 0/4] migration fixes

[Qemu-devel] [PULL 0/4] migration fixes

[Amit Shah]

The following changes since commit a648c137383d84bc4f95696e5293978d9541a26e:

  Merge remote-tracking branch 'remotes/kraxel/tags/pull-ui-20160309-1' into staging (2016-03-10 02:51:14 +0000)

are available in the git repository at:

  https://git.kernel.org/pub/scm/virt/qemu/amit/migration.git tags/migration-for-2.6-7

for you to fetch changes up to 32c3db5b26a1001dbe0763bdf72fdc8017c6b7b8:

  postcopy: Remove the x- (2016-03-11 17:53:59 +0530)

----------------------------------------------------------------
migration:
 - postcopy is no longer experimental
 - fix a use-after-free in postcopy
 - fix a compile warning

----------------------------------------------------------------


Denis V. Lunev (1):
  migration: fix use-after-free in loadvm_postcopy_handle_run_bh

Dr. David Alan Gilbert (2):
  postcopy: listen thread is never joined
  postcopy: Remove the x-

Peter Xu (1):
  migration: fix warning for source_return_path_thread

 docs/migration.txt    |  2 +-
 hmp-commands.hx       |  2 +-
 migration/migration.c |  9 ++++-----
 migration/savevm.c    | 18 +++++++++++++-----
 qapi-schema.json      |  8 ++++----
 qmp-commands.hx       |  6 +++---
 6 files changed, 26 insertions(+), 19 deletions(-)

-- 
2.5.0

[Qemu-devel] [PULL 1/4] migration: fix warning for source_return_path_thread

[Amit Shah]

From: Peter Xu <peterx@redhat.com>

max_len is not necessary, while it brings a warning during compilation
when specify "-Wstack-usage=1000000". Replacing using sizeof().

Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <1457503932-31763-1-git-send-email-peterx@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 migration/migration.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 7d13377..a858159 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1269,8 +1269,7 @@ static void *source_return_path_thread(void *opaque)
     MigrationState *ms = opaque;
     QEMUFile *rp = ms->rp_state.from_dst_file;
     uint16_t header_len, header_type;
-    const int max_len = 512;
-    uint8_t buf[max_len];
+    uint8_t buf[512];
     uint32_t tmp32, sibling_error;
     ram_addr_t start = 0; /* =0 to silence warning */
     size_t  len = 0, expected_len;
@@ -1293,7 +1292,7 @@ static void *source_return_path_thread(void *opaque)
 
         if ((rp_cmd_args[header_type].len != -1 &&
             header_len != rp_cmd_args[header_type].len) ||
-            header_len > max_len) {
+            header_len > sizeof(buf)) {
             error_report("RP: Received '%s' message (0x%04x) with"
                     "incorrect length %d expecting %zu",
                     rp_cmd_args[header_type].name, header_type, header_len,
-- 
2.5.0

[Qemu-devel] [PULL 2/4] migration: fix use-after-free in loadvm_postcopy_handle_run_bh

[Amit Shah]

From: "Denis V. Lunev" <den@openvz.org>

MigrationState is destroyed before we can come into bottom half.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Amit Shah <amit.shah@redhat.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <1457537708-8622-1-git-send-email-den@openvz.org>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 migration/savevm.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 96e7db5..384e872 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1501,10 +1501,15 @@ static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
     return 0;
 }
 
+
+typedef struct {
+    QEMUBH *bh;
+} HandleRunBhData;
+
 static void loadvm_postcopy_handle_run_bh(void *opaque)
 {
     Error *local_err = NULL;
-    MigrationIncomingState *mis = opaque;
+    HandleRunBhData *data = opaque;
 
     /* TODO we should move all of this lot into postcopy_ram.c or a shared code
      * in migration.c
@@ -1532,13 +1537,15 @@ static void loadvm_postcopy_handle_run_bh(void *opaque)
         runstate_set(RUN_STATE_PAUSED);
     }
 
-    qemu_bh_delete(mis->bh);
+    qemu_bh_delete(data->bh);
+    g_free(data);
 }
 
 /* After all discards we can start running and asking for pages */
 static int loadvm_postcopy_handle_run(MigrationIncomingState *mis)
 {
     PostcopyState ps = postcopy_state_set(POSTCOPY_INCOMING_RUNNING);
+    HandleRunBhData *data;
 
     trace_loadvm_postcopy_handle_run();
     if (ps != POSTCOPY_INCOMING_LISTENING) {
@@ -1546,8 +1553,9 @@ static int loadvm_postcopy_handle_run(MigrationIncomingState *mis)
         return -1;
     }
 
-    mis->bh = qemu_bh_new(loadvm_postcopy_handle_run_bh, NULL);
-    qemu_bh_schedule(mis->bh);
+    data = g_new(HandleRunBhData, 1);
+    data->bh = qemu_bh_new(loadvm_postcopy_handle_run_bh, data);
+    qemu_bh_schedule(data->bh);
 
     /* We need to finish reading the stream from the package
      * and also stop reading anything more from the stream that loaded the
-- 
2.5.0

[Qemu-devel] [PULL 3/4] postcopy: listen thread is never joined

[Amit Shah]

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

We don't join the listen thread, it does its own cleanup.
Mark as detached not joinable.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reported-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1457690016-9070-2-git-send-email-dgilbert@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 migration/savevm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 384e872..0a33c22 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1494,7 +1494,7 @@ static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
     qemu_sem_init(&mis->listen_thread_sem, 0);
     qemu_thread_create(&mis->listen_thread, "postcopy/listen",
                        postcopy_ram_listen_thread, mis->from_src_file,
-                       QEMU_THREAD_JOINABLE);
+                       QEMU_THREAD_DETACHED);
     qemu_sem_wait(&mis->listen_thread_sem);
     qemu_sem_destroy(&mis->listen_thread_sem);
 
-- 
2.5.0

[Qemu-devel] [PULL 4/4] postcopy: Remove the x-

[Amit Shah]

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

Postcopy seems to have survived a cycle with only a few fixes,
and Jiri has the current libvirt wired up and working
( https://www.redhat.com/archives/libvir-list/2016-March/msg00080.html )
so remove the experimental tag.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1457690016-9070-3-git-send-email-dgilbert@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 docs/migration.txt    | 2 +-
 hmp-commands.hx       | 2 +-
 migration/migration.c | 4 ++--
 qapi-schema.json      | 8 ++++----
 qmp-commands.hx       | 6 +++---
 5 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/docs/migration.txt b/docs/migration.txt
index fda8d61..90209ab 100644
--- a/docs/migration.txt
+++ b/docs/migration.txt
@@ -333,7 +333,7 @@ doesn't finish in a given time the switch is made to postcopy.
 To enable postcopy, issue this command on the monitor prior to the
 start of migration:
 
-migrate_set_capability x-postcopy-ram on
+migrate_set_capability postcopy-ram on
 
 The normal commands are then used to start a migration, which is still
 started in precopy mode.  Issuing:
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 664d794..639205b 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1026,7 +1026,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "Followup to a migration command to switch the migration"
-                      " to postcopy mode. The x-postcopy-ram capability must "
+                      " to postcopy mode. The 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 a858159..034a918 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -706,7 +706,7 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
              */
             error_report("Postcopy is not currently compatible with "
                          "compression");
-            s->enabled_capabilities[MIGRATION_CAPABILITY_X_POSTCOPY_RAM] =
+            s->enabled_capabilities[MIGRATION_CAPABILITY_POSTCOPY_RAM] =
                 false;
         }
     }
@@ -1125,7 +1125,7 @@ bool migrate_postcopy_ram(void)
 
     s = migrate_get_current();
 
-    return s->enabled_capabilities[MIGRATION_CAPABILITY_X_POSTCOPY_RAM];
+    return s->enabled_capabilities[MIGRATION_CAPABILITY_POSTCOPY_RAM];
 }
 
 bool migrate_auto_converge(void)
diff --git a/qapi-schema.json b/qapi-schema.json
index 362c9d8..6269c37 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -540,15 +540,15 @@
 # @auto-converge: If enabled, QEMU will automatically throttle down the guest
 #          to speed up convergence of RAM migration. (since 1.6)
 #
-# @x-postcopy-ram: Start executing on the migration target before all of RAM has
+# @postcopy-ram: Start executing on the migration target before all of RAM has
 #          been migrated, pulling the remaining pages along as needed. NOTE: If
-#          the migration fails during postcopy the VM will fail.  (since 2.5)
+#          the migration fails during postcopy the VM will fail.  (since 2.6)
 #
 # Since: 1.2
 ##
 { 'enum': 'MigrationCapability',
   'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks',
-           'compress', 'events', 'x-postcopy-ram'] }
+           'compress', 'events', 'postcopy-ram'] }
 
 ##
 # @MigrationCapabilityStatus
@@ -705,7 +705,7 @@
 # @migrate-start-postcopy
 #
 # Followup to a migration command to switch the migration to postcopy mode.
-# The x-postcopy-ram capability must be set before the original migration
+# The postcopy-ram capability must be set before the original migration
 # command.
 #
 # Since: 2.5
diff --git a/qmp-commands.hx b/qmp-commands.hx
index b629673..9e05365 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3683,7 +3683,7 @@ Enable/Disable migration capabilities
 - "zero-blocks": compress zero blocks during block migration
 - "compress": use multiple compression threads to accelerate live migration
 - "events": generate events for each migration state change
-- "x-postcopy-ram": postcopy mode for live migration
+- "postcopy-ram": postcopy mode for live migration
 
 Arguments:
 
@@ -3713,7 +3713,7 @@ Query current migration capabilities
          - "zero-blocks" : Zero Blocks state (json-bool)
          - "compress": Multiple compression threads state (json-bool)
          - "events": Migration state change event state (json-bool)
-         - "x-postcopy-ram": postcopy ram state (json-bool)
+         - "postcopy-ram": postcopy ram state (json-bool)
 
 Arguments:
 
@@ -3727,7 +3727,7 @@ Example:
      {"state": false, "capability": "zero-blocks"},
      {"state": false, "capability": "compress"},
      {"state": true, "capability": "events"},
-     {"state": false, "capability": "x-postcopy-ram"}
+     {"state": false, "capability": "postcopy-ram"}
    ]}
 
 EQMP
-- 
2.5.0

Re: [Qemu-devel] [PULL 0/4] migration fixes

[Peter Maydell]

On 11 March 2016 at 12:32, Amit Shah <amit.shah@redhat.com> wrote:
> The following changes since commit a648c137383d84bc4f95696e5293978d9541a26e:
>
>   Merge remote-tracking branch 'remotes/kraxel/tags/pull-ui-20160309-1' into staging (2016-03-10 02:51:14 +0000)
>
> are available in the git repository at:
>
>   https://git.kernel.org/pub/scm/virt/qemu/amit/migration.git tags/migration-for-2.6-7
>
> for you to fetch changes up to 32c3db5b26a1001dbe0763bdf72fdc8017c6b7b8:
>
>   postcopy: Remove the x- (2016-03-11 17:53:59 +0530)
>
> ----------------------------------------------------------------
> migration:
>  - postcopy is no longer experimental
>  - fix a use-after-free in postcopy
>  - fix a compile warning
>
> ----------------------------------------------------------------
>

Applied, thanks.

-- PMM