aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMarkus Armbruster <armbru@redhat.com>2017-07-18 14:42:04 +0200
committerMarkus Armbruster <armbru@redhat.com>2017-07-24 13:35:11 +0200
commit01fa55982692fb51a16049b63b571651a1053989 (patch)
treef433368ed1c6a56b6ef45b287c7731b8cd87bbeb
parent1bda8b3c6950f74482ba19e8529db72b511ba977 (diff)
downloadqemu-arm-01fa55982692fb51a16049b63b571651a1053989.tar.gz
migration: Use JSON null instead of "" to reset parameter to default
migrate-set-parameters sets migration parameters according to is arguments like this: * Present means "set the parameter to this value" * Absent means "leave the parameter unchanged" * Except for parameters tls_creds and tls_hostname, "" means "reset the parameter to its default value The first two are perfectly normal: presence of the parameter makes the command do something. The third one overloads the parameter with a second meaning. The overloading is *implicit*, i.e. it's not visible in the types. Works here, because "" is neither a valid TLS credentials ID, nor a valid host name. Pressing argument values the schema accepts, but are semantically invalid, into service to mean "reset to default" is not general, as suitable invalid values need not exist. I also find it ugly. To clean this up, we could add a separate flag argument to ask for "reset to default", or add a distinct value to @tls_creds and @tls_hostname. This commit implements the latter: add JSON null to the values of @tls_creds and @tls_hostname, deprecate "". Because we're so close to the 2.10 freeze, implement it in the stupidest way possible: have qmp_migrate_set_parameters() rewrite null to "" before anything else can see the null. The proper way to do it would be rewriting "" to null, but that requires fixing up code to work with null. Add TODO comments for that. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
-rw-r--r--hmp.c8
-rw-r--r--migration/migration.c27
-rw-r--r--qapi-schema.json20
3 files changed, 47 insertions, 8 deletions
diff --git a/hmp.c b/hmp.c
index 4b6b1149bb..fd80dce758 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1586,11 +1586,15 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
break;
case MIGRATION_PARAMETER_TLS_CREDS:
p->has_tls_creds = true;
- visit_type_str(v, param, &p->tls_creds, &err);
+ p->tls_creds = g_new0(StrOrNull, 1);
+ p->tls_creds->type = QTYPE_QSTRING;
+ visit_type_str(v, param, &p->tls_creds->u.s, &err);
break;
case MIGRATION_PARAMETER_TLS_HOSTNAME:
p->has_tls_hostname = true;
- visit_type_str(v, param, &p->tls_hostname, &err);
+ p->tls_hostname = g_new0(StrOrNull, 1);
+ p->tls_hostname->type = QTYPE_QSTRING;
+ visit_type_str(v, param, &p->tls_hostname->u.s, &err);
break;
case MIGRATION_PARAMETER_MAX_BANDWIDTH:
p->has_max_bandwidth = true;
diff --git a/migration/migration.c b/migration/migration.c
index 205b801b70..085c32c994 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -770,11 +770,13 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
}
if (params->has_tls_creds) {
- dest->tls_creds = g_strdup(params->tls_creds);
+ assert(params->tls_creds->type == QTYPE_QSTRING);
+ dest->tls_creds = g_strdup(params->tls_creds->u.s);
}
if (params->has_tls_hostname) {
- dest->tls_hostname = g_strdup(params->tls_hostname);
+ assert(params->tls_hostname->type == QTYPE_QSTRING);
+ dest->tls_hostname = g_strdup(params->tls_hostname->u.s);
}
if (params->has_max_bandwidth) {
@@ -822,12 +824,14 @@ static void migrate_params_apply(MigrateSetParameters *params)
if (params->has_tls_creds) {
g_free(s->parameters.tls_creds);
- s->parameters.tls_creds = g_strdup(params->tls_creds);
+ assert(params->tls_creds->type == QTYPE_QSTRING);
+ s->parameters.tls_creds = g_strdup(params->tls_creds->u.s);
}
if (params->has_tls_hostname) {
g_free(s->parameters.tls_hostname);
- s->parameters.tls_hostname = g_strdup(params->tls_hostname);
+ assert(params->tls_hostname->type == QTYPE_QSTRING);
+ s->parameters.tls_hostname = g_strdup(params->tls_hostname->u.s);
}
if (params->has_max_bandwidth) {
@@ -858,6 +862,21 @@ void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
{
MigrationParameters tmp;
+ /* TODO Rewrite "" to null instead */
+ if (params->has_tls_creds
+ && params->tls_creds->type == QTYPE_QNULL) {
+ QDECREF(params->tls_creds->u.n);
+ params->tls_creds->type = QTYPE_QSTRING;
+ params->tls_creds->u.s = strdup("");
+ }
+ /* TODO Rewrite "" to null instead */
+ if (params->has_tls_hostname
+ && params->tls_hostname->type == QTYPE_QNULL) {
+ QDECREF(params->tls_hostname->u.n);
+ params->tls_hostname->type = QTYPE_QSTRING;
+ params->tls_hostname->u.s = strdup("");
+ }
+
migrate_params_test_apply(params, &tmp);
if (!migrate_params_check(&tmp, errp)) {
diff --git a/qapi-schema.json b/qapi-schema.json
index 70c35412df..9c6c3e1a53 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -116,6 +116,22 @@
{ 'command': 'qmp_capabilities' }
##
+# @StrOrNull:
+#
+# This is a string value or the explicit lack of a string (null
+# pointer in C). Intended for cases when 'optional absent' already
+# has a different meaning.
+#
+# @s: the string value
+# @n: no string value
+#
+# Since: 2.10
+##
+{ 'alternate': 'StrOrNull',
+ 'data': { 's': 'str',
+ 'n': 'null' } }
+
+##
# @LostTickPolicy:
#
# Policy for handling lost ticks in timer devices.
@@ -1098,8 +1114,8 @@
'*decompress-threads': 'int',
'*cpu-throttle-initial': 'int',
'*cpu-throttle-increment': 'int',
- '*tls-creds': 'str',
- '*tls-hostname': 'str',
+ '*tls-creds': 'StrOrNull',
+ '*tls-hostname': 'StrOrNull',
'*max-bandwidth': 'int',
'*downtime-limit': 'int',
'*x-checkpoint-delay': 'int',