[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL 10/10] migration: Use JSON null instead of "" to rese
From: |
Markus Armbruster |
Subject: |
[Qemu-devel] [PULL 10/10] migration: Use JSON null instead of "" to reset parameter to default |
Date: |
Tue, 18 Jul 2017 22:29:20 +0200 |
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 <address@hidden>
Reviewed-by: Daniel P. Berrange <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
---
hmp.c | 8 ++++++--
migration/migration.c | 18 ++++++++++++++++--
qapi-schema.json | 20 ++++++++++++++++++--
3 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/hmp.c b/hmp.c
index 0a5de75..40ebeef 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1541,11 +1541,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 f6a9443..e2cfb99 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -703,6 +703,20 @@ void qmp_migrate_set_parameters(MigrateSetParameters
*params, Error **errp)
"x_checkpoint_delay",
"is invalid, it should be positive");
}
+ /* 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("");
+ }
/*
* TODO if we fuse MigrateSetParameters back into
@@ -726,11 +740,11 @@ void qmp_migrate_set_parameters(MigrateSetParameters
*params, Error **errp)
}
if (params->has_tls_creds) {
g_free(s->parameters.tls_creds);
- s->parameters.tls_creds = g_strdup(params->tls_creds);
+ 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);
+ s->parameters.tls_hostname = g_strdup(params->tls_hostname->u.s);
}
if (params->has_max_bandwidth) {
s->parameters.max_bandwidth = params->max_bandwidth;
diff --git a/qapi-schema.json b/qapi-schema.json
index b5ec942..9bd3390 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',
--
2.7.5
- [Qemu-devel] [PULL 00/10] QAPI patches for 2017-07-18, Markus Armbruster, 2017/07/18
- [Qemu-devel] [PULL 03/10] qapi: Introduce a first class 'null' type, Markus Armbruster, 2017/07/18
- [Qemu-devel] [PULL 08/10] migration: Add TODO comments on duplication of QAPI_CLONE(), Markus Armbruster, 2017/07/18
- [Qemu-devel] [PULL 06/10] hmp: Clean up and simplify hmp_migrate_set_parameter(), Markus Armbruster, 2017/07/18
- [Qemu-devel] [PULL 10/10] migration: Use JSON null instead of "" to reset parameter to default,
Markus Armbruster <=
- [Qemu-devel] [PULL 02/10] qapi: Use QNull for a more regular visit_type_null(), Markus Armbruster, 2017/07/18
- [Qemu-devel] [PULL 04/10] tests/test-qobject-input-visitor: Drop redundant test, Markus Armbruster, 2017/07/18
- [Qemu-devel] [PULL 07/10] migration: Clean up around tls_creds, tls_hostname, Markus Armbruster, 2017/07/18
- [Qemu-devel] [PULL 01/10] qapi: Separate type QNull from QObject, Markus Armbruster, 2017/07/18
- [Qemu-devel] [PULL 05/10] block: Use JSON null instead of "" to disable backing file, Markus Armbruster, 2017/07/18
- [Qemu-devel] [PULL 09/10] migration: Unshare MigrationParameters struct for now, Markus Armbruster, 2017/07/18
- Re: [Qemu-devel] [PULL 00/10] QAPI patches for 2017-07-18, Peter Maydell, 2017/07/19