Skip to contentSkip to conversation

migration: Introduce MIGRATION_STATUS_FINISHING

PATCH In review Pavel Fedin 2 participants 5 messages Oct 26, 2015 – Oct 27, 2015

lore
Share & tools
lore

Pavel Fedin

author
#0 lore raw

PATCH 1/1 migration: Introduce MIGRATION_STATUS_FINISHING

This status is set after vm_stop_force_state(), and is telling us that all CPUs are stopped, and we are finishing up with the migration.

Also, call notifier_list_notify() when this status is set. This will be necessary for ITS live migration on ARM, which will have to dump its state into guest RAM at this point.

Signed-off-by Pavel Fedin <[email protected]>
---
 hmp.c                         |  1 +
 include/migration/migration.h |  1 +
 migration/migration.c         | 19 ++++++++++++++++---
 qapi-schema.json              |  4 +++-
 4 files changed, 21 insertions(+), 4 deletions(-)
hmp.c +1 -0
index 5048eee..202768f 100644
@@ -1467,6 +1467,7 @@ static void hmp_migrate_status_cb(void *opaque)
 
     info = qmp_query_migrate(NULL);
     if (!info->has_status || info->status == MIGRATION_STATUS_ACTIVE ||
+        info->status == MIGRATION_STATUS_FINISHING ||
         info->status == MIGRATION_STATUS_SETUP) {
         if (info->has_disk) {
             int progress;
include/migration/migration.h +1 -0
index 8334621..ff4bfcb 100644
@@ -117,6 +117,7 @@ int migrate_fd_close(MigrationState *s);
 void add_migration_state_change_notifier(Notifier *notify);
 void remove_migration_state_change_notifier(Notifier *notify);
 bool migration_in_setup(MigrationState *);
+bool migration_in_finishing(MigrationState *);
 bool migration_has_finished(MigrationState *);
 bool migration_has_failed(MigrationState *);
 MigrationState *migrate_get_current(void);
migration/migration.c +16 -3
index b092f38..8f28585 100644
@@ -427,6 +427,7 @@ MigrationInfo *qmp_query_migrate(Error **errp)
         info->has_total_time = false;
         break;
     case MIGRATION_STATUS_ACTIVE:
+    case MIGRATION_STATUS_FINISHING:
     case MIGRATION_STATUS_CANCELLING:
         info->has_status = true;
         info->has_total_time = true;

@@ -507,6 +508,7 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
     MigrationCapabilityStatusList *cap;
 
     if (s->state == MIGRATION_STATUS_ACTIVE ||
+        s->state == MIGRATION_STATUS_FINISHING ||
         s->state == MIGRATION_STATUS_SETUP) {
         error_setg(errp, QERR_MIGRATION_ACTIVE);
         return;

@@ -641,7 +643,8 @@ static void migrate_fd_cancel(MigrationState *s)
     do {
         old_state = s->state;
         if (old_state != MIGRATION_STATUS_SETUP &&
-            old_state != MIGRATION_STATUS_ACTIVE) {
+            old_state != MIGRATION_STATUS_ACTIVE &&
+            old_state != MIGRATION_STATUS_FINISHING) {
             break;
         }
         migrate_set_state(s, old_state, MIGRATION_STATUS_CANCELLING);

@@ -674,6 +677,11 @@ bool migration_in_setup(MigrationState *s)
     return s->state == MIGRATION_STATUS_SETUP;
 }
 
+bool migration_in_finishing(MigrationState *s)
+{
+    return s->state == MIGRATION_STATUS_FINISHING;
+}
+
 bool migration_has_finished(MigrationState *s)
 {
     return s->state == MIGRATION_STATUS_COMPLETED;

@@ -774,6 +782,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
     params.shared = has_inc && inc;
 
     if (s->state == MIGRATION_STATUS_ACTIVE ||
+        s->state == MIGRATION_STATUS_FINISHING ||
         s->state == MIGRATION_STATUS_SETUP ||
         s->state == MIGRATION_STATUS_CANCELLING) {
         error_setg(errp, QERR_MIGRATION_ACTIVE);

@@ -985,6 +994,7 @@ int64_t migrate_xbzrle_cache_size(void)
 static void migration_completion(MigrationState *s, bool *old_vm_running,
                                  int64_t *start_time)
 {
+    MigrationStatus state = MIGRATION_STATUS_ACTIVE;
     int ret;
 
     qemu_mutex_lock_iothread();

@@ -996,6 +1006,9 @@ static void migration_completion(MigrationState *s, bool *old_vm_running,
     if (!ret) {
         ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
         if (ret >= 0) {
+            state = MIGRATION_STATUS_FINISHING;
+            migrate_set_state(s, MIGRATION_STATUS_ACTIVE, state);
+            notifier_list_notify(&migration_state_notifiers, s);
             qemu_file_set_rate_limit(s->file, INT64_MAX);
             qemu_savevm_state_complete(s->file);
         }

@@ -1011,11 +1024,11 @@ static void migration_completion(MigrationState *s, bool *old_vm_running,
         goto fail;
     }
 
-    migrate_set_state(s, MIGRATION_STATUS_ACTIVE, MIGRATION_STATUS_COMPLETED);
+    migrate_set_state(s, state, MIGRATION_STATUS_COMPLETED);
     return;
 
 fail:
-    migrate_set_state(s, MIGRATION_STATUS_ACTIVE, MIGRATION_STATUS_FAILED);
+    migrate_set_state(s, state, MIGRATION_STATUS_FAILED);
 }
 
 /* migration thread support */
qapi-schema.json +3 -1
index f60be29..c6226da 100644
@@ -430,6 +430,8 @@
 #
 # @active: in the process of doing migration.
 #
+# @finishing: migration is being finished, CPUs have been stopped.
+#
 # @completed: migration is finished.
 #
 # @failed: some error occurred during migration process.

@@ -439,7 +441,7 @@
 ##
 { 'enum': 'MigrationStatus',
   'data': [ 'none', 'setup', 'cancelling', 'cancelled',
-            'active', 'completed', 'failed' ] }
+            'active', 'finishing', 'completed', 'failed' ] }
 
 ##
 # @MigrationInfo
Signature · 1 line
1.9.5.msysgit.0
#1 lore raw
Quoted 9 lines · Pavel Fedin

This status is set after vm_stop_force_state(), and is telling us that all CPUs are stopped, and we are finishing up with the migration.

Also, call notifier_list_notify() when this status is set. This will be necessary for ITS live migration on ARM, which will have to dump its state into guest RAM at this point.

Signed-off-by Pavel Fedin <[email protected]>

---

Interface review only:

qapi-schema.json line 434

Quoted 7 lines

+++ b/qapi-schema.json

qapi-schema.json +2 -0
@@ -430,6 +430,8 @@
 #
 # @active: in the process of doing migration.
 #
+# @finishing: migration is being finished, CPUs have been stopped.
+#

Missing a '(since 2.5)' notation. Also, adding new user-visible states has a tendency to break existing clients that aren't prepared for unexpected states (although technically such bugs are in the client - in the past, libvirt used to be one such client, although we've tried to fix it to gracefully ignore unknown states). Are we sure no other existing state works for this? I'm not opposed to the addition, just wanting to make sure we have good reason for it.

One design idea for adding new states is making sure the new state will not be exposed unless the client specifies some new option to enable the state, so that old clients will never see the state and new clients have expressed their interest in the state by opting-in to it with the new option.

Quoted 13 lines
qapi-schema.json +1 -1
 # @completed: migration is finished.
 #
 # @failed: some error occurred during migration process.

@@ -439,7 +441,7 @@
 ##
 { 'enum': 'MigrationStatus',
   'data': [ 'none', 'setup', 'cancelling', 'cancelled',
-            'active', 'completed', 'failed' ] }
+            'active', 'finishing', 'completed', 'failed' ] }
 
 ##
 # @MigrationInfo
Signature · 2 lines
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org
author
#2 lore raw

Hello!

Quoted 5 lines

adding new user-visible states
has a tendency to break existing clients that aren't prepared for unexpected states (although technically such bugs are in the client - in the past, libvirt used to be one such client, although we've tried to fix it to gracefully ignore unknown states).

Yes, i know this, my host uses libvirt v1.2.9.3 (i backport necessary patches to it) and it barfed. I didn't check master though...

Quoted 3 lines

Are we sure no other
existing state works for this? I'm not opposed to the addition, just wanting to make sure we have good reason for it.

You know, actually, this is a thing only for qemu's internal use, we don't need a new state at all. Instead, we could introduce a 'bool cpus_stopped' to MigrationState structure and test for it in migration_in_finishing(), like:
--- cut ---
bool migration_in_finishing(MigrationState *s)

{
    return s->cpus_stopped;
}

--- cut ---
What do you think about this solution? It is much less complicated than...

Quoted 4 lines

One design idea for adding new states is making sure the new state will not be exposed unless the client specifies some new option to enable the state, so that old clients will never see the state and new clients have expressed their interest in the state

With this kind of approach we would not be able to migrate ITS without this option. Because it's not external clients being interested in this state at the moment, but qemu itself.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

#3 lore raw
Quoted 14 lines · Pavel Fedin

Hello!

Quoted 5 lines

adding new user-visible states
has a tendency to break existing clients that aren't prepared for unexpected states (although technically such bugs are in the client - in the past, libvirt used to be one such client, although we've tried to fix it to gracefully ignore unknown states).

Yes, i know this, my host uses libvirt v1.2.9.3 (i backport necessary patches to it) and it barfed. I didn't check master though...

Quoted 3 lines

Are we sure no other
existing state works for this? I'm not opposed to the addition, just wanting to make sure we have good reason for it.

Quoted 8 lines

You know, actually, this is a thing only for qemu's internal use, we don't need a new state at all. Instead, we could introduce a 'bool cpus_stopped' to MigrationState structure and test for it in migration_in_finishing(), like:
--- cut ---
bool migration_in_finishing(MigrationState *s)

{
    return s->cpus_stopped;
}

--- cut ---
What do you think about this solution? It is much less complicated than...

If it is truly internal, then avoiding exposing the state to external clients is indeed the way to go.

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