Fabiano Rosas <faro...@suse.de> writes:
> Markus Armbruster <arm...@redhat.com> writes:
>
>> Fabiano Rosas <faro...@suse.de> writes:
>>
>>> Now that the TLS options have been made the same between
>>> migrate-set-parameters and query-migrate-parameters, a single type can
>>> be used. Remove MigrateSetParameters.
>>>
>>> The TLS options documentation from MigrationParameters were replaced
>>> with the ones from MigrateSetParameters which was more complete.
>>>
>>> I'm choosing to somewhat ignore any ambiguity between "query" and
>>> "set" because other options' docs are already ambiguous in that
>>> regard.
>>>
>>> Signed-off-by: Fabiano Rosas <faro...@suse.de>
>>> ---
>>> migration/migration-hmp-cmds.c | 4 +-
>>> migration/options.c | 6 +-
>>> qapi/migration.json | 221 +++------------------------------
>>> 3 files changed, 20 insertions(+), 211 deletions(-)
>>>
>>> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
>>> index bc8179c582..aacffdc532 100644
>>> --- a/migration/migration-hmp-cmds.c
>>> +++ b/migration/migration-hmp-cmds.c
>>> @@ -490,7 +490,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const
>>> QDict *qdict)
>>> const char *param = qdict_get_str(qdict, "parameter");
>>> const char *valuestr = qdict_get_str(qdict, "value");
>>> Visitor *v = string_input_visitor_new(valuestr);
>>> - MigrateSetParameters *p = g_new0(MigrateSetParameters, 1);
>>> + MigrationParameters *p = g_new0(MigrationParameters, 1);
>>> uint64_t valuebw = 0;
>>> uint64_t cache_size;
>>> Error *err = NULL;
>>> @@ -656,7 +656,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const
>>> QDict *qdict)
>>> qmp_migrate_set_parameters(p, &err);
>>>
>>> cleanup:
>>> - qapi_free_MigrateSetParameters(p);
>>> + qapi_free_MigrationParameters(p);
>>> visit_free(v);
>>> hmp_handle_error(mon, err);
>>> }
>>> diff --git a/migration/options.c b/migration/options.c
>>> index 45a95dc6da..e49d584a99 100644
>>> --- a/migration/options.c
>>> +++ b/migration/options.c
>>> @@ -1227,7 +1227,7 @@ bool migrate_params_check(MigrationParameters
>>> *params, Error **errp)
>>> return true;
>>> }
>>>
>>> -static void migrate_params_test_apply(MigrateSetParameters *params,
>>> +static void migrate_params_test_apply(MigrationParameters *params,
>>> MigrationParameters *dest)
>>> {
>>> *dest = migrate_get_current()->parameters;
>>> @@ -1350,7 +1350,7 @@ static void
>>> migrate_params_test_apply(MigrateSetParameters *params,
>>> }
>>> }
>>>
>>> -static void migrate_params_apply(MigrateSetParameters *params, Error
>>> **errp)
>>> +static void migrate_params_apply(MigrationParameters *params, Error **errp)
>>> {
>>> MigrationState *s = migrate_get_current();
>>>
>>> @@ -1479,7 +1479,7 @@ static void migrate_params_apply(MigrateSetParameters
>>> *params, Error **errp)
>>> }
>>> }
>>>
>>> -void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
>>> +void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp)
>>> {
>>> MigrationParameters tmp;
>>>
>>> diff --git a/qapi/migration.json b/qapi/migration.json
>>> index fa42d94810..080968993a 100644
>>> --- a/qapi/migration.json
>>> +++ b/qapi/migration.json
>>> @@ -914,202 +914,6 @@
>>> 'zero-page-detection',
>>> 'direct-io'] }
>>>
>>> -##
>>> -# @MigrateSetParameters:
>>
>> Only use is argument type of migrate-set-parameters. You're replacing
>> it by MigrationParameters there. Let's compare the deleted docs to
>> their replacement. I'll quote replacement docs exactly where they
>> differ.
>>
>> # @MigrationParameters:
>> #
>> # Migration parameters. Optional members are optional when used with
>> # an input command, otherwise mandatory.
>>
>> Figuring out which commands are input commands is left to the reader.
>> Why not simply "optional with migrate-set-parameters"?
>>
>
> Future patches include migrate and migrate-incoming. I can enumerate
> them if that's better.
Not necessary if you move the note to commands as discussed below.
>> However, it doesn't end there. The paragraph creates a problem with
>> John Snow's "inliner", which I hope to merge later this year. Let me
>> explain.
>>
>> Generated command documentation normally looks like this:
>>
>> Command migrate-set-capabilities (Since: 1.2)
>>
>> Enable/Disable the following migration capabilities (like xbzrle)
>>
>> Arguments:
>> * **capabilities** ("[""MigrationCapabilityStatus""]") -- json
>> array of capability modifications to make
>>
>> Except when we happen to use a named type for the arguments. This
>> should be an implementation detail, and it is, except for generated
>> documentation, which looks like
>>
>> Command migrate-set-parameters (Since: 2.4)
>>
>> Set various migration parameters.
>>
>> Arguments:
>> * The members of "MigrationParameters".
>>
>> The arguments are hidden behind a link. The "inliner" will show the
>> them normally *always*, for better usability. It will not, however,
>> inline the introductory paragraph above. I can explain why if
>> necessary.
>>
>> To compensate for the loss of that paragraph, we'll have to add suitable
>> text to migrate-set-parameters's doc comment.
>>
>> I think we could just as well do that *now*: scratch the paragraph here,
>> add a suitable paragraph there.
>>
>
> Ok, no worries.
[...]
