On Sat, Nov 01, 2014 at 05:48:19PM +0100, Andreas Färber wrote:
> Am 01.11.2014 um 17:45 schrieb Eduardo Habkost:
> > On Sat, Nov 01, 2014 at 05:40:20PM +0100, Andreas Färber wrote:
> >> Am 01.11.2014 um 16:56 schrieb Eduardo Habkost:
> >>> Make sure we try to list properties from classes that can be safely used
> >>> with
> >>> "-device".
> >>>
> >>> Fixes the following crashes:
> >>>
> >>> $ qemu-system-x86_64 -device x86_64-cpu,help
> >>> **
> >>> ERROR:qom/object.c:336:object_initialize_with_type: assertion failed:
> >>> (type->abstract == false)
> >>> Aborted (core dumped)
> >>> $ qemu-system-x86_64 -device host-x86_64-cpu,help
> >>> qemu-system-x86_64: [...]/target-i386/cpu.c:1329: host_x86_cpu_initfn:
> >>> Assertion `(kvm_allowed)' failed.
> >>> Aborted (core dumped)
> >>>
> >>> After applying this patch:
> >>>
> >>> $ qemu-system-x86_64 -device x86_64-cpu,help
> >>> Parameter 'driver' expects non-abstract device type
> >>> $ qemu-system-x86_64 -device host-x86_64-cpu,help
> >>> Parameter 'driver' expects pluggable device type
> >>>
> >>> Signed-off-by: Eduardo Habkost <ehabk...@redhat.com>
> >>> ---
> >>> qdev-monitor.c | 9 +++------
> >>> 1 file changed, 3 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/qdev-monitor.c b/qdev-monitor.c
> >>> index a9702d8..ebfa701 100644
> >>> --- a/qdev-monitor.c
> >>> +++ b/qdev-monitor.c
> >>> @@ -235,12 +235,9 @@ int qdev_device_help(QemuOpts *opts)
> >>> return 0;
> >>> }
> >>>
> >>> - if (!object_class_by_name(driver)) {
> >>> - const char *typename = find_typename_by_alias(driver);
> >>> -
> >>> - if (typename) {
> >>> - driver = typename;
> >>> - }
> >>> + qdev_get_device_class(&driver, &local_err);
> >>> + if (local_err) {
> >>> + goto error;
> >>> }
> >>>
> >>> prop_list = qmp_device_list_properties(driver, &local_err);
> >>
> >> Is dc->cannot_instantiate_with_device_add_yet || (qdev_hotplug &&
> >> !dc->hotpluggable) really relevant here? Or should that rather remain
> >> outside the common function in 1/3?
> >
> > cannot_instantiate_with_device_add_yet makes sure we won't try to
> > instantiate classes that are not device_add-safe yet (like X86CPU, that
> > has lots of assumptions and side-effects inside instance_init()).
>
> Maybe I'm misunderstanding? Does this code path apply only to device_add
> (then you are right) or does it also apply to -device?
It applies to both -device and device_add, and both reject
cannot_instantiate_with_device_add_yet classes.
The question here is whether it is safe to blindly call object_new() on
the class or not. I assume that a non-hotpluggable or
cannot_instantiate_with_device_add_yet class indicates it is not safe to
do that.
I am being conservative because I don't know how many classes have
side-effects on instance_init today. I believe the next steps could be:
1) Moving the dc->hotpluggable check to qdev_device_add(). Preferably
after ensuring all (!hotpluggable && !cannot_instantiate_with_device_add_yet)
classes have no side-effects on instance_init.
2) Moving the cannot_instantiate_with_device_add_yet check to
qdev_device_add(). Preferably after ensuring all classes have no
side-effects on instance_init.
Or we could just move directly to (1) or (2), and live with the
possibility of having QEMU crashing or misbehaving when using
"-device ...,help" or "device_add ...,help" with some class names.
--
Eduardo
