| Message ID | 20180305094938.31374-1-peterx@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | migration: fix applying wrong capabilities | expand |
* Peter Xu (peterx@redhat.com) wrote: > When setting migration capabilities via QMP/HMP, we'll apply them even > if the capability check failed. Fix it. > > Fixes: 4a84214ebe ("migration: provide migrate_caps_check()", 2017-07-18) > Signed-off-by: Peter Xu <peterx@redhat.com> OK, yes, that works, so: Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> It is a little odd in a way; 'caps_check' you might expect only checked and didn't change anything. migrate_params is organised a bit differently; and somewhat more confusingly. Dave > --- > migration/migration.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/migration/migration.c b/migration/migration.c > index 0aa596f867..88ed9375aa 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -747,13 +747,15 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params, > { > MigrationState *s = migrate_get_current(); > MigrationCapabilityStatusList *cap; > + bool cap_list[MIGRATION_CAPABILITY__MAX]; > > if (migration_is_setup_or_active(s->state)) { > error_setg(errp, QERR_MIGRATION_ACTIVE); > return; > } > > - if (!migrate_caps_check(s->enabled_capabilities, params, errp)) { > + memcpy(cap_list, s->enabled_capabilities, sizeof(cap_list)); > + if (!migrate_caps_check(cap_list, params, errp)) { > return; > } > > -- > 2.14.3 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Tue, Mar 06, 2018 at 08:08:37PM +0000, Dr. David Alan Gilbert wrote: > * Peter Xu (peterx@redhat.com) wrote: > > When setting migration capabilities via QMP/HMP, we'll apply them even > > if the capability check failed. Fix it. > > > > Fixes: 4a84214ebe ("migration: provide migrate_caps_check()", 2017-07-18) > > Signed-off-by: Peter Xu <peterx@redhat.com> > > OK, yes, that works, so: > > > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Thanks. > > > It is a little odd in a way; 'caps_check' you might expect only checked > and didn't change anything. migrate_params is organised a bit > differently; and somewhat more confusingly. Indeed. Maybe the cap_list copy should be within the function, and then define the function as: static bool migrate_caps_check(MigrationCapabilityStatusList *params, Error **errp); Then it at least looks more like the param_check one. Let me know if you think it's good; I can post another one after all, and this one would be easy. :)
* Peter Xu (peterx@redhat.com) wrote: > When setting migration capabilities via QMP/HMP, we'll apply them even > if the capability check failed. Fix it. > > Fixes: 4a84214ebe ("migration: provide migrate_caps_check()", 2017-07-18) > Signed-off-by: Peter Xu <peterx@redhat.com> Queued > --- > migration/migration.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/migration/migration.c b/migration/migration.c > index 0aa596f867..88ed9375aa 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -747,13 +747,15 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params, > { > MigrationState *s = migrate_get_current(); > MigrationCapabilityStatusList *cap; > + bool cap_list[MIGRATION_CAPABILITY__MAX]; > > if (migration_is_setup_or_active(s->state)) { > error_setg(errp, QERR_MIGRATION_ACTIVE); > return; > } > > - if (!migrate_caps_check(s->enabled_capabilities, params, errp)) { > + memcpy(cap_list, s->enabled_capabilities, sizeof(cap_list)); > + if (!migrate_caps_check(cap_list, params, errp)) { > return; > } > > -- > 2.14.3 > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff --git a/migration/migration.c b/migration/migration.c index 0aa596f867..88ed9375aa 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -747,13 +747,15 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params, { MigrationState *s = migrate_get_current(); MigrationCapabilityStatusList *cap; + bool cap_list[MIGRATION_CAPABILITY__MAX]; if (migration_is_setup_or_active(s->state)) { error_setg(errp, QERR_MIGRATION_ACTIVE); return; } - if (!migrate_caps_check(s->enabled_capabilities, params, errp)) { + memcpy(cap_list, s->enabled_capabilities, sizeof(cap_list)); + if (!migrate_caps_check(cap_list, params, errp)) { return; }
When setting migration capabilities via QMP/HMP, we'll apply them even if the capability check failed. Fix it. Fixes: 4a84214ebe ("migration: provide migrate_caps_check()", 2017-07-18) Signed-off-by: Peter Xu <peterx@redhat.com> --- migration/migration.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)