[PATCH v2 05/18] migration: Merge parameter structs instead of assigning one by one

[Fabiano Rosas <farosas@suse.de>]

Convert the code in migrate_params_test_apply() from an open-coded
copy of every migration parameter to a merge operation using QAPI
visitors and QDict.

The purpose of that routine is to update a temporary structure
(pre-populated with the current migration parameters), with the values
received from the user via QAPI. As a result, the temporary structure
will then contain the "to be applied" parameters and it's validated
before being used to overwrite the parameters currently in use.

The update is currently done as follows:

where 'params' is the user input from QAPI,
for each parameter:

  a) check if the option is present
     params->has_<name> == true
     params-><name> != NULL // for strings

  b) if the parameter is a pointer, free the to-be-assigned member and
     allocate memory for the copy from params

  c) assign the user provided value to the temporary structure.

Step (a) is the same in principle as what the QAPI visitors do at
visit_type_MigrationParameters_members().

Steps (b) and (c) are roughly the same as what the QDict
implementation does when qdict_del() and qdict_put_obj() are combined.

Therefore, replace the open-coded function with
migrate_params_merge(), which achieves the same goal, but uses
visitors and QDict. This hides the details of QAPI (has_*) from the
migration code and avoids the need to update
migrate_params_test_apply() every time a new migration parameter is
added.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 migration/options.c | 200 ++++++++++++++------------------------------
 1 file changed, 65 insertions(+), 135 deletions(-)

diff --git a/migration/options.c b/migration/options.c
index 6b78795080..143710815a 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -20,6 +20,9 @@
 #include "qapi/qapi-commands-migration.h"
 #include "qapi/qapi-visit-migration.h"
 #include "qapi/qmp/qerror.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/qobject-output-visitor.h"
+#include "qobject/qdict.h"
 #include "qobject/qnull.h"
 #include "system/runstate.h"
 #include "migration/colo.h"
@@ -1074,6 +1077,28 @@ static void tls_opt_to_str(StrOrNull *opt)
     opt->u.s = g_strdup("");
 }
 
+static QDict *migrate_params_to_dict(MigrationParameters *p, Error **errp)
+{
+    QObject *obj = NULL;
+    Visitor *v = qobject_output_visitor_new(&obj);
+
+    if (visit_type_MigrationParameters(v, NULL, &p, errp)) {
+        visit_complete(v, &obj);
+    }
+    visit_free(v);
+    return qobject_to(QDict, obj);
+}
+
+static MigrationParameters *migrate_params_from_dict(QDict *d, Error **errp)
+{
+    Visitor *v = qobject_input_visitor_new(QOBJECT(d));
+    MigrationParameters *tmp = NULL;
+
+    visit_type_MigrationParameters(v, NULL, &tmp, errp);
+    visit_free(v);
+    return tmp;
+}
+
 /*
  * query-migrate-parameters expects all members of MigrationParameters
  * to be present, but we cannot mark them non-optional in QAPI because
@@ -1166,6 +1191,38 @@ static void migrate_post_update_params(MigrationParameters *new, Error **errp)
     }
 }
 
+static bool migrate_params_merge(MigrationParameters *base,
+                                 MigrationParameters *updates,
+                                 MigrationParameters **new,
+                                 Error **errp)
+{
+    g_autoptr(QDict) d_base = NULL;
+    g_autoptr(QDict) d_upd = NULL;
+    const QDictEntry *e;
+
+    d_base = migrate_params_to_dict(base, errp);
+    if (!d_base) {
+        return false;
+    }
+
+    d_upd = migrate_params_to_dict(updates, errp);
+    if (!d_upd) {
+        return false;
+    }
+
+    for (e = qdict_first(d_upd); e; e = qdict_next(d_upd, e)) {
+        const char *key = qdict_entry_key(e);
+        QObject *value = qdict_entry_value(e);
+
+        qobject_ref(value);
+        qdict_put_obj(d_base, key, value);
+    }
+
+    *new = migrate_params_from_dict(d_base, errp);
+
+    return !!*new;
+}
+
 /*
  * Check whether the parameters are valid. Error will be put into errp
  * (if provided). Return true if valid, otherwise false.
@@ -1328,133 +1385,6 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
     return true;
 }
 
-static void migrate_params_test_apply(MigrationParameters *params,
-                                      MigrationParameters *dest)
-{
-    MigrationState *s = migrate_get_current();
-
-    QAPI_CLONE_MEMBERS(MigrationParameters, dest, &s->parameters);
-
-    if (params->has_throttle_trigger_threshold) {
-        dest->throttle_trigger_threshold = params->throttle_trigger_threshold;
-    }
-
-    if (params->has_cpu_throttle_initial) {
-        dest->cpu_throttle_initial = params->cpu_throttle_initial;
-    }
-
-    if (params->has_cpu_throttle_increment) {
-        dest->cpu_throttle_increment = params->cpu_throttle_increment;
-    }
-
-    if (params->has_cpu_throttle_tailslow) {
-        dest->cpu_throttle_tailslow = params->cpu_throttle_tailslow;
-    }
-
-    if (params->tls_creds) {
-        qapi_free_StrOrNull(dest->tls_creds);
-        dest->tls_creds = QAPI_CLONE(StrOrNull, params->tls_creds);
-    }
-
-    if (params->tls_hostname) {
-        qapi_free_StrOrNull(dest->tls_hostname);
-        dest->tls_hostname = QAPI_CLONE(StrOrNull, params->tls_hostname);
-    }
-
-    if (params->tls_authz) {
-        qapi_free_StrOrNull(dest->tls_authz);
-        dest->tls_authz = QAPI_CLONE(StrOrNull, params->tls_authz);
-    }
-
-    if (params->has_max_bandwidth) {
-        dest->max_bandwidth = params->max_bandwidth;
-    }
-
-    if (params->has_avail_switchover_bandwidth) {
-        dest->avail_switchover_bandwidth = params->avail_switchover_bandwidth;
-    }
-
-    if (params->has_downtime_limit) {
-        dest->downtime_limit = params->downtime_limit;
-    }
-
-    if (params->has_x_checkpoint_delay) {
-        dest->x_checkpoint_delay = params->x_checkpoint_delay;
-    }
-
-    if (params->has_multifd_channels) {
-        dest->multifd_channels = params->multifd_channels;
-    }
-    if (params->has_multifd_compression) {
-        dest->multifd_compression = params->multifd_compression;
-    }
-    if (params->has_multifd_qatzip_level) {
-        dest->multifd_qatzip_level = params->multifd_qatzip_level;
-    }
-    if (params->has_multifd_zlib_level) {
-        dest->multifd_zlib_level = params->multifd_zlib_level;
-    }
-    if (params->has_multifd_zstd_level) {
-        dest->multifd_zstd_level = params->multifd_zstd_level;
-    }
-    if (params->has_xbzrle_cache_size) {
-        dest->xbzrle_cache_size = params->xbzrle_cache_size;
-    }
-    if (params->has_max_postcopy_bandwidth) {
-        dest->max_postcopy_bandwidth = params->max_postcopy_bandwidth;
-    }
-    if (params->has_max_cpu_throttle) {
-        dest->max_cpu_throttle = params->max_cpu_throttle;
-    }
-    if (params->has_announce_initial) {
-        dest->announce_initial = params->announce_initial;
-    }
-    if (params->has_announce_max) {
-        dest->announce_max = params->announce_max;
-    }
-    if (params->has_announce_rounds) {
-        dest->announce_rounds = params->announce_rounds;
-    }
-    if (params->has_announce_step) {
-        dest->announce_step = params->announce_step;
-    }
-
-    if (params->has_block_bitmap_mapping) {
-        qapi_free_BitmapMigrationNodeAliasList(dest->block_bitmap_mapping);
-        dest->block_bitmap_mapping = QAPI_CLONE(BitmapMigrationNodeAliasList,
-                                                params->block_bitmap_mapping);
-    }
-
-    if (params->has_x_vcpu_dirty_limit_period) {
-        dest->x_vcpu_dirty_limit_period =
-            params->x_vcpu_dirty_limit_period;
-    }
-    if (params->has_vcpu_dirty_limit) {
-        dest->vcpu_dirty_limit = params->vcpu_dirty_limit;
-    }
-
-    if (params->has_mode) {
-        dest->mode = params->mode;
-    }
-
-    if (params->has_zero_page_detection) {
-        dest->zero_page_detection = params->zero_page_detection;
-    }
-
-    if (params->has_direct_io) {
-        dest->direct_io = params->direct_io;
-    }
-
-    if (params->has_x_rdma_chunk_size) {
-        dest->x_rdma_chunk_size = params->x_rdma_chunk_size;
-    }
-
-    if (params->has_cpr_exec_command) {
-        qapi_free_strList(dest->cpr_exec_command);
-        dest->cpr_exec_command = QAPI_CLONE(strList, params->cpr_exec_command);
-    }
-}
-
 /*
  * Caller must ensure the has_* fields of @params are true so they all
  * get copied and the pointer members don't dangle.
@@ -1472,7 +1402,8 @@ static void migrate_params_apply(MigrationParameters *params)
 
 void qmp_migrate_set_parameters(MigrationParameters *input, Error **errp)
 {
-    MigrationParameters new;
+    MigrationParameters *cur = &migrate_get_current()->parameters;
+    g_autoptr(MigrationParameters) new = NULL;
 
     /*
      * Convert QTYPE_QNULL and NULL to the empty string (""). Even
@@ -1486,14 +1417,13 @@ void qmp_migrate_set_parameters(MigrationParameters *input, Error **errp)
     tls_opt_to_str(input->tls_hostname);
     tls_opt_to_str(input->tls_authz);
 
-    migrate_params_test_apply(input, &new);
+    /* merge input on top of current */
+    if (!migrate_params_merge(cur, input, &new, errp)) {
+        return;
+    }
 
-    if (migrate_params_check(&new, errp)) {
-        migrate_params_apply(&new);
+    if (migrate_params_check(new, errp)) {
+        migrate_params_apply(new);
         migrate_post_update_params(input, errp);
     }
-
-    migrate_tls_opts_free(&new);
-    qapi_free_BitmapMigrationNodeAliasList(new.block_bitmap_mapping);
-    qapi_free_strList(new.cpr_exec_command);
 }
-- 
2.53.0