Migration notifiers will notify at any of three places: (1) SETUP
phase, (2) migration completes, (3) migration fails.
There's actually a special case for spice: one can refer to
b82fc321bf ("Postcopy+spice: Pass spice migration data earlier"). It
doesn't need another 4th event because in commit 9d9babf78d ("migration:
MigrationEvent for notifiers") we merged it together with the DONE event.
The merge makes some sense if we treat "switchover" of postcopy as "DONE",
however that also means for postcopy we'll notify DONE twice.. The other
one at the end of postcopy when migration_cleanup().
In reality, the current code base will also notify FAILED for postcopy
twice. It's because an (maybe accidental) change in commit
4af667f87c ("migration: notifier error checking").
First of all, we still need that notification when switchover as stated in
Dave's commit, however that's only needed for spice. To fix it, introduce
POSTCOPY_START event to differenciate it from DONE. Use that instead in
postcopy_start(). Then spice will need to capture this event too.
Then we remove the extra FAILED notification in postcopy_start().
If one wonder if other DONE users should also monitor POSTCOPY_START
event.. We have two more DONE users:
- kvm_arm_gicv3_notifier
- cpr_exec_notifier
Both of them do not need a notification for POSTCOPY_START, but only when
migration completed. Actually, both of them are used in CPR, which doesn't
support postcopy.
I didn't attach Fixes: because I am not aware of any real bug on such
double reporting. I'm wildly guessing the 2nd notify might be silently
ignored in many cases. However this is still worth fixing.
Cc: Marc-André Lureau <[email protected]>
Cc: Dr. David Alan Gilbert <[email protected]>
Signed-off-by: Peter Xu <[email protected]>
---
include/migration/misc.h | 1 +
migration/migration.c | 3 +--
ui/spice-core.c | 3 ++-
3 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/include/migration/misc.h b/include/migration/misc.h
index e26d418a6e..b002466e10 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -63,6 +63,7 @@ typedef enum MigrationEventType {
MIG_EVENT_PRECOPY_SETUP,
MIG_EVENT_PRECOPY_DONE,
MIG_EVENT_PRECOPY_FAILED,
+ MIG_EVENT_POSTCOPY_START,
MIG_EVENT_MAX
} MigrationEventType;
diff --git a/migration/migration.c b/migration/migration.c
index 47d9189aaf..91775f8472 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2857,7 +2857,7 @@ static int postcopy_start(MigrationState *ms, Error
**errp)
* at the transition to postcopy and after the device state; in particular
* spice needs to trigger a transition now
*/
- migration_call_notifiers(ms, MIG_EVENT_PRECOPY_DONE, NULL);
+ migration_call_notifiers(ms, MIG_EVENT_POSTCOPY_START, NULL);
migration_downtime_end(ms);
@@ -2906,7 +2906,6 @@ fail:
migrate_set_state(&ms->state, ms->state, MIGRATION_STATUS_FAILED);
}
migration_block_activate(NULL);
- migration_call_notifiers(ms, MIG_EVENT_PRECOPY_FAILED, NULL);
bql_unlock();
return -1;
}
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 8a6050f4ae..ce3c2954e3 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -585,7 +585,8 @@ static int migration_state_notifier(NotifierWithReturn
*notifier,
if (e->type == MIG_EVENT_PRECOPY_SETUP) {
spice_server_migrate_start(spice_server);
- } else if (e->type == MIG_EVENT_PRECOPY_DONE) {
+ } else if (e->type == MIG_EVENT_PRECOPY_DONE ||
+ e->type == MIG_EVENT_POSTCOPY_START) {
spice_server_migrate_end(spice_server, true);
spice_have_target_host = false;
} else if (e->type == MIG_EVENT_PRECOPY_FAILED) {
--
2.50.1
