On Fri, Oct 02, 2026 at 01:02:56PM +0200, Denis V. Lunev wrote: > From: Denis V. Lunev <[email protected]> > > Commit 7b842fe354c ("migration: Introduce POSTCOPY_DEVICE state") made > the destination enter POSTCOPY_ACTIVE in loadvm_postcopy_handle_run(), > before it schedules loadvm_postcopy_handle_run_bh(), and only the BH > starts the VM or moves it to RUN_STATE_PAUSED. Management reacting to > the destination's postcopy-active event can therefore still find the > VM in RUN_STATE_INMIGRATE, where blockdev-add creates inactive nodes > and attaching a device to them fails with "Block node is read-only".
Yes, this is a fact that we have two "status" during postcopy phase, "query migrate" and "query status". I don't think we have clear definition on how they can correlate, and if there's strict order of state changes.. Pure question is, why not the mgmt listen to the vmstate change event directly (or both)? IOW, I believe mgmt still can have full knowledge of the status quo depending on how mgmt is implemented. > > Move the transition to the end of the run BH, after vm_start() or > runstate_set(), as process_incoming_migration_bh() does for precopy > COMPLETED. Without a return path the listen thread enters True, it is still reasonable to at least keep them aligned, that if we update vmstate first then always do that. > POSTCOPY_ACTIVE directly, and the fast snapshot load path switches > right after scheduling the BH; in both cases the state is no longer > POSTCOPY_DEVICE when the BH runs and the transition is a no-op. > > Signed-off-by: Denis V. Lunev <[email protected]> > Cc: Peter Xu <[email protected]> > Cc: Fabiano Rosas <[email protected]> > Cc: Juraj Marcin <[email protected]> > --- > migration/savevm.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/migration/savevm.c b/migration/savevm.c > index 5ec09d4889..bc1bc98a0e 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -2263,6 +2263,10 @@ static void loadvm_postcopy_handle_run_bh(void *opaque) > } > > trace_vmstate_downtime_checkpoint("dst-postcopy-bh-vm-started"); > + > + /* Already POSTCOPY_ACTIVE without a return path or in fast snapshot > load */ > + migrate_set_state(&mis->state, MIGRATION_STATUS_POSTCOPY_DEVICE, > + MIGRATION_STATUS_POSTCOPY_ACTIVE); I think the "if" is good here, rather than relying on migrate_set_state() behavior.. I actually want to change it, low priority but still: the current "if old state is not specified silently ignore" behavior is error prone. > qemu_event_set(&mis->postcopy_run_event); > } > > @@ -2277,11 +2281,6 @@ static int > loadvm_postcopy_handle_run(MigrationIncomingState *mis, Error **errp) > return -1; > } > > - /* We might be already in POSTCOPY_ACTIVE if there is no return path */ > - if (mis->state == MIGRATION_STATUS_POSTCOPY_DEVICE) { > - migrate_set_state(&mis->state, MIGRATION_STATUS_POSTCOPY_DEVICE, > - MIGRATION_STATUS_POSTCOPY_ACTIVE); > - } > postcopy_state_set(POSTCOPY_INCOMING_RUNNING); > migration_bh_schedule(loadvm_postcopy_handle_run_bh, mis); > > -- > 2.53.0 > -- Peter Xu
