Skip to contentSkip to conversation

migration: fix applying wrong capabilities

PATCH In review Peter Xu 2 participants 4 messages Mar 5, 2018 – Mar 9, 2018

lore
Share & tools
lore

Peter Xu

author
#0 lore raw

PATCH 1/1 migration: fix applying wrong capabilities

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 <[email protected]>
---
 migration/migration.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
migration/migration.c +3 -1
index 0aa596f867..88ed9375aa 100644
@@ -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;
     }
Signature · 1 line
2.14.3
Reviewed-by on v1 1/1
#1 lore raw
Quoted 5 lines · * Peter Xu ([email protected]) 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 <[email protected]>

OK, yes, that works, so:

Reviewed-by Dr. David Alan Gilbert <[email protected]>

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

Quoted 28 lines
---
 migration/migration.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
migration/migration.c +3 -1
index 0aa596f867..88ed9375aa 100644
@@ -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;
     }
Signature · 1 line
2.14.3

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

author
#2 lore raw
Quoted 11 lines · Dr. David Alan Gilbert
Quoted 5 lines · * Peter Xu ([email protected]) 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 <[email protected]>

OK, yes, that works, so:

Reviewed-by Dr. David Alan Gilbert <[email protected]>

Thanks.

Quoted 5 lines

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. :)

Signature · 1 line
Peter Xu
#3 lore raw
In reply to Peter Xu
Quoted 5 lines · * Peter Xu ([email protected]) 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 <[email protected]>

Queued

Quoted 29 lines
---
 migration/migration.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
migration/migration.c +3 -1
index 0aa596f867..88ed9375aa 100644
@@ -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;
     }
Signature · 1 line
2.14.3

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK