On Fri, Feb 14, 2025 at 09:25:12AM -0300, Fabiano Rosas wrote:
> Peter Xu <pet...@redhat.com> writes:
>
> > On Thu, Feb 13, 2025 at 02:59:24PM -0300, Fabiano Rosas wrote:
> >> The expected outcome from qmp_migrate_cancel() is that the source
> >> migration goes to the terminal state
> >> MIGRATION_STATUS_CANCELLED. Anything different from this is a bug when
> >> cancelling.
> >>
> >> Make sure there is never a state transition from an unspecified state
> >> into FAILED. Code that sets FAILED, should always either make sure
> >> that the old state is not CANCELLING or specify the old state.
> >>
> >> Note that the destination is allowed to go into FAILED, so there's no
> >> issue there.
> >>
> >> (I don't think this is relevant as a backport because cancelling does
> >> work, it just doesn't show the right state at the end)
> >>
> >> Fixes: 3dde8fdbad ("migration: Merge precopy/postcopy on switchover start")
> >> Fixes: d0edb8a173 ("migration: Create the postcopy preempt channel
> >> asynchronously")
> >> Fixes: 8518278a6a ("migration: implementation of background snapshot
> >> thread")
> >> Fixes: bf78a046b9 ("migration: refactor migrate_fd_connect failures")
> >> Signed-off-by: Fabiano Rosas <faro...@suse.de>
> >
> > Not like migrate_set_state_failure(MigrationState *s)? Not a huge deal,
> > though..
>
> I thought we had agreed over IRC that it was best to hold that until the
> other MigrationStatus work happens?
If we touched this anyway, IMHO no hurt to add a helper too.
migrate_set_state_failure() can then be renamed to migrate_set_failure(),
take a Error* instead so it might help that effort too.
>
> Anyway, looking closer at this, there are places that handle CANCELLING
> beforehand (_detect_error) and places that only set FAILED after
> specific states (multifd), so a single helper will require more
> churn. Let's postpone that please.
Sure. Let's go ahead with this.
>
> >
> > Reviewed-by: Peter Xu <pet...@redhat.com>
>
--
Peter Xu
