Quotting from Markus in his replies:
migrate-set-parameters sets migration parameters, and
query-migrate-parameters gets them. Unsurprisingly, the former's
argument type MigrateSetParameters is quite close to the latter's
return type MigrationParameters. The differences are subtle:
1. Since migrate-set-parameters supports setting selected parameters,
its arguments must all be optional (so you can omit the ones you
don't want to change). query-migrate-parameters results are also
all optional, but almost all of them are in fact always present.
2. For parameters @tls_creds, @tls_hostname, @tls_authz,
migrate-set-parameters interprets special value "" as "reset to
default". Works, because "" is semantically invalid. Not a
general solution, because a semantically invalid value need not
exist. Markus added a general solution in commit 01fa559826
("migration: Use JSON null instead of "" to reset parameter to
default"). This involved changing the type from 'str' to
'StrOrNull'.
3. When parameter @block-bitmap-mapping has not been set,
query-migrate-parameters does not return it (absent optional
member). Clean (but undocumented). When parameters @tls_creds,
@tls_hostname, @tls_authz have not been set, it returns the
semantically invalid value "". Not so clean (and just as
undocumented).
Here to deduplicate the two objects: keep @MigrationParameters as the name
of object to use in both places, drop @MigrateSetParameters, at the
meantime switch types of @tls* fields from "str" to "StrOrNull" types.
I found that the TLS code wasn't so much relying on tls_* fields being
non-NULL at all. Actually on the other way round: if we set tls_authz to
an empty string (NOTE: currently, migrate_init() missed initializing
tls_authz; also touched it up in this patch), we can already fail one of
the migration-test (tls/x509/default-host), as qauthz_is_allowed_by_id()
will assume tls_authz set even if tls_auths is an empty string.
It means we're actually relying on tls_* fields being NULL even if it's the
empty string.
Let's just make it a rule to return NULL for empty string on these fields
internally. For that, when converting a StrOrNull into a char* (where we
introduced a helper here in this patch) we'll also make the empty string to
be NULL, to make it always work. And it doesn't show any issue either when
applying that logic to both tls_creds and tls_hostname.
With above, we can safely change both migration_tls_client_create() and
migrate_tls() to not check the empty string too finally.. not needed
anymore.
Also, we can drop the hackish conversions in qmp_migrate_set_parameters()
where we want to make sure it's a QSTRING; it's not needed now.
This greatly deduplicates the code not only in qapi/migration.json, but
also in the generic migration code.
Markus helped greatly with this patch. Besides a better commit
message (where I just "stole" from the reply), debugged and resolved a
double free, but also provided the StrOrNull property implementation to be
used in MigrationState object when switching tls_* fields to StrOrNull.
Co-developed-by: Markus Armbruster
Signed-off-by: Peter Xu
---
qapi/migration.json| 191 +---
include/hw/qdev-properties.h | 3 +
migration/options.h| 3 +
hw/core/qdev-properties.c | 40 ++
migration/migration-hmp-cmds.c | 20 +--
migration/options.c| 220 ++---
migration/tls.c| 3 +-
7 files changed, 125 insertions(+), 355 deletions(-)
diff --git a/qapi/migration.json b/qapi/migration.json
index 8843e74b59..45d69787ae 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -851,189 +851,6 @@
{ 'name': 'x-vcpu-dirty-limit-period', 'features': ['unstable'] },
'vcpu-dirty-limit'] }
-##
-# @MigrateSetParameters:
-#
-# @announce-initial: Initial delay (in milliseconds) before sending
-# the first announce (Since 4.0)
-#
-# @announce-max: Maximum delay (in milliseconds) between packets in
-# the announcement (Since 4.0)
-#
-# @announce-rounds: Number of self-announce packets sent after
-# migration (Since 4.0)
-#
-# @announce-step: Increase in delay (in milliseconds) between
-# subsequent packets in the announcement (Since 4.0)
-#
-# @compress-level: compression level
-#
-# @compress-threads: compression thread count
-#
-# @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
-#
-# @throttle-trigger-threshold: The ratio of bytes_dirty_period and
-# bytes_xfer_period to trigger throttling. It is expressed as
-# percentage. The default value is 50. (Since 5.0)
-#
-#
