Re: [PATCH v4 10/25] migration: Add Error** argument to qemu_savevm_state_setup()

[Peter Xu]

On Fri, Mar 15, 2024 at 11:17:45AM +0100, Cédric Le Goater wrote:
> > migrate_set_state is also unintuitive because it ignores invalid state
> > transitions and we've been using that property to deal with special
> > states such as POSTCOPY_PAUSED and FAILED:
> > 
> > - After the migration goes into POSTCOPY_PAUSED, the resumed migration's
> >    migrate_init() will try to set the state NONE->SETUP, which is not
> >    valid.
> > 
> > - After save_setup fails, the migration goes into FAILED, but wait_unplug
> >    will try to transition SETUP->ACTIVE, which is also not valid.
> > 
> 
> I am not sure I understand what the plan is. Both solutions are problematic
> regarding the state transitions.
> 
> Should we consider that waiting for failover devices to unplug is an internal
> step of the SETUP phase not transitioning to ACTIVE ?

If to unblock this series, IIUC the simplest solution is to do what Fabiano
suggested, that we move qemu_savevm_wait_unplug() to be before the check of
setup() ret.  In that case, the state change in qemu_savevm_wait_unplug()
should be benign and we should see a super small window it became ACTIVE
but then it should be FAILED (and IIUC the patch itself will need to use
ACTIVE as "old_state", not SETUP anymore).

For the long term, maybe we can remove the WAIT_UNPLUG state?  The only
Libvirt support seems to be here:

commit 8a226ddb3602586a2ba2359afc4448c02f566a0e
Author: Laine Stump <laine@redhat.com>
Date:   Wed Jan 15 16:38:57 2020 -0500

    qemu: add wait-unplug to qemu migration status enum

Considering that qemu_savevm_wait_unplug() can be a noop if the device is
already unplugged, I think it means no upper layer app should rely on this
state to present.

Thanks,

-- 
Peter Xu