Eric Blake <ebl...@redhat.com> writes: > On 07/18/2017 08:41 AM, Markus Armbruster wrote: >> 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 <arm...@redhat.com> >> --- >> +++ b/qapi-schema.json >> @@ -116,6 +116,13 @@ >> { 'command': 'qmp_capabilities' } >> >> ## >> +# @StrOrNull: > > A little light on the documentation.
Care to suggest improvements? I figure the schema is obvious enough without any, but the generated documentation could perhaps use some. [...]