[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH for-2.10 10/10] migration: Use JSON null instead
From: |
Daniel P. Berrange |
Subject: |
Re: [Qemu-block] [PATCH for-2.10 10/10] migration: Use JSON null instead of "" to reset parameter to default |
Date: |
Tue, 18 Jul 2017 17:03:09 +0100 |
User-agent: |
Mutt/1.8.3 (2017-05-23) |
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 <address@hidden>
> ---
> hmp.c | 8 ++++++--
> migration/migration.c | 18 ++++++++++++++++--
> qapi-schema.json | 11 +++++++++--
> 3 files changed, 31 insertions(+), 6 deletions(-)
Reviewed-by: Daniel P. Berrange <address@hidden>
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 :|
- Re: [Qemu-block] [PATCH for-2.10 01/10] qapi: Separate type QNull from QObject, (continued)
- [Qemu-block] [PATCH for-2.10 07/10] migration: Clean up around tls_creds, tls_hostname, Markus Armbruster, 2017/07/18
- [Qemu-block] [PATCH for-2.10 04/10] tests/test-qobject-input-visitor: Drop redundant test, Markus Armbruster, 2017/07/18
- [Qemu-block] [PATCH for-2.10 10/10] migration: Use JSON null instead of "" to reset parameter to default, Markus Armbruster, 2017/07/18
- Re: [Qemu-block] [PATCH for-2.10 10/10] migration: Use JSON null instead of "" to reset parameter to default,
Daniel P. Berrange <=
- Re: [Qemu-block] [PATCH for-2.10 10/10] migration: Use JSON null instead of "" to reset parameter to default, Eric Blake, 2017/07/18
- Re: [Qemu-block] [Qemu-devel] [PATCH for-2.10 10/10] migration: Use JSON null instead of "" to reset parameter to default, Markus Armbruster, 2017/07/18
- Re: [Qemu-block] [Qemu-devel] [PATCH for-2.10 10/10] migration: Use JSON null instead of "" to reset parameter to default, Markus Armbruster, 2017/07/18
- Re: [Qemu-block] [Qemu-devel] [PATCH for-2.10 10/10] migration: Use JSON null instead of "" to reset parameter to default, Eric Blake, 2017/07/18
- Re: [Qemu-block] [Qemu-devel] [PATCH for-2.10 10/10] migration: Use JSON null instead of "" to reset parameter to default, Markus Armbruster, 2017/07/18
- Re: [Qemu-block] [Qemu-devel] [PATCH for-2.10 10/10] migration: Use JSON null instead of "" to reset parameter to default, Eric Blake, 2017/07/18
Re: [Qemu-block] [PATCH for-2.10 10/10] migration: Use JSON null instead of "" to reset parameter to default, Dr. David Alan Gilbert, 2017/07/18
[Qemu-block] [PATCH for-2.10 03/10] qapi: Introduce a first class 'null' type, Markus Armbruster, 2017/07/18