* [PATCH 0/3] qapi/migration: Doc fixes
@ 2024-03-22 13:51 Markus Armbruster
2024-03-22 13:51 ` [PATCH 1/3] qapi: Improve migration TLS documentation Markus Armbruster
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Markus Armbruster @ 2024-03-22 13:51 UTC (permalink / raw)
To: qemu-devel; +Cc: peterx, farosas, eblake, berrange
I'd like to get these into the release. Please review.
Markus Armbruster (3):
qapi: Improve migration TLS documentation
qapi: Resync MigrationParameter and MigrateSetParameters
qapi: Fix bogus documentation of query-migrationthreads
qapi/migration.json | 91 +++++++++++++++++++++++++--------------------
1 file changed, 50 insertions(+), 41 deletions(-)
--
2.44.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/3] qapi: Improve migration TLS documentation
2024-03-22 13:51 [PATCH 0/3] qapi/migration: Doc fixes Markus Armbruster
@ 2024-03-22 13:51 ` Markus Armbruster
2024-03-22 14:01 ` Fabiano Rosas
2024-03-22 13:51 ` [PATCH 2/3] qapi: Resync MigrationParameter and MigrateSetParameters Markus Armbruster
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Markus Armbruster @ 2024-03-22 13:51 UTC (permalink / raw)
To: qemu-devel; +Cc: peterx, farosas, eblake, berrange
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
+# hostname from the migration URI, if any. A non-empty value is
+# required when using x509 based TLS credentials and the migration
+# URI does not include a hostname, such as fd: or exec: based
+# migration. (Since 2.7)
+#
+# Note: empty value works only since 2.9.
#
# @tls-authz: ID of the 'authz' object subclass that provides access
# control checking of the TLS x509 certificate distinguished name.
@@ -1006,22 +1009,22 @@
# 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.9) Previously (since 2.7), this was
-# reported by omitting tls-creds instead.
+# rather than TLS. This is the default. (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) An empty string means that QEMU will use
-# the hostname associated with the migration URI, if any. (Since
-# 2.9) Previously (since 2.7), this was reported by omitting
-# tls-hostname instead.
+# @tls-hostname: migration target's hostname for validating the
+# server's x509 certificate identify. If empty, QEMU will use the
+# hostname from the migration URI, if any. A non-empty value is
+# required when using x509 based TLS credentials and the migration
+# URI does not include a hostname, such as fd: or exec: based
+# migration. (Since 2.7)
+#
+# Note: empty value works only since 2.9.
#
# @tls-authz: ID of the 'authz' object subclass that provides access
# control checking of the TLS x509 certificate distinguished name.
-# (Since 4.0)
+# This object is only resolved at time of use, so can be deleted
+# and recreated on the fly while the migration server is active.
+# If missing, it will default to denying access (Since 4.0)
#
# @max-bandwidth: to set maximum speed for migration. maximum speed
# in bytes per second. (Since 2.8)
@@ -1240,17 +1243,15 @@
# must be for a 'client' endpoint, while for the incoming side the
# credentials must be for a 'server' endpoint. An empty string
# means that QEMU will use plain text mode for migration, rather
-# than TLS (Since 2.7) Note: 2.8 reports this by omitting
-# tls-creds instead.
+# 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) An empty string means that QEMU will use
-# the hostname associated with the migration URI, if any. (Since
-# 2.9) Note: 2.8 reports this by omitting tls-hostname instead.
+# Note: 2.8 omits empty @tls-creds instead.
+#
+# @tls-hostname: migration target's hostname for validating the
+# server's x509 certificate identify. If empty, QEMU will use the
+# hostname from the migration URI, if any. (Since 2.7)
+#
+# Note: 2.8 omits empty @tls-hostname instead.
#
# @tls-authz: ID of the 'authz' object subclass that provides access
# control checking of the TLS x509 certificate distinguished name.
--
2.44.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] qapi: Resync MigrationParameter and MigrateSetParameters
2024-03-22 13:51 [PATCH 0/3] qapi/migration: Doc fixes Markus Armbruster
2024-03-22 13:51 ` [PATCH 1/3] qapi: Improve migration TLS documentation Markus Armbruster
@ 2024-03-22 13:51 ` Markus Armbruster
2024-03-22 14:10 ` Fabiano Rosas
2024-03-22 13:51 ` [PATCH 3/3] qapi: Fix bogus documentation of query-migrationthreads Markus Armbruster
2024-03-22 15:06 ` [PATCH 0/3] qapi/migration: Doc fixes Peter Xu
3 siblings, 1 reply; 10+ messages in thread
From: Markus Armbruster @ 2024-03-22 13:51 UTC (permalink / raw)
To: qemu-devel; +Cc: peterx, farosas, eblake, berrange
Enum MigrationParameter mirrors the members of struct
MigrateSetParameters. Differences to MigrateSetParameters's member
documentation are pointless. Clean them up:
* @compress-level, @compress-threads, @decompress-threads, and
x-checkpoint-delay are more thoroughly documented for
MigrationParameter, so use that version for both.
* @max-cpu-throttle is almost the same. Use MigrationParameter's
version for both.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qapi/migration.json | 24 +++++++++++++++++-------
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/qapi/migration.json b/qapi/migration.json
index cbcc6946eb..f6238b6980 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -966,16 +966,26 @@
# @announce-step: Increase in delay (in milliseconds) between
# subsequent packets in the announcement (Since 4.0)
#
-# @compress-level: compression level
+# @compress-level: Set the compression level to be used in live
+# migration, the compression level is an integer between 0 and 9,
+# where 0 means no compression, 1 means the best compression
+# speed, and 9 means best compression ratio which will consume
+# more CPU.
#
-# @compress-threads: compression thread count
+# @compress-threads: Set compression thread count to be used in live
+# migration, the compression thread count is an integer between 1
+# and 255.
#
# @compress-wait-thread: Controls behavior when all compression
# threads are currently busy. If true (default), wait for a free
# compression thread to become available; otherwise, send the page
# uncompressed. (Since 3.1)
#
-# @decompress-threads: decompression thread count
+# @decompress-threads: Set decompression thread count to be used in
+# live migration, the decompression thread count is an integer
+# between 1 and 255. Usually, decompression is at least 4 times as
+# fast as compression, so set the decompress-threads to the number
+# about 1/4 of compress-threads is adequate.
#
# @throttle-trigger-threshold: The ratio of bytes_dirty_period and
# bytes_xfer_period to trigger throttling. It is expressed as
@@ -1042,8 +1052,8 @@
# @downtime-limit: set maximum tolerated downtime for migration.
# maximum downtime in milliseconds (Since 2.8)
#
-# @x-checkpoint-delay: the delay time between two COLO checkpoints.
-# (Since 2.8)
+# @x-checkpoint-delay: The delay time (in ms) between two COLO
+# checkpoints in periodic mode. (Since 2.8)
#
# @block-incremental: Affects how much storage is migrated when the
# block migration capability is enabled. When false, the entire
@@ -1064,8 +1074,8 @@
# postcopy. Defaults to 0 (unlimited). In bytes per second.
# (Since 3.0)
#
-# @max-cpu-throttle: maximum cpu throttle percentage. The default
-# value is 99. (Since 3.1)
+# @max-cpu-throttle: maximum cpu throttle percentage. Defaults to 99.
+# (Since 3.1)
#
# @multifd-compression: Which compression method to use. Defaults to
# none. (Since 5.0)
--
2.44.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] qapi: Fix bogus documentation of query-migrationthreads
2024-03-22 13:51 [PATCH 0/3] qapi/migration: Doc fixes Markus Armbruster
2024-03-22 13:51 ` [PATCH 1/3] qapi: Improve migration TLS documentation Markus Armbruster
2024-03-22 13:51 ` [PATCH 2/3] qapi: Resync MigrationParameter and MigrateSetParameters Markus Armbruster
@ 2024-03-22 13:51 ` Markus Armbruster
2024-03-22 14:11 ` Fabiano Rosas
2024-03-22 15:25 ` John Snow
2024-03-22 15:06 ` [PATCH 0/3] qapi/migration: Doc fixes Peter Xu
3 siblings, 2 replies; 10+ messages in thread
From: Markus Armbruster @ 2024-03-22 13:51 UTC (permalink / raw)
To: qemu-devel; +Cc: peterx, farosas, eblake, berrange, John Snow
The doc comment documents an argument that doesn't exist. Would
fail compilation if it was marked up correctly. Delete.
The Returns: section fails to refer to the data type, leaving the user
to guess. Fix that.
The command name violates QAPI naming rules: it should be
query-migration-threads. Too late to fix.
Reported-by: John Snow <jsnow@redhat.com>
Fixes: 671326201dac (migration: Introduce interface query-migrationthreads)
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qapi/migration.json | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/qapi/migration.json b/qapi/migration.json
index f6238b6980..e47ad7a63b 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -2419,9 +2419,7 @@
#
# Returns information of migration threads
#
-# data: migration thread name
-#
-# Returns: information about migration threads
+# Returns: @MigrationThreadInfo
#
# Since: 7.2
##
--
2.44.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] qapi: Improve migration TLS documentation
2024-03-22 13:51 ` [PATCH 1/3] qapi: Improve migration TLS documentation Markus Armbruster
@ 2024-03-22 14:01 ` Fabiano Rosas
2024-03-22 14:12 ` Markus Armbruster
0 siblings, 1 reply; 10+ messages in thread
From: Fabiano Rosas @ 2024-03-22 14:01 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: peterx, eblake, berrange
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
> +# hostname from the migration URI, if any. A non-empty value is
> +# required when using x509 based TLS credentials and the migration
> +# URI does not include a hostname, such as fd: or exec: based
> +# migration. (Since 2.7)
> +#
> +# Note: empty value works only since 2.9.
> #
> # @tls-authz: ID of the 'authz' object subclass that provides access
> # control checking of the TLS x509 certificate distinguished name.
> @@ -1006,22 +1009,22 @@
> # 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.9) Previously (since 2.7), this was
> -# reported by omitting tls-creds instead.
> +# rather than TLS. This is the default. (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) An empty string means that QEMU will use
> -# the hostname associated with the migration URI, if any. (Since
> -# 2.9) Previously (since 2.7), this was reported by omitting
> -# tls-hostname instead.
> +# @tls-hostname: migration target's hostname for validating the
> +# server's x509 certificate identify. If empty, QEMU will use the
same here
> +# hostname from the migration URI, if any. A non-empty value is
> +# required when using x509 based TLS credentials and the migration
> +# URI does not include a hostname, such as fd: or exec: based
> +# migration. (Since 2.7)
> +#
> +# Note: empty value works only since 2.9.
> #
> # @tls-authz: ID of the 'authz' object subclass that provides access
> # control checking of the TLS x509 certificate distinguished name.
> -# (Since 4.0)
> +# This object is only resolved at time of use, so can be deleted
> +# and recreated on the fly while the migration server is active.
> +# If missing, it will default to denying access (Since 4.0)
> #
> # @max-bandwidth: to set maximum speed for migration. maximum speed
> # in bytes per second. (Since 2.8)
> @@ -1240,17 +1243,15 @@
> # must be for a 'client' endpoint, while for the incoming side the
> # credentials must be for a 'server' endpoint. An empty string
> # means that QEMU will use plain text mode for migration, rather
> -# than TLS (Since 2.7) Note: 2.8 reports this by omitting
> -# tls-creds instead.
> +# 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) An empty string means that QEMU will use
> -# the hostname associated with the migration URI, if any. (Since
> -# 2.9) Note: 2.8 reports this by omitting tls-hostname instead.
> +# Note: 2.8 omits empty @tls-creds instead.
> +#
> +# @tls-hostname: migration target's hostname for validating the
> +# server's x509 certificate identify. If empty, QEMU will use the
and here
> +# hostname from the migration URI, if any. (Since 2.7)
> +#
> +# Note: 2.8 omits empty @tls-hostname instead.
> #
> # @tls-authz: ID of the 'authz' object subclass that provides access
> # control checking of the TLS x509 certificate distinguished name.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] qapi: Resync MigrationParameter and MigrateSetParameters
2024-03-22 13:51 ` [PATCH 2/3] qapi: Resync MigrationParameter and MigrateSetParameters Markus Armbruster
@ 2024-03-22 14:10 ` Fabiano Rosas
0 siblings, 0 replies; 10+ messages in thread
From: Fabiano Rosas @ 2024-03-22 14:10 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: peterx, eblake, berrange
Markus Armbruster <armbru@redhat.com> writes:
> Enum MigrationParameter mirrors the members of struct
> MigrateSetParameters. Differences to MigrateSetParameters's member
> documentation are pointless. Clean them up:
>
> * @compress-level, @compress-threads, @decompress-threads, and
> x-checkpoint-delay are more thoroughly documented for
> MigrationParameter, so use that version for both.
>
> * @max-cpu-throttle is almost the same. Use MigrationParameter's
> version for both.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] qapi: Fix bogus documentation of query-migrationthreads
2024-03-22 13:51 ` [PATCH 3/3] qapi: Fix bogus documentation of query-migrationthreads Markus Armbruster
@ 2024-03-22 14:11 ` Fabiano Rosas
2024-03-22 15:25 ` John Snow
1 sibling, 0 replies; 10+ messages in thread
From: Fabiano Rosas @ 2024-03-22 14:11 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: peterx, eblake, berrange, John Snow
Markus Armbruster <armbru@redhat.com> writes:
> The doc comment documents an argument that doesn't exist. Would
> fail compilation if it was marked up correctly. Delete.
>
> The Returns: section fails to refer to the data type, leaving the user
> to guess. Fix that.
>
> The command name violates QAPI naming rules: it should be
> query-migration-threads. Too late to fix.
>
> Reported-by: John Snow <jsnow@redhat.com>
> Fixes: 671326201dac (migration: Introduce interface query-migrationthreads)
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] qapi: Improve migration TLS documentation
2024-03-22 14:01 ` Fabiano Rosas
@ 2024-03-22 14:12 ` Markus Armbruster
0 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2024-03-22 14:12 UTC (permalink / raw)
To: Fabiano Rosas; +Cc: Markus Armbruster, qemu-devel, peterx, eblake, berrange
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!
[...]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] qapi/migration: Doc fixes
2024-03-22 13:51 [PATCH 0/3] qapi/migration: Doc fixes Markus Armbruster
` (2 preceding siblings ...)
2024-03-22 13:51 ` [PATCH 3/3] qapi: Fix bogus documentation of query-migrationthreads Markus Armbruster
@ 2024-03-22 15:06 ` Peter Xu
3 siblings, 0 replies; 10+ messages in thread
From: Peter Xu @ 2024-03-22 15:06 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, farosas, eblake, berrange
On Fri, Mar 22, 2024 at 02:51:14PM +0100, Markus Armbruster wrote:
> I'd like to get these into the release. Please review.
>
> Markus Armbruster (3):
> qapi: Improve migration TLS documentation
> qapi: Resync MigrationParameter and MigrateSetParameters
> qapi: Fix bogus documentation of query-migrationthreads
With Fabiano's fixup squashed:
Reviewed-by: Peter Xu <peterx@redhat.com>
Thanks!
--
Peter Xu
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] qapi: Fix bogus documentation of query-migrationthreads
2024-03-22 13:51 ` [PATCH 3/3] qapi: Fix bogus documentation of query-migrationthreads Markus Armbruster
2024-03-22 14:11 ` Fabiano Rosas
@ 2024-03-22 15:25 ` John Snow
1 sibling, 0 replies; 10+ messages in thread
From: John Snow @ 2024-03-22 15:25 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, Peter Xu, Fabiano Rosas, Eric Blake, Daniel Berrange
[-- Attachment #1: Type: text/plain, Size: 1162 bytes --]
On Fri, Mar 22, 2024, 9:51 AM Markus Armbruster <armbru@redhat.com> wrote:
> The doc comment documents an argument that doesn't exist. Would
> fail compilation if it was marked up correctly. Delete.
>
> The Returns: section fails to refer to the data type, leaving the user
> to guess. Fix that.
>
> The command name violates QAPI naming rules: it should be
> query-migration-threads. Too late to fix.
>
> Reported-by: John Snow <jsnow@redhat.com>
> Fixes: 671326201dac (migration: Introduce interface query-migrationthreads)
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
Reviewed-by: John Snow <jsnow@redhat.com>
---
> qapi/migration.json | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/qapi/migration.json b/qapi/migration.json
> index f6238b6980..e47ad7a63b 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -2419,9 +2419,7 @@
> #
> # Returns information of migration threads
> #
> -# data: migration thread name
> -#
> -# Returns: information about migration threads
> +# Returns: @MigrationThreadInfo
> #
> # Since: 7.2
> ##
> --
> 2.44.0
>
>
[-- Attachment #2: Type: text/html, Size: 1957 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-03-22 15:25 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-22 13:51 [PATCH 0/3] qapi/migration: Doc fixes Markus Armbruster
2024-03-22 13:51 ` [PATCH 1/3] qapi: Improve migration TLS documentation Markus Armbruster
2024-03-22 14:01 ` Fabiano Rosas
2024-03-22 14:12 ` Markus Armbruster
2024-03-22 13:51 ` [PATCH 2/3] qapi: Resync MigrationParameter and MigrateSetParameters Markus Armbruster
2024-03-22 14:10 ` Fabiano Rosas
2024-03-22 13:51 ` [PATCH 3/3] qapi: Fix bogus documentation of query-migrationthreads Markus Armbruster
2024-03-22 14:11 ` Fabiano Rosas
2024-03-22 15:25 ` John Snow
2024-03-22 15:06 ` [PATCH 0/3] qapi/migration: Doc fixes Peter Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).