[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.10 10/10] migration: Use JSON null instead
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH for-2.10 10/10] migration: Use JSON null instead of "" to reset parameter to default |
Date: |
Tue, 18 Jul 2017 21:24:08 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) |
"Dr. David Alan Gilbert" <address@hidden> writes:
> * Markus Armbruster (address@hidden) 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
>
> Is this really what's happening? IMHO the tls_creds and tls_hostname
> behaviour isn't that "" resets to the default, it just is the default.
> I don't think there's anything special cased for tls_creds and
> tls_hostname in the existing code. It's this patch that's
> adding more special casing.
>
> (I'm not going to nack this, but I just don't get why it's such a big
> deal)
It wouldn't call it a big deal. In fact, I called it a "minor QMP
interface design flaw".
The implementation encodes "no TLS credentials, do not use TLS" and "no
host name, fall back to the one in the migration URI" as "". Works
because "" is neither a valid TLS credentials ID, nor a valid host name.
Until commit 4af245d (v2.9.0), the implementation used NULL / optional
absent rather than "". Cleaner, because it doesn't rely on "" being
invalid.
The reason why commit 4af245d changed it to "" was that
migrate-set-parameters can't do NULL / optional absent. It can't,
because it already uses optional absent for "do not change this
parameter".
Replacing NULL by "" side-stepped this problem. I dislike that because
it's not general (only works as long as "" is not a valid value), and
ugly.
Instead of side-stepping the problem, I proposed to tackle it: make JSON
null mean NULL in migrate-set-parameters. However, the freeze was
literally the other day, we needed *some* solution, and only the one
making "" special was ready. So I let it pass.
Related: NULL used to be shown as "" in query-migrate-parameters.
Commit de63ab6 (v2.8.0) fixed that oddity, but commit 4af245d (v2.9.0)
regressed the fix, probably unintentionally.
My patch doesn't fix that regression. It doesn't revert the flip from
NULL to "". It merely corrects the QAPI interface, in the stupidest way
possible, because that's all I could do for 2.10's soft freeze.
I'm open to fixing the query-migrate-parameters regression during
freeze.
See also
Message-ID: <address@hidden>
https://lists.gnu.org/archive/html/qemu-devel/2017-07/msg04526.html
and
Message-ID: <address@hidden>
https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg02841.html
- Re: [Qemu-devel] [PATCH for-2.10 01/10] qapi: Separate type QNull from QObject, (continued)
- [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-devel] [PATCH for-2.10 10/10] migration: Use JSON null instead of "" to reset parameter to default, Daniel P. Berrange, 2017/07/18
- Re: [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-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-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-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-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-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-devel] [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-devel] [PATCH for-2.10 09/10] migration: Unshare MigrationParameters struct for now, Markus Armbruster, 2017/07/18
Re: [Qemu-devel] [PATCH for-2.10 00/10] Correct two minor QMP interface design flaws, no-reply, 2017/07/18
Re: [Qemu-devel] [PATCH for-2.10 00/10] Correct two minor QMP interface design flaws, Daniel P. Berrange, 2017/07/18