Re: [Qemu-devel] [PATCH for-2.10 10/10] migration: Use JSON null instead of "" to reset parameter to default

["Daniel P. Berrange" <berrange@redhat.com>]

On Tue, Jul 18, 2017 at 03:41:26PM +0200, 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 <armbru@redhat.com>
> ---
>  hmp.c                 |  8 ++++++--
>  migration/migration.c | 18 ++++++++++++++++--
>  qapi-schema.json      | 11 +++++++++--
>  3 files changed, 31 insertions(+), 6 deletions(-)

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|