Re: [PATCH 3/4] migration: Use MigrationStatus instead of int

[Peter Xu <peterx@redhat.com>]

On Thu, Jun 13, 2024 at 11:59:20AM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > On Wed, Jun 12, 2024 at 10:42:27AM -0400, Peter Xu wrote:
> >> @@ -1544,7 +1545,7 @@ bool migration_in_postcopy(void)
> >>      }
> >>  }
> >>  
> >> -bool migration_postcopy_is_alive(int state)
> >> +bool migration_postcopy_is_alive(MigrationStatus state)
> >>  {
> >>      switch (state) {
> >>      case MIGRATION_STATUS_POSTCOPY_ACTIVE:
> >> @@ -1598,7 +1599,7 @@ bool migration_is_idle(void)
> >>      case MIGRATION_STATUS_DEVICE:
> >>      case MIGRATION_STATUS_WAIT_UNPLUG:
> >>          return false;
> >> -    case MIGRATION_STATUS__MAX:
> >> +    default:
> >>          g_assert_not_reached();
> >>      }
> >
> > This survives the tests, but I just found that it's risky, as it's not
> > covering all the states..
> >
> > I'll squash below when I send v2 instead.
> >
> > ===8<===
> > From 1fc42c76294044c0ccca8841e482472486de5459 Mon Sep 17 00:00:00 2001
> > From: Peter Xu <peterx@redhat.com>
> > Date: Wed, 12 Jun 2024 10:57:26 -0400
> > Subject: [PATCH] fixup! migration: Use MigrationStatus instead of int
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  migration/migration.c | 13 +------------
> >  1 file changed, 1 insertion(+), 12 deletions(-)
> >
> > diff --git a/migration/migration.c b/migration/migration.c
> > index 9475dce7dc..706cee1b69 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -1637,20 +1637,9 @@ bool migration_is_idle(void)
> >      case MIGRATION_STATUS_COMPLETED:
> >      case MIGRATION_STATUS_FAILED:
> >          return true;
> > -    case MIGRATION_STATUS_SETUP:
> > -    case MIGRATION_STATUS_CANCELLING:
> > -    case MIGRATION_STATUS_ACTIVE:
> > -    case MIGRATION_STATUS_POSTCOPY_ACTIVE:
> > -    case MIGRATION_STATUS_COLO:
> > -    case MIGRATION_STATUS_PRE_SWITCHOVER:
> > -    case MIGRATION_STATUS_DEVICE:
> > -    case MIGRATION_STATUS_WAIT_UNPLUG:
> > -        return false;
> >      default:
> > -        g_assert_not_reached();
> > +        return false;
> >      }
> 
> Unrelated, but should we consider POSTCOPY_PAUSED as idle?

I think not; idle should be about "whether a migration is in progress" in
verbose, IMHO.

E.g. see migrate_add_blocker_modes() -> is_busy(), otherwise we'll allow
changing migration blockers when postcopy is paused.

PS: offtopic, but.. "is_busy" may deserve some better name in the future..

-- 
Peter Xu