Re: [Qemu-devel] [PATCH] migration: allow clearing migration string parameters

[Markus Armbruster <armbru@redhat.com>]

"Daniel P. Berrange" <berrange@redhat.com> writes:

> On Wed, Mar 01, 2017 at 08:36:03AM -0600, Eric Blake wrote:
>> On 03/01/2017 06:32 AM, Daniel P. Berrange wrote:
>> >      }
>> >      if (params->has_tls_creds) {
>> >          g_free(s->parameters.tls_creds);
>> > -        s->parameters.tls_creds = g_strdup(params->tls_creds);
>> > +        if (*params->tls_creds == '\0') {
>> > +            s->parameters.tls_creds = NULL;
>> 
>> I'm wondering if you should also do s->parameters.has_tls_creds = false
>> at this point?  The visitors expect that if has_tls_creds is true, then
>> the string is non-NULL.
>
> The fact that s->parameters contains has_* fields is completely ignored
> by the migration code afaict. IOW the code behaves as if all the has_*
> fields are hardwired to true in s->parameters, even though that is not
> the case :-) The has_* fields are only used when the various migration
> QMP methods are executed, and those all use a separate MigrationParameters
> struct instance.

Not keeping the has_ members up-to-date is harmless as long as you don't
pass the thing to visitors, including the one hiding in qapi_free_FOO().
That one ignores scalars, though.


>From a more abstract point of view, we have two related data types: one
for the state, and one for state changes requests.

In state, members are always present.

A state change request is a bag of state member change requests, and
each request can either specify the new value or ask for a reset to
default.  Absent member means no change.

We press the same QAPI type into service for both by making all members
optional.

For the state case, we hardwire the has_ to true.  Or even ignore them
completely.

For the state change request, we use has_ = false for "no change", has_
= true with a special value for "reset to default" (new in this patch)
and has_ = true with a non-special value for "set to this value".

Requires a special value outside the set of non-special values.  The
obvious one is JSON null, but the QAPI generator doesn't quite support
that, yet.  "" works here, but is not general.

I think I can get you null support in 2.10.  Would that work for you?