Re: [Qemu-devel] [PATCH] migration: fix applying wrong capabilities

[Dr. David Alan Gilbert]

* Peter Xu (pet...@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 

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 / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH] migration: fix applying wrong capabilities

[Peter Xu]

On Tue, Mar 06, 2018 at 08:08:37PM +, Dr. David Alan Gilbert wrote:
> * Peter Xu (pet...@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 
> 
> OK, yes, that works, so:
> 
> 
> Reviewed-by: Dr. David Alan Gilbert 

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

Re: [Qemu-devel] [PATCH] migration: fix applying wrong capabilities

[Dr. David Alan Gilbert]

* Peter Xu (pet...@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 

OK, yes, that works, so:


Reviewed-by: Dr. David Alan Gilbert 


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 / dgilb...@redhat.com / Manchester, UK

[Qemu-devel] [PATCH] migration: fix applying wrong capabilities

[Peter Xu]

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 
---
 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