Re: [PATCH 09/11] qdev: Avoid QemuOpts in QMP device_add

[Kevin Wolf]

Am 04.10.2021 um 14:18 hat Damien Hedde geschrieben:
> 
> 
> On 10/1/21 16:42, Peter Krempa wrote:
> > On Fri, Sep 24, 2021 at 11:04:25 +0200, Kevin Wolf wrote:
> > > Directly call qdev_device_add_from_qdict() for QMP device_add instead of
> > > first going through QemuOpts and converting back to QDict.
> > > 
> > > Note that this changes the behaviour of device_add, though in ways that
> > > should be considered bug fixes:
> > > 
> > > QemuOpts ignores differences between data types, so you could
> > > successfully pass a string "123" for an integer property, or a string
> > > "on" for a boolean property (and vice versa).  After this change, the
> > > correct data type for the property must be used in the JSON input.
> > > 
> > > qemu_opts_from_qdict() also silently ignores any options whose value is
> > > a QDict, QList or QNull.
> > 
> > Sorry for chiming in a bit late, but preferrably this commit should be
> > postponed to at least the next release so that we decrease the amount of
> > libvirt users broken by this.
> > 
> > Granted users are supposed to use new libvirt with new qemu but that's
> > not always the case.
> > 
> > Anyways, libvirt is currently mangling all the types to strings with
> > device_add. I'm currently working on fixing it and it will hopefully be
> > done until next-month's release, but preferrably we increase the window
> > of working combinations by postponing this until the next release.
> 
> Switching to qdict is really an improvement I think.
> 
> If we choose to keep legacy behavior working for now, I think we
> should find a way to still do this switch. Maybe we can temporarily
> keep the str_to_int and str_to_bool conversion when converting the
> qdict to the qdev properties afterward?

I guess we can keep the detour through QemuOpts for QMP for now, and
make sure that the command line code bypasses this bit and still
requires correct types for JSON input. It's only this patch that breaks
compatibility with libvirt, patch 8 should still be okay.

Kevin