Peter Xu <pet...@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,
