On Fri, Jan 23, 2026 at 09:52:31AM -0300, Fabiano Rosas wrote: > Peter Xu <[email protected]> writes: > > > 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; > > With the new state there's a doc text to update at > migration_add_notifier below.
Yeh I noticed it, I didn't do it as I found POSTCOPY_START is special in this case marking an optional phase for postcopy-only. I can still update it, though.. Then it'll look like: - MIG_EVENT_SETUP [-> MIG_EVENT_POSTCOPY_START] -> MIG_EVENT_DONE - MIG_EVENT_SETUP [-> MIG_EVENT_POSTCOPY_START] -> MIG_EVENT_FAILED - MIG_EVENT_FAILED I'll also move it above the enum instead if no objections. -- Peter Xu
