Hi, Markus,
On Tue, Oct 10, 2023 at 09:18:23PM +0200, Markus Armbruster wrote:
[...]
> >> The point I was trying to make is this. Before the patch, we reject
> >> attempts to set the property value to null. Afterwards, we accept them,
> >> i.e. the patch loses "reject null property value". If this loss is
> >> undesirable, we better replace it with suitable hand-written code.
> >
> > I don't even know how to set it to NULL before.. as it can only be accessed
> > via cmdline "-global" as mentioned above, which must be a string anyway.
> > So I assume this is not an issue.
>
> Something like
>
> {"execute": "migrate-set-parameters",
> "arguments": {"tls-authz": null}}
>
> Hmm, crashes in migrate_params_apply(), which is a bug. I'm getting
> more and more suspicious about user-facing migration code...
Did you apply patch 1 of this series?
https://lore.kernel.org/qemu-devel/20230905162335.235619-2-pet...@redhat.com/
QMP "migrate-set-parameters" does not go via migration_properties, so even
if we change handling of migration_properties, it shouldn't yet affect the
QMP interface of that.
>
> 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.
> In my "QAPI string visitors crashes" memo, I demonstrated that the crash
> on funny property type predates your series. You merely add another
> instance. Moreover, crashing -global is less serious than a crashing
> monitor command, because only the latter can take down a running guest.
> Can't see why your series needs to wait for a fix of the crash bug.
> Makes sense?
What's your suggestion to move on with this series without a fix for that
crash bug?
I started this series with making tls_* all strings (rather than StrOrNull)
and that actually worked out, mostly. We switched to StrOrNull just
because we think it's cleaner and 100% not breaking anyone (even though I
still don't think the other way will). I don't see how I can proceed this
series without fixing this visitor issue but keep using StrOrNull.
Please don't worry on blocking my work: it won't anymore. The thing I need is:
https://lore.kernel.org/qemu-devel/20230905193802.250440-1-pet...@redhat.com/
While this whole series is just paving way for it. If I can't get
immediate results out of this series, I'll just go with the triplications,
leaving all the rest for later.
Thanks,
--
Peter Xu
