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

[Markus Armbruster]

"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