[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.10 10/10] migration: Use JSON

From: Markus Armbruster
Subject: Re: [Qemu-block] [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

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

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

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

See also

    Message-ID: <address@hidden>


    Message-ID: <address@hidden>

reply via email to

[Prev in Thread] Current Thread [Next in Thread]