Re: [PATCH v3 3/4] migration/qapi: Replace @MigrateSetParameters with @MigrationParameters

[Markus Armbruster]

Peter Xu <peterx@redhat.com> writes:

> On Wed, Oct 11, 2023 at 06:28:26AM +0200, Markus Armbruster wrote:

[...]

>> >> If the migration object is accessible with qom-set, then that's another
>> >> way to assign null values.
>> >
>> > I see what you meant.  IMHO we just don't need to worry on breaking that as
>> > I am not aware of anyone using that to set migration parameters, and I
>> > think the whole idea of migration_properties is for debugging.  The only
>> > legal way an user should set migration parameters should be via QMP, afaik.
>> 
>> No matter the intended purpose of an interface, its meaning should be
>> clear.  If we accept null, what does it mean?
>
> IMHO here it means anything parsed from migration_properties will be a
> qstring and impossible to be a qnull, at least if with that of my (probably
> unmature, or even wrong..) fix:
>
> https://lore.kernel.org/all/ZRsff7Lmy7TnggK9@x1n/
>
> +static bool string_input_start_alternate(Visitor *v, const char *name,
> +                                         GenericAlternate **obj, size_t size,
> +                                         Error **errp)
> +{
> +    *obj = g_malloc0(size);
> +    (*obj)->type = QTYPE_QSTRING;          <---------------- constantly set 
> to string type
> +    return true;
> +}
>
> I think it means when we parse the string, we'll always go with:
>
> visit_type_StrOrNull():
>     switch ((*obj)->type) {
>     case QTYPE_QSTRING:
>         ok = visit_type_str(v, name, &(*obj)->u.s, errp);
>         break;
>
> Finally, parse_type_str().
>
> So it'll be impossible to specify null for -global migration.tls_*=XXX.

Only because -global can't do null.  Changes the moment we improve it to
support JSON values.  We already improved several other CLI options that
way.

> I suppose it's fine then?  Because it actually matches with previous when
> it was still a string, and we use empty string to show tls disabled.
>
> Thanks,