From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60744) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZfTD7-0004zW-2v for qemu-devel@nongnu.org; Fri, 25 Sep 2015 09:38:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZfTD3-0005qy-SK for qemu-devel@nongnu.org; Fri, 25 Sep 2015 09:38:49 -0400 References: <1443121042-3409-1-git-send-email-armbru@redhat.com> <1443121042-3409-7-git-send-email-armbru@redhat.com> From: Thomas Huth Message-ID: <56054E5E.3090005@redhat.com> Date: Fri, 25 Sep 2015 15:38:38 +0200 MIME-Version: 1.0 In-Reply-To: <1443121042-3409-7-git-send-email-armbru@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 6/7] qdev: Protect device-list-properties against broken devices List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , qemu-devel@nongnu.org Cc: Peter Maydell , ehabkost@redhat.com, Peter Crosthwaite , qemu-stable@nongnu.org, Alexander Graf , Alistair Francis , Christian Borntraeger , qemu-ppc@nongnu.org, Antony Pavlov , stefanha@redhat.com, Cornelia Huck , Paolo Bonzini , afaerber@suse.de, Li Guang , Richard Henderson On 24/09/15 20:57, Markus Armbruster wrote: > Several devices don't survive object_unref(object_new(T)): they crash > or hang during cleanup, or they leave dangling pointers behind. >=20 > This breaks at least device-list-properties, because > qmp_device_list_properties() needs to create a device to find its > properties. Broken in commit f4eb32b "qmp: show QOM properties in > device-list-properties", v2.1. Example reproducer: >=20 > $ qemu-system-aarch64 -nodefaults -display none -machine none -S -q= mp stdio > {"QMP": {"version": {"qemu": {"micro": 50, "minor": 4, "major": 2},= "package": ""}, "capabilities": []}} > { "execute": "qmp_capabilities" } > {"return": {}} > { "execute": "device-list-properties", "arguments": { "typename": "= pxa2xx-pcmcia" } } > qemu-system-aarch64: /home/armbru/work/qemu/memory.c:1307: memory_r= egion_finalize: Assertion `((&mr->subregions)->tqh_first =3D=3D ((void *)= 0))' failed. > Aborted (core dumped) > [Exit 134 (SIGABRT)] >=20 > Unfortunately, I can't fix the problems in these devices right now. > Instead, add DeviceClass member cannot_even_create_with_object_new_yet > to mark them: >=20 > * Crash during init (didn't debug, so I can't say why): "spapr-rng" >=20 > * Crash or hang during cleanup (didn't debug, so I can't say why): > "pxa2xx-pcmcia", "realview_pci", "versatile_pci", > "s390-sclp-event-facility", "sclp" >=20 > * Dangling pointers: most CPUs, plus "allwinner-a10", "digic", > "fsl,imx25", "fsl,imx31", "xlnx,zynqmp", because they create such > CPUs >=20 > * Assert kvm_enabled(): "host-x86_64-cpu", host-i386-cpu", > "host-powerpc64-cpu", "host-embedded-powerpc-cpu", > "host-powerpc-cpu" (the powerpc ones can't currently reach the > assertion, because the CPUs are only registered when KVM is enabled, > but the assertion is arguably in the wrong place all the same) >=20 > Make qmp_device_list_properties() fail cleanly when the device is so > marked. This improves device-list-properties from "crashes or hangs" > to "fails". Not a complete fix, just a better-than-nothing > work-around. In the above reproducer, device-list-properties now > fails with "Can't list properties of device 'pxa2xx-pcmcia'". >=20 > This also protects -device FOO,help, which uses the same machinery > since commit ef52358 "qdev-monitor: include QOM properties in -device > FOO, help output", v2.2. Example reproducer: >=20 > $ qemu-system-aarch64 -machine none -device pxa2xx-pcmcia,help >=20 > Before: >=20 > qemu-system-aarch64: .../memory.c:1307: memory_region_finalize: Ass= ertion `((&mr->subregions)->tqh_first =3D=3D ((void *)0))' failed. >=20 > After: >=20 > Can't list properties of device 'pxa2xx-pcmcia' >=20 > Cc: "Andreas F=C3=A4rber" > Cc: Alexander Graf > Cc: Alistair Francis > Cc: Antony Pavlov > Cc: Christian Borntraeger > Cc: Cornelia Huck > Cc: Eduardo Habkost > Cc: Li Guang > Cc: Paolo Bonzini > Cc: Peter Crosthwaite > Cc: Peter Maydell > Cc: Richard Henderson > Cc: qemu-ppc@nongnu.org > Cc: qemu-stable@nongnu.org > Signed-off-by: Markus Armbruster > --- ... > static void pxa2xx_pcmcia_register_types(void) > diff --git a/hw/ppc/spapr_rng.c b/hw/ppc/spapr_rng.c > index ed43d5e..e1b115d 100644 > --- a/hw/ppc/spapr_rng.c > +++ b/hw/ppc/spapr_rng.c > @@ -169,6 +169,11 @@ static void spapr_rng_class_init(ObjectClass *oc, = void *data) > dc->realize =3D spapr_rng_realize; > set_bit(DEVICE_CATEGORY_MISC, dc->categories); > dc->props =3D spapr_rng_properties; > + > + /* > + * Reason: crashes device-introspect-test for unknown reason. > + */ > + dc->cannot_even_create_with_object_new_yet =3D true; > } Please don't do that! That breaks the help output from "-device spapr-rng,?" which should help the user to see how to use this device! I tried to debug why this device breaks the test, but the test environment is giving me a hard time ... how do you best hook a gdb into that framework, so you can trace such problems? Anyway, with some trial and error, I found out that it seems like the object_resolve_path_type("", TYPE_SPAPR_RNG, NULL) in spapr_rng_instance_init() is causing the problems. Could it be that object_resolve_path_type is not working with the test environment? Thomas