The MigrationParameters object is defined by the QAPI. It is also used
by the migration code as its internal store of parameters.
Due to a historic coupling of the migration code with qdev properties,
when a migration parameter has its initial value set by qdev, the QAPI
present flag for that parameter is kept unset. This is fine as long as
the MigrationParameters object is not used to interact with the QAPI
(e.g. as input to query-migrate) or the visitors infrastructure
(e.g. using QAPI_CLONE to duplicate an object).
Recent changes have removed code duplication and complexity in the
aforementioned interactions by adding a helper that attempts to ensure
consistency by setting all of the present flags for a (manually kept)
list of parameters. See commit f55f4ef632 ("migration: Extract code to
mark all parameters as present").
There are still some drawbacks to this scheme such as the need to keep
the list of parameters updated.
Replace the logic with a walk from a custom visitor that force sets
each present flag and dispenses with all manual updates as it uses the
QAPI generated code and will not become out of sync with
MigrationParameters.
This also allows the removal in the next patch of the
MigrationParameter enum which is the last place where migration
parameter names need to be replicated.
Suggested-by: Markus Armbruster <[email protected]>
Signed-off-by: Fabiano Rosas <[email protected]>
---
include/qapi/clone-visitor.h | 1 +
migration/migration.c | 2 +-
migration/options.c | 77 ++++++++++++------------------------
migration/options.h | 2 +-
qapi/qapi-clone-visitor.c | 13 ++++++
5 files changed, 41 insertions(+), 54 deletions(-)
diff --git a/include/qapi/clone-visitor.h b/include/qapi/clone-visitor.h
index ebc182b034..04cd1a88d1 100644
--- a/include/qapi/clone-visitor.h
+++ b/include/qapi/clone-visitor.h
@@ -22,6 +22,7 @@
typedef struct QapiCloneVisitor QapiCloneVisitor;
Visitor *qapi_clone_visitor_new(void);
+Visitor *qapi_clone_visitor_new_all(void);
Visitor *qapi_clone_members_visitor_new(void);
/*
diff --git a/migration/migration.c b/migration/migration.c
index fcfb82ad57..f92d13234f 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -4076,7 +4076,7 @@ static void migration_instance_init(Object *obj)
qemu_event_init(&ms->pause_event, false);
qemu_mutex_init(&ms->error_mutex);
- migrate_params_init(&ms->parameters);
+ migrate_params_init(ms);
qemu_sem_init(&ms->postcopy_pause_sem, 0);
qemu_sem_init(&ms->rp_state.rp_sem, 0);
diff --git a/migration/options.c b/migration/options.c
index 02b9c64ae6..8d869209ee 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -233,6 +233,31 @@ const Property migration_properties[] = {
};
const size_t migration_properties_count = ARRAY_SIZE(migration_properties);
+/*
+ * After qdev sets the defaults for the migration parameters using the
+ * migration_properties above, s->parameters is left inconsistent from
+ * QAPI point of view because the parameters' present flag is not
+ * set. Use the custom-built qapi_clone_visitor_all() to copy
+ * s->parameters back to itself while setting all present flags. The
+ * resulting object is fit to be used with QAPI and the visitor
+ * infrastructure.
+ *
+ * WARNING: missing entries in the migration_properties array in
+ * respect to MigrationParameters will still have their corresponding
+ * struct member marked as present.
+ */
+void migrate_params_init(MigrationState *s)
+{
+ Visitor *v = qapi_clone_visitor_new_all();
+ g_autoptr(MigrationParameters) p = &s->parameters;
+
+ visit_type_MigrationParameters(v, NULL, &p, &error_abort);
+ visit_free(v);
+ migrate_params_free(&s->parameters, &error_abort);
+ QAPI_CLONE_MEMBERS(MigrationParameters, &s->parameters, p);
+}
+
+
static void get_StrOrNull(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
@@ -1121,53 +1146,6 @@ static MigrationParameters
*migrate_params_from_dict(QDict *d, Error **errp)
return tmp;
}
-/*
- * query-migrate-parameters expects all members of MigrationParameters
- * to be present, but we cannot mark them non-optional in QAPI because
- * the structure is also used for migrate-set-parameters, which needs
- * the optionality. Force all parameters to be seen as present
- * now. Note that this depends on some form of default being set for
- * every member of MigrationParameters, currently done during qdev
- * init using migration_properties defined in this file. The TLS
- * options are a special case because they don't have a default and
- * need to be normalized before use.
- */
-static void migrate_mark_all_params_present(MigrationParameters *p)
-{
- int len, n_str_args = 3; /* tls-creds, tls-hostname, tls-authz */
- bool *has_fields[] = {
- &p->has_throttle_trigger_threshold, &p->has_cpu_throttle_initial,
- &p->has_cpu_throttle_increment, &p->has_cpu_throttle_tailslow,
- &p->has_max_bandwidth, &p->has_avail_switchover_bandwidth,
- &p->has_downtime_limit, &p->has_x_checkpoint_delay,
- &p->has_multifd_channels, &p->has_multifd_compression,
- &p->has_multifd_zlib_level, &p->has_multifd_qatzip_level,
- &p->has_multifd_zstd_level, &p->has_xbzrle_cache_size,
- &p->has_max_postcopy_bandwidth, &p->has_max_cpu_throttle,
- &p->has_announce_initial, &p->has_announce_max,
&p->has_announce_rounds,
- &p->has_announce_step, &p->has_block_bitmap_mapping,
- &p->has_x_vcpu_dirty_limit_period, &p->has_vcpu_dirty_limit,
- &p->has_mode, &p->has_zero_page_detection, &p->has_direct_io,
- &p->has_x_rdma_chunk_size, &p->has_cpr_exec_command, &p->has_local,
- &p->has_xbzrle, &p->has_rdma_pin_all,
- &p->has_auto_converge, &p->has_events,
- &p->has_postcopy_ram, &p->has_x_colo, &p->has_release_ram,
- &p->has_return_path, &p->has_pause_before_switchover, &p->has_multifd,
- &p->has_dirty_bitmaps, &p->has_postcopy_blocktime,
- &p->has_late_block_activate, &p->has_x_ignore_shared,
- &p->has_validate_uuid, &p->has_background_snapshot,
- &p->has_zero_copy_send, &p->has_postcopy_preempt,
- &p->has_switchover_ack, &p->has_dirty_limit, &p->has_mapped_ram,
- };
-
- len = ARRAY_SIZE(has_fields);
- assert(len + n_str_args == MIGRATION_PARAMETER__MAX);
-
- for (int i = 0; i < len; i++) {
- *has_fields[i] = true;
- }
-}
-
MigrationParameters *qmp_query_migrate_parameters(Error **errp)
{
MigrationState *s = migrate_get_current();
@@ -1188,11 +1166,6 @@ MigrationParameters *qmp_query_migrate_parameters(Error
**errp)
return params;
}
-void migrate_params_init(MigrationParameters *params)
-{
- migrate_mark_all_params_present(params);
-}
-
static void migrate_post_update_params(MigrationParameters *new, Error **errp)
{
MigrationState *s = migrate_get_current();
diff --git a/migration/options.h b/migration/options.h
index 28cf762ace..7abd527610 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -78,7 +78,7 @@ ZeroPageDetection migrate_zero_page_detection(void);
uint64_t migrate_rdma_chunk_size(void);
bool migrate_params_check(MigrationParameters *params, Error **errp);
-void migrate_params_init(MigrationParameters *params);
+void migrate_params_init(MigrationState *s);
bool migrate_params_free(MigrationParameters *params, Error **errp);
QDict *migrate_params_to_dict(MigrationParameters *p, Error **errp);
bool migrate_capability_get_compat(MigrationParameters *params, int i);
diff --git a/qapi/qapi-clone-visitor.c b/qapi/qapi-clone-visitor.c
index 30997638de..8e2135b78e 100644
--- a/qapi/qapi-clone-visitor.c
+++ b/qapi/qapi-clone-visitor.c
@@ -174,6 +174,19 @@ Visitor *qapi_clone_visitor_new(void)
return &v->visitor;
}
+static void qapi_clone_optional_all(Visitor *v, const char *name, bool
*present)
+{
+ *present = true;
+}
+
+Visitor *qapi_clone_visitor_new_all(void)
+{
+ Visitor *v = qapi_clone_visitor_new();
+
+ v->optional = qapi_clone_optional_all;
+ return v;
+}
+
Visitor *qapi_clone_members_visitor_new(void)
{
Visitor *v = qapi_clone_visitor_new();
--
2.53.0
