QEMU uses "int" in most cases even if it stores MigrationStatus. I don't
know why, so let's try to do that right and see what blows up..
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/migration.h | 9 +++++----
migration/migration.c | 13 +++++++------
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/migration/migration.h b/migration/migration.h
index 6af01362d4..38aa1402d5 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -160,7 +160,7 @@ struct MigrationIncomingState {
/* PostCopyFD's for external userfaultfds & handlers of shared memory */
GArray *postcopy_remote_fds;
- int state;
+ MigrationStatus state;
/*
* The incoming migration coroutine, non-NULL during qemu_loadvm_state().
@@ -301,7 +301,7 @@ struct MigrationState {
/* params from 'migrate-set-parameters' */
MigrationParameters parameters;
- int state;
+ MigrationStatus state;
/* State related to return path */
struct {
@@ -459,7 +459,8 @@ struct MigrationState {
bool rdma_migration;
};
-void migrate_set_state(int *state, int old_state, int new_state);
+void migrate_set_state(MigrationStatus *state, MigrationStatus old_state,
+ MigrationStatus new_state);
void migration_fd_process_incoming(QEMUFile *f);
void migration_ioc_process_incoming(QIOChannel *ioc, Error **errp);
@@ -479,7 +480,7 @@ int migrate_init(MigrationState *s, Error **errp);
bool migration_is_blocked(Error **errp);
/* True if outgoing migration has entered postcopy phase */
bool migration_in_postcopy(void);
-bool migration_postcopy_is_alive(int state);
+bool migration_postcopy_is_alive(MigrationStatus state);
MigrationState *migrate_get_current(void);
bool migration_has_failed(MigrationState *);
bool migrate_mode_is_cpr(MigrationState *);
diff --git a/migration/migration.c b/migration/migration.c
index d41e00ed4c..bfbd657035 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -390,7 +390,7 @@ void migration_incoming_state_destroy(void)
yank_unregister_instance(MIGRATION_YANK_INSTANCE);
}
-static void migrate_generate_event(int new_state)
+static void migrate_generate_event(MigrationStatus new_state)
{
if (migrate_events()) {
qapi_event_send_migration(new_state);
@@ -1273,8 +1273,6 @@ static void fill_destination_migration_info(MigrationInfo *info)
}
switch (mis->state) {
- case MIGRATION_STATUS_NONE:
- return;
case MIGRATION_STATUS_SETUP:
case MIGRATION_STATUS_CANCELLING:
case MIGRATION_STATUS_CANCELLED:
@@ -1290,6 +1288,8 @@ static void fill_destination_migration_info(MigrationInfo *info)
info->has_status = true;
fill_destination_postcopy_migration_info(info);
break;
+ default:
+ return;
}
info->status = mis->state;
@@ -1337,7 +1337,8 @@ void qmp_migrate_start_postcopy(Error **errp)
/* shared migration helpers */
-void migrate_set_state(int *state, int old_state, int new_state)
+void migrate_set_state(MigrationStatus *state, MigrationStatus old_state,
+ MigrationStatus new_state)
{
assert(new_state < MIGRATION_STATUS__MAX);
if (qatomic_cmpxchg(state, old_state, new_state) == old_state) {
@@ -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();
}
--
2.45.0On 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;
}
-
- return false;
}
bool migration_is_active(void)
--
2.45.0
===8<===
--
Peter Xu
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?
> -
> - return false;
> }
>
> bool migration_is_active(void)
> --
> 2.45.0
>
> ===8<===
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
© 2016 - 2025 Red Hat, Inc.