On Thu, Jun 13, 2024 at 11:59:20AM -0300, Fabiano Rosas wrote:
> Peter Xu <pet...@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 <pet...@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 <pet...@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
