Re: [PATCH v3 6/9] migration: Don't set FAILED state when cancelling

[Peter Xu <peterx@redhat.com>]

On Fri, Feb 14, 2025 at 09:25:12AM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@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 <farosas@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 <peterx@redhat.com>
> 

-- 
Peter Xu