| Message ID | 20230302163410.11399-4-quintela@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Migration: Create options.c for capabilities/params/properties | expand |
* Juan Quintela (quintela@redhat.com) wrote: > And remove the convoluted use of qmp_migrate_set_capabilities() to > enable disable MIGRATION_CAPABILITY_BLOCK. > > Signed-off-by: Juan Quintela <quintela@redhat.com> > --- > migration/migration.c | 34 ++++++++++++++++------------------ > 1 file changed, 16 insertions(+), 18 deletions(-) > > diff --git a/migration/migration.c b/migration/migration.c > index 119027a656..e3062530f0 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -1910,25 +1910,24 @@ void migrate_set_state(int *state, int old_state, int new_state) > } > } > > -static MigrationCapabilityStatus *migrate_cap_add(MigrationCapability index, > - bool state) > +static bool migrate_cap_set(int cap, bool value, Error **errp) Why int cap rather than MigrationCapability ? Dave > { > - MigrationCapabilityStatus *cap; > + MigrationState *s = migrate_get_current(); > + bool new_caps[MIGRATION_CAPABILITY__MAX]; > > - cap = g_new0(MigrationCapabilityStatus, 1); > - cap->capability = index; > - cap->state = state; > + if (migration_is_running(s->state)) { > + error_setg(errp, QERR_MIGRATION_ACTIVE); > + return false; > + } > > - return cap; > -} > + memcpy(new_caps, s->capabilities, sizeof(new_caps)); > + new_caps[cap] = value; > > -void migrate_set_block_enabled(bool value, Error **errp) > -{ > - MigrationCapabilityStatusList *cap = NULL; > - > - QAPI_LIST_PREPEND(cap, migrate_cap_add(MIGRATION_CAPABILITY_BLOCK, value)); > - qmp_migrate_set_capabilities(cap, errp); > - qapi_free_MigrationCapabilityStatusList(cap); > + if (!migrate_caps_check(s->capabilities, new_caps, errp)) { > + return false; > + } > + s->capabilities[cap] = value; > + return true; > } > > static void migrate_set_block_incremental(MigrationState *s, bool value) > @@ -1940,7 +1939,7 @@ static void block_cleanup_parameters(MigrationState *s) > { > if (s->must_remove_block_options) { > /* setting to false can never fail */ > - migrate_set_block_enabled(false, &error_abort); > + migrate_cap_set(MIGRATION_CAPABILITY_BLOCK, false, &error_abort); > migrate_set_block_incremental(s, false); > s->must_remove_block_options = false; > } > @@ -2427,8 +2426,7 @@ static bool migrate_prepare(MigrationState *s, bool blk, bool blk_inc, > "current migration capabilities"); > return false; > } > - migrate_set_block_enabled(true, &local_err); > - if (local_err) { > + if (!migrate_cap_set(MIGRATION_CAPABILITY_BLOCK, true, &local_err)) { > error_propagate(errp, local_err); > return false; > } > -- > 2.39.2 >
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: > * Juan Quintela (quintela@redhat.com) wrote: >> And remove the convoluted use of qmp_migrate_set_capabilities() to >> enable disable MIGRATION_CAPABILITY_BLOCK. >> >> Signed-off-by: Juan Quintela <quintela@redhat.com> >> --- >> migration/migration.c | 34 ++++++++++++++++------------------ >> 1 file changed, 16 insertions(+), 18 deletions(-) >> >> diff --git a/migration/migration.c b/migration/migration.c >> index 119027a656..e3062530f0 100644 >> --- a/migration/migration.c >> +++ b/migration/migration.c >> @@ -1910,25 +1910,24 @@ void migrate_set_state(int *state, int old_state, int new_state) >> } >> } >> >> -static MigrationCapabilityStatus *migrate_cap_add(MigrationCapability index, >> - bool state) >> +static bool migrate_cap_set(int cap, bool value, Error **errp) > > Why int cap rather than MigrationCapability ? It is the index in one array, so it is int. And it is much, much shorter. In this particular case I think that we don't lost anything having it as int, but will not fight over this O:-) Later, Juan.
diff --git a/migration/migration.c b/migration/migration.c index 119027a656..e3062530f0 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -1910,25 +1910,24 @@ void migrate_set_state(int *state, int old_state, int new_state) } } -static MigrationCapabilityStatus *migrate_cap_add(MigrationCapability index, - bool state) +static bool migrate_cap_set(int cap, bool value, Error **errp) { - MigrationCapabilityStatus *cap; + MigrationState *s = migrate_get_current(); + bool new_caps[MIGRATION_CAPABILITY__MAX]; - cap = g_new0(MigrationCapabilityStatus, 1); - cap->capability = index; - cap->state = state; + if (migration_is_running(s->state)) { + error_setg(errp, QERR_MIGRATION_ACTIVE); + return false; + } - return cap; -} + memcpy(new_caps, s->capabilities, sizeof(new_caps)); + new_caps[cap] = value; -void migrate_set_block_enabled(bool value, Error **errp) -{ - MigrationCapabilityStatusList *cap = NULL; - - QAPI_LIST_PREPEND(cap, migrate_cap_add(MIGRATION_CAPABILITY_BLOCK, value)); - qmp_migrate_set_capabilities(cap, errp); - qapi_free_MigrationCapabilityStatusList(cap); + if (!migrate_caps_check(s->capabilities, new_caps, errp)) { + return false; + } + s->capabilities[cap] = value; + return true; } static void migrate_set_block_incremental(MigrationState *s, bool value) @@ -1940,7 +1939,7 @@ static void block_cleanup_parameters(MigrationState *s) { if (s->must_remove_block_options) { /* setting to false can never fail */ - migrate_set_block_enabled(false, &error_abort); + migrate_cap_set(MIGRATION_CAPABILITY_BLOCK, false, &error_abort); migrate_set_block_incremental(s, false); s->must_remove_block_options = false; } @@ -2427,8 +2426,7 @@ static bool migrate_prepare(MigrationState *s, bool blk, bool blk_inc, "current migration capabilities"); return false; } - migrate_set_block_enabled(true, &local_err); - if (local_err) { + if (!migrate_cap_set(MIGRATION_CAPABILITY_BLOCK, true, &local_err)) { error_propagate(errp, local_err); return false; }
And remove the convoluted use of qmp_migrate_set_capabilities() to enable disable MIGRATION_CAPABILITY_BLOCK. Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/migration.c | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-)