[PATCH 1/6] migration: Fix and clean up around @tls-authz

[Markus Armbruster]

Commit d2f1d29b95 "migration: add support for a "tls-authz" migration
parameter" added MigrationParameters member @tls-authz.  Whereas the
other members aren't really optional (see commit 1bda8b3c695), this
one is genuinely optional: migration_instance_init() leaves it absent,
and migration_tls_channel_process_incoming() passes it to
qcrypto_tls_session_new(), which checks for null.

Commit d2f1d29b95 has a number of issues, though:

* When qmp_query_migrate_parameters() copies migration parameters into
  its reply, it ignores has_tls_authz, and assumes true instead.  When
  it is false,

  - HMP info migrate_parameters prints the null pointer (crash bug on
    some systems), and

  - QMP query-migrate-parameters replies "tls-authz": "" (because the
    QObject output visitor silently maps null pointer to "", which it
    really shouldn't).

  The HMP defect was noticed and fixed in commit 7cd75cbdb8
  'migration: use "" instead of (null) for tls-authz'.  Unfortunately,
  the fix papered over the real bug: it made
  qmp_query_migrate_parameters() map null tls_authz to "".  It also
  dropped the check for has_tls_authz from
  hmp_info_migrate_parameters().

  Revert, and fix qmp_query_migrate_parameters() not to screw up
  has_tls_authz.  No change to HMP.  QMP now has "tls-authz" in the
  reply only when it's actually present in
  migrate_get_current()->parameters.  If we prefer to remain
  bug-compatible, we should make tls_authz non-optional there.

* migrate_params_test_apply() neglects to apply tls_authz.  Currently
  harmless, because migrate_params_check() doesn't care.  Fix it
  anyway.

* qmp_migrate_set_parameters() crashes:

    {"execute": "migrate-set-parameters", "arguments": {"tls-authz": null}}

  Add the necessary rewrite of null to "".  For background
  information, see commit 01fa559826 "migration: Use JSON null instead
  of "" to reset parameter to default".

Fixes: d2f1d29b95aa45d13262b39153ff501ed6b1ac95
Cc: Daniel P. Berrangé <berra...@redhat.com>
Signed-off-by: Markus Armbruster <arm...@redhat.com>
---
 qapi/migration.json   |  2 +-
 migration/migration.c | 17 ++++++++++++++---
 monitor/hmp-cmds.c    |  2 +-
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/qapi/migration.json b/qapi/migration.json
index 3c75820527..688e8da749 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -928,7 +928,7 @@
 ##
 # @MigrationParameters:
 #
-# The optional members aren't actually optional.
+# The optional members aren't actually optional, except for @tls-authz.
 #
 # @announce-initial: Initial delay (in milliseconds) before sending the
 #                    first announce (Since 4.0)
diff --git a/migration/migration.c b/migration/migration.c
index 3263aa55a9..cad56fbf8c 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -855,9 +855,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error 
**errp)
     params->tls_creds = g_strdup(s->parameters.tls_creds);
     params->has_tls_hostname = true;
     params->tls_hostname = g_strdup(s->parameters.tls_hostname);
-    params->has_tls_authz = true;
-    params->tls_authz = g_strdup(s->parameters.tls_authz ?
-                                 s->parameters.tls_authz : "");
+    params->has_tls_authz = s->parameters.has_tls_authz;
+    params->tls_authz = g_strdup(s->parameters.tls_authz);
     params->has_max_bandwidth = true;
     params->max_bandwidth = s->parameters.max_bandwidth;
     params->has_downtime_limit = true;
@@ -1433,6 +1432,11 @@ static void 
migrate_params_test_apply(MigrateSetParameters *params,
         dest->tls_hostname = params->tls_hostname->u.s;
     }
 
+    if (params->has_tls_authz) {
+        assert(params->tls_authz->type == QTYPE_QSTRING);
+        dest->tls_authz = params->tls_authz->u.s;
+    }
+
     if (params->has_max_bandwidth) {
         dest->max_bandwidth = params->max_bandwidth;
     }
@@ -1622,6 +1626,13 @@ void qmp_migrate_set_parameters(MigrateSetParameters 
*params, Error **errp)
         params->tls_hostname->type = QTYPE_QSTRING;
         params->tls_hostname->u.s = strdup("");
     }
+    /* TODO Rewrite "" to null instead */
+    if (params->has_tls_authz
+        && params->tls_authz->type == QTYPE_QNULL) {
+        qobject_unref(params->tls_authz->u.n);
+        params->tls_authz->type = QTYPE_QSTRING;
+        params->tls_authz->u.s = strdup("");
+    }
 
     migrate_params_test_apply(params, &tmp);
 
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index a6a6684df1..492789248f 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -476,7 +476,7 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict 
*qdict)
             params->max_postcopy_bandwidth);
         monitor_printf(mon, "%s: '%s'\n",
             MigrationParameter_str(MIGRATION_PARAMETER_TLS_AUTHZ),
-            params->tls_authz);
+            params->has_tls_authz ? params->tls_authz : "");
 
         if (params->has_block_bitmap_mapping) {
             const BitmapMigrationNodeAliasList *bmnal;
-- 
2.26.2