[RFC PATCH 12/13] [PoC] migration: Add query/set commands for MigrationConfig

Fabiano Rosas posted 13 patches 7 months, 1 week ago
There is a newer version of this series
[RFC PATCH 12/13] [PoC] migration: Add query/set commands for MigrationConfig
Posted by Fabiano Rosas 7 months, 1 week ago
Add the QMP commands query-migrate-config and migrate-set-config to
read and write the migration configuration options.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 migration/options.c | 24 ++++++++++++++++++++++++
 migration/options.h |  2 +-
 qapi/migration.json | 42 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/migration/options.c b/migration/options.c
index 4e3792dec3..c85ee2e506 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -1441,3 +1441,27 @@ void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
     migrate_config_apply(&tmp);
     migrate_post_update_config(&tmp, errp);
 }
+
+void qmp_migrate_set_config(MigrationConfig *config, Error **errp)
+{
+    if (!migrate_config_check(config, errp)) {
+        /* Invalid parameter */
+        return;
+    }
+
+    migrate_config_apply(config);
+    migrate_post_update_config(config, errp);
+}
+
+MigrationConfig *qmp_query_migrate_config(Error **errp)
+{
+    MigrationState *s = migrate_get_current();
+    MigrationConfig *config = g_new0(MigrationConfig, 1);
+
+    QAPI_CLONE_MEMBERS(MigrationConfig, config, &s->config);
+
+    /* set the has_* fields for every option */
+    migrate_config_init(config);
+
+    return config;
+}
diff --git a/migration/options.h b/migration/options.h
index 61ee854bb0..0e36dafe80 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -72,7 +72,7 @@ uint64_t migrate_xbzrle_cache_size(void);
 ZeroPageDetection migrate_zero_page_detection(void);
 
 bool migrate_config_check(MigrationConfig *params, Error **errp);
-void migrate_config_init(MigrationConfig *params);
+void migrate_config_init(MigrationConfig *config);
 bool migrate_config_get_cap_compat(MigrationConfig *config, int i);
 bool migrate_caps_check(MigrationConfig *new, Error **errp);
 #endif
diff --git a/qapi/migration.json b/qapi/migration.json
index 5e39f21adc..bb2487dbc6 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -2552,3 +2552,45 @@
   'data': { '*tls-creds': 'str',
             '*tls-hostname': 'str',
             '*tls-authz': 'str' } }
+
+##
+# @query-migrate-config:
+#
+# Returns information about the current migration configuration
+# options
+#
+# Returns: @MigrationConfig
+#
+# Since: 10.1
+#
+# .. qmp-example::
+#
+#     -> { "execute": "query-migrate-config" }
+#     <- { "return": {
+#              "multifd-channels": 2,
+#              "cpu-throttle-increment": 10,
+#              "cpu-throttle-initial": 20,
+#              "max-bandwidth": 33554432,
+#              "downtime-limit": 300
+#           }
+#        }
+##
+{ 'command': 'query-migrate-config',
+  'returns': 'MigrationConfig' }
+
+##
+# @migrate-set-config:
+#
+# Set various migration configuration options.
+#
+# Since: 10.1
+#
+# .. qmp-example::
+#
+#     -> { "execute": "migrate-set-config" ,
+#          "arguments": { "max-bandwidth": 33554432,
+#                         "downtime-limit": 300 } }
+#     <- { "return": {} }
+##
+{ 'command': 'migrate-set-config', 'boxed': true,
+  'data': 'MigrationConfig' }
-- 
2.35.3
Re: [RFC PATCH 12/13] [PoC] migration: Add query/set commands for MigrationConfig
Posted by Markus Armbruster 5 months, 3 weeks ago
Fabiano Rosas <farosas@suse.de> writes:

> Add the QMP commands query-migrate-config and migrate-set-config to
> read and write the migration configuration options.

These supersede query-migrate-capabilities, query-migrate-parameters,
migrate-set-capabilities, and migrate-set-parameters, right?

> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>  migration/options.c | 24 ++++++++++++++++++++++++
>  migration/options.h |  2 +-
>  qapi/migration.json | 42 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 67 insertions(+), 1 deletion(-)
>
> diff --git a/migration/options.c b/migration/options.c
> index 4e3792dec3..c85ee2e506 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -1441,3 +1441,27 @@ void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
>      migrate_config_apply(&tmp);
>      migrate_post_update_config(&tmp, errp);
>  }
> +
> +void qmp_migrate_set_config(MigrationConfig *config, Error **errp)
> +{
> +    if (!migrate_config_check(config, errp)) {
> +        /* Invalid parameter */
> +        return;
> +    }
> +
> +    migrate_config_apply(config);
> +    migrate_post_update_config(config, errp);
> +}
> +
> +MigrationConfig *qmp_query_migrate_config(Error **errp)
> +{
> +    MigrationState *s = migrate_get_current();
> +    MigrationConfig *config = g_new0(MigrationConfig, 1);
> +
> +    QAPI_CLONE_MEMBERS(MigrationConfig, config, &s->config);
> +
> +    /* set the has_* fields for every option */
> +    migrate_config_init(config);
> +
> +    return config;
> +}
> diff --git a/migration/options.h b/migration/options.h
> index 61ee854bb0..0e36dafe80 100644
> --- a/migration/options.h
> +++ b/migration/options.h
> @@ -72,7 +72,7 @@ uint64_t migrate_xbzrle_cache_size(void);
>  ZeroPageDetection migrate_zero_page_detection(void);
>  
>  bool migrate_config_check(MigrationConfig *params, Error **errp);
> -void migrate_config_init(MigrationConfig *params);
> +void migrate_config_init(MigrationConfig *config);

Have you considered renaming the declaration's parameter when you change
its type in PATCH 08, or when you rename the definition's parameter in
PATCH 11?

>  bool migrate_config_get_cap_compat(MigrationConfig *config, int i);
>  bool migrate_caps_check(MigrationConfig *new, Error **errp);
>  #endif
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 5e39f21adc..bb2487dbc6 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -2552,3 +2552,45 @@
>    'data': { '*tls-creds': 'str',
>              '*tls-hostname': 'str',
>              '*tls-authz': 'str' } }
> +
> +##
> +# @query-migrate-config:
> +#
> +# Returns information about the current migration configuration
> +# options
> +#
> +# Returns: @MigrationConfig
> +#
> +# Since: 10.1
> +#
> +# .. qmp-example::
> +#
> +#     -> { "execute": "query-migrate-config" }
> +#     <- { "return": {
> +#              "multifd-channels": 2,
> +#              "cpu-throttle-increment": 10,
> +#              "cpu-throttle-initial": 20,
> +#              "max-bandwidth": 33554432,
> +#              "downtime-limit": 300
> +#           }
> +#        }
> +##
> +{ 'command': 'query-migrate-config',
> +  'returns': 'MigrationConfig' }
> +
> +##
> +# @migrate-set-config:
> +#
> +# Set various migration configuration options.
> +#
> +# Since: 10.1
> +#
> +# .. qmp-example::
> +#
> +#     -> { "execute": "migrate-set-config" ,
> +#          "arguments": { "max-bandwidth": 33554432,
> +#                         "downtime-limit": 300 } }
> +#     <- { "return": {} }
> +##
> +{ 'command': 'migrate-set-config', 'boxed': true,
> +  'data': 'MigrationConfig' }

This patch exposes MigrationConfig externally.  We should double-check
its documentation to make sure it's what we want there.  Known issue:
how to reset @tls-creds & friends.  Touched in my review of PATCH 07.
Re: [RFC PATCH 12/13] [PoC] migration: Add query/set commands for MigrationConfig
Posted by Fabiano Rosas 5 months, 3 weeks ago
Markus Armbruster <armbru@redhat.com> writes:

> Fabiano Rosas <farosas@suse.de> writes:
>
>> Add the QMP commands query-migrate-config and migrate-set-config to
>> read and write the migration configuration options.
>
> These supersede query-migrate-capabilities, query-migrate-parameters,
> migrate-set-capabilities, and migrate-set-parameters, right?
>

They could. We haven't made a decision at this moment.

>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> ---
>>  migration/options.c | 24 ++++++++++++++++++++++++
>>  migration/options.h |  2 +-
>>  qapi/migration.json | 42 ++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 67 insertions(+), 1 deletion(-)
>>
>> diff --git a/migration/options.c b/migration/options.c
>> index 4e3792dec3..c85ee2e506 100644
>> --- a/migration/options.c
>> +++ b/migration/options.c
>> @@ -1441,3 +1441,27 @@ void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
>>      migrate_config_apply(&tmp);
>>      migrate_post_update_config(&tmp, errp);
>>  }
>> +
>> +void qmp_migrate_set_config(MigrationConfig *config, Error **errp)
>> +{
>> +    if (!migrate_config_check(config, errp)) {
>> +        /* Invalid parameter */
>> +        return;
>> +    }
>> +
>> +    migrate_config_apply(config);
>> +    migrate_post_update_config(config, errp);
>> +}
>> +
>> +MigrationConfig *qmp_query_migrate_config(Error **errp)
>> +{
>> +    MigrationState *s = migrate_get_current();
>> +    MigrationConfig *config = g_new0(MigrationConfig, 1);
>> +
>> +    QAPI_CLONE_MEMBERS(MigrationConfig, config, &s->config);
>> +
>> +    /* set the has_* fields for every option */
>> +    migrate_config_init(config);
>> +
>> +    return config;
>> +}
>> diff --git a/migration/options.h b/migration/options.h
>> index 61ee854bb0..0e36dafe80 100644
>> --- a/migration/options.h
>> +++ b/migration/options.h
>> @@ -72,7 +72,7 @@ uint64_t migrate_xbzrle_cache_size(void);
>>  ZeroPageDetection migrate_zero_page_detection(void);
>>  
>>  bool migrate_config_check(MigrationConfig *params, Error **errp);
>> -void migrate_config_init(MigrationConfig *params);
>> +void migrate_config_init(MigrationConfig *config);
>
> Have you considered renaming the declaration's parameter when you change
> its type in PATCH 08, or when you rename the definition's parameter in
> PATCH 11?
>

Yes, that looks like a mistake during rebase.

>>  bool migrate_config_get_cap_compat(MigrationConfig *config, int i);
>>  bool migrate_caps_check(MigrationConfig *new, Error **errp);
>>  #endif
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index 5e39f21adc..bb2487dbc6 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -2552,3 +2552,45 @@
>>    'data': { '*tls-creds': 'str',
>>              '*tls-hostname': 'str',
>>              '*tls-authz': 'str' } }
>> +
>> +##
>> +# @query-migrate-config:
>> +#
>> +# Returns information about the current migration configuration
>> +# options
>> +#
>> +# Returns: @MigrationConfig
>> +#
>> +# Since: 10.1
>> +#
>> +# .. qmp-example::
>> +#
>> +#     -> { "execute": "query-migrate-config" }
>> +#     <- { "return": {
>> +#              "multifd-channels": 2,
>> +#              "cpu-throttle-increment": 10,
>> +#              "cpu-throttle-initial": 20,
>> +#              "max-bandwidth": 33554432,
>> +#              "downtime-limit": 300
>> +#           }
>> +#        }
>> +##
>> +{ 'command': 'query-migrate-config',
>> +  'returns': 'MigrationConfig' }
>> +
>> +##
>> +# @migrate-set-config:
>> +#
>> +# Set various migration configuration options.
>> +#
>> +# Since: 10.1
>> +#
>> +# .. qmp-example::
>> +#
>> +#     -> { "execute": "migrate-set-config" ,
>> +#          "arguments": { "max-bandwidth": 33554432,
>> +#                         "downtime-limit": 300 } }
>> +#     <- { "return": {} }
>> +##
>> +{ 'command': 'migrate-set-config', 'boxed': true,
>> +  'data': 'MigrationConfig' }
>
> This patch exposes MigrationConfig externally.  We should double-check
> its documentation to make sure it's what we want there.  Known issue:
> how to reset @tls-creds & friends.  Touched in my review of PATCH 07.

I'll take a closer look at tls options after Daniel suggested changing
MigrationParameters' TLS options type from str to StrOrNull. That allows
us to drop MigrateSetParameters entirely.