Re: [PATCH 1/3] qapi: Improve migration TLS documentation

[Markus Armbruster]

Fabiano Rosas <farosas@suse.de> writes:

> Markus Armbruster <armbru@redhat.com> writes:
>
>> MigrateSetParameters is about setting parameters, and
>> MigrationParameters is about querying them.  Their documentation of
>> @tls-creds and @tls-hostname has residual damage from a failed attempt
>> at de-duplicating them (see commit de63ab61241 "migrate: Share common
>> MigrationParameters struct" and commit 1bda8b3c695 "migration: Unshare
>> MigrationParameters struct for now").
>>
>> MigrateSetParameters documentation issues:
>>
>> * It claims plain text mode "was reported by omitting tls-creds"
>>   before 2.9.  MigrateSetParameters is not used for reporting, so this
>>   is misleading.  Delete.
>>
>> * It similarly claims hostname defaulting to migration URI "was
>>   reported by omitting tls-hostname" before 2.9.  Delete as well.
>>
>> Rephrase the remaining @tls-hostname contents for clarity.
>>
>> Enum MigrationParameter mirrors the members of struct
>> MigrateSetParameters.  Differences to MigrateSetParameters's member
>> documentation are pointless.  Copy the new text to MigrationParameter.
>>
>> MigrationParameters documentation issues:
>>
>> * @tls-creds runs the two last sentences together without punctuation.
>>   Fix that.
>>
>> * Much of the contents on @tls-hostname only applies to setting
>>   parameters, resulting in confusion.  Replace by a suitable abridged
>>   version of the new MigrateSetParameters text, and a note on
>>   @tls-hostname omission in 2.8.
>>
>> Additional damage is due to flawed doc fix commit
>> 66fcb9d651d (qapi/migration: Add missing tls-authz documentation):
>> since it copied the missing MigrateSetParameters text from
>> MigrationParameters instead of MigrationParameter, the part on
>> recreating @tls-authz on the fly is missing.  Copy that, too.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  qapi/migration.json | 63 +++++++++++++++++++++++----------------------
>>  1 file changed, 32 insertions(+), 31 deletions(-)
>>
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index aa1b39bce1..cbcc6946eb 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -809,16 +809,19 @@
>>  #     for establishing a TLS connection over the migration data
>>  #     channel.  On the outgoing side of the migration, the credentials
>>  #     must be for a 'client' endpoint, while for the incoming side the
>> -#     credentials must be for a 'server' endpoint.  Setting this will
>> -#     enable TLS for all migrations.  The default is unset, resulting
>> -#     in unsecured migration at the QEMU level.  (Since 2.7)
>> +#     credentials must be for a 'server' endpoint.  Setting this to a
>> +#     non-empty string enables TLS for all migrations.  An empty
>> +#     string means that QEMU will use plain text mode for migration,
>> +#     rather than TLS.  (Since 2.7)
>>  #
>> -# @tls-hostname: hostname of the target host for the migration.  This
>> -#     is required when using x509 based TLS credentials and the
>> -#     migration URI does not already include a hostname.  For example
>> -#     if using fd: or exec: based migration, the hostname must be
>> -#     provided so that the server's x509 certificate identity can be
>> -#     validated.  (Since 2.7)
>> +# @tls-hostname: migration target's hostname for validating the
>> +#     server's x509 certificate identify.  If empty, QEMU will use the
>
> identity

ACK!  Also the other two copies you noted.  Thanks!

[...]