Thomas Huth <th...@redhat.com> writes: > On 12.07.2018 18:22, Peter Maydell wrote: >> On 12 July 2018 at 17:16, Markus Armbruster <arm...@redhat.com> wrote: >>> Thomas Huth <th...@redhat.com> writes: >>> >>>> On 12.07.2018 14:06, Markus Armbruster wrote: >>>>> Peter Maydell <peter.mayd...@linaro.org> writes: >>>>> >>>>>> On 11 July 2018 at 17:12, Eduardo Habkost <ehabk...@redhat.com> wrote: >>>>>>> On Wed, Jul 11, 2018 at 09:21:48AM +0200, Thomas Huth wrote: >>>>>>>> Hm, ok, so how to continue here now? Shall we at least mark the >>>>>>>> bcm2836/7 devices with user_creatable=false, so that users can not >>>>>>>> crash >>>>>>>> their QEMU so easily with device_add? The problem with introspection >>>>>>>> via >>>>>>>> device-list-properties would still continue to exist, but I think >>>>>>>> that's >>>>>>>> less likely used in practice... otherwise we could still move the >>>>>>>> qdev_set_parent_bus() calls to the realize() function instead, and just >>>>>>>> add a big fat FIXME comment in front of the code block, so that we >>>>>>>> remember to clean that up one day... >>>>>>> >>>>>>> Crashing device-list-properties should be a blocker bug, IMO. >>>>> >>>>> Seconded. >>>> >>>> Well, maybe I should then not suggest to add a hmp("info qtree") below >>>> the hmp("info qom-tree") in test_one_device() of >>>> tests/device-introspect-test.c ... otherwise we'll be quite busy in the >>>> next weeks... >>> >>> If we can't fix these bugs in time, we can bring back >>> cannot_destroy_with_object_finalize_yet, as Eduardo mentioned upthread. >>> Would be sad, but sad beats crash. >> >> ...but are they actually interesting crashes? Nobody is ever >> going to actually start emulation of an integratorcp machine and >> then try to add a bcm2837 device via the QMP interface, except >> if they're deliberately doing exhaustive testing. > > It's not "device_add" that is a real problem here (otherwise we could > simply use user_creatable=false which we likely should do for this > device anyway), but rather the "device-list-properties" QMP command, > which also works for devices that are marked as user_creatable=false. > > I think it's valid that an upper layer tool scans all devices for their > properties. But since nobody complained about crashes in the past here > already, it seems like no upper layer tool is currently doing this. So I > agree with you that this should not be a blocker for the 3.0 release.
Fixing the crashes by bringing back cannot_destroy_with_object_finalize_yet would be a bit of a cop out, but it would also be so easy that there's really no excuse for leaving them unfixed.