All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Andreas Färber" <afaerber@suse.de>
To: Markus Armbruster <armbru@redhat.com>, qemu-devel@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>,
	Eduardo Habkost <ehabkost@redhat.com>,
	Richard Henderson <rth@twiddle.net>,
	Bastian Koppelmann <kbastian@mail.uni-paderborn.de>,
	Anthony Green <green@moxielogic.com>,
	Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>,
	Alexander Graf <agraf@suse.de>,
	qemu-stable@nongnu.org, Blue Swirl <blauwirbel@gmail.com>,
	Max Filippov <jcmvbkbc@gmail.com>,
	Michael Walle <michael@walle.cc>,
	qemu-ppc@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>,
	"Edgar E. Iglesias" <edgar.iglesias@gmail.com>,
	Guan Xuetao <gxt@mprc.pku.edu.cn>,
	Leon Alrae <leon.alrae@imgtec.com>,
	Aurelien Jarno <aurelien@aurel32.net>, Jia Liu <proljc@gmail.com>
Subject: Re: [Qemu-devel] [PULL v3 11/12] qdev: Protect device-list-properties against broken devices
Date: Fri, 9 Oct 2015 17:32:11 +0200	[thread overview]
Message-ID: <5617DDFB.1070005@suse.de> (raw)
In-Reply-To: <1444401407-7849-12-git-send-email-armbru@redhat.com>

Am 09.10.2015 um 16:36 schrieb Markus Armbruster:
> Several devices don't survive object_unref(object_new(T)): they crash
> or hang during cleanup, or they leave dangling pointers behind.
> 
> 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:
> 
>     $ qemu-system-aarch64 -nodefaults -display none -machine none -S -qmp 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_region_finalize: Assertion `((&mr->subregions)->tqh_first == ((void *)0))' failed.
>     Aborted (core dumped)
>     [Exit 134 (SIGABRT)]
> 
> Unfortunately, I can't fix the problems in these devices right now.
> Instead, add DeviceClass member cannot_destroy_with_object_finalize_yet
> to mark them:
> 
> * Hang during cleanup (didn't debug, so I can't say why):
>   "realview_pci", "versatile_pci".
> 
> * Dangling pointer in cpus: most CPUs, plus "allwinner-a10", "digic",
>   "fsl,imx25", "fsl,imx31", "xlnx,zynqmp", because they create such
>   CPUs
> 
> * 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)
> 
> Make qmp_device_list_properties() fail cleanly when the device is so
> marked.  This improves device-list-properties from "crashes, hangs or
> leaves dangling pointers behind" 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'".
> 
> 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:
> 
>     $ qemu-system-aarch64 -machine none -device pxa2xx-pcmcia,help
> 
> Before:
> 
>     qemu-system-aarch64: .../memory.c:1307: memory_region_finalize: Assertion `((&mr->subregions)->tqh_first == ((void *)0))' failed.
> 
> After:
> 
>     Can't list properties of device 'pxa2xx-pcmcia'
> 
> Cc: "Andreas Färber" <afaerber@suse.de>

In case a v4 comes up, ;)

Acked-by: Andreas Färber <afaerber@suse.de>

Thanks for updating,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)

  reply	other threads:[~2015-10-09 15:32 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-09 14:36 [Qemu-devel] [PULL v3 00/12] Fix device introspection regressions Markus Armbruster
2015-10-09 14:36 ` [Qemu-devel] [PULL v3 01/12] update-linux-headers: Rename SW_MAX to SW_MAX_ Markus Armbruster
2015-10-09 14:36 ` [Qemu-devel] [PULL v3 02/12] virtio-input: Fix device introspection on non-Linux hosts Markus Armbruster
2015-10-09 14:36 ` [Qemu-devel] [PULL v3 03/12] memory: allow destroying a non-empty MemoryRegion Markus Armbruster
2015-10-09 14:36 ` [Qemu-devel] [PULL v3 04/12] hw: do not pass NULL to memory_region_init from instance_init Markus Armbruster
2015-10-09 14:36 ` [Qemu-devel] [PULL v3 05/12] macio: move DBDMA_init from instance_init to realize Markus Armbruster
2015-10-09 14:36 ` [Qemu-devel] [PULL v3 06/12] tests: Fix how qom-test is run Markus Armbruster
2015-10-09 14:36 ` [Qemu-devel] [PULL v3 07/12] libqtest: Clean up unused QTestState member sigact_old Markus Armbruster
2015-10-09 14:36 ` [Qemu-devel] [PULL v3 08/12] libqtest: New hmp() & friends Markus Armbruster
2015-10-09 14:36 ` [Qemu-devel] [PULL v3 09/12] device-introspect-test: New, covering device introspection Markus Armbruster
2015-10-09 14:36 ` [Qemu-devel] [PULL v3 10/12] qmp: Fix device-list-properties not to crash for abstract device Markus Armbruster
2015-10-09 14:36 ` [Qemu-devel] [PULL v3 11/12] qdev: Protect device-list-properties against broken devices Markus Armbruster
2015-10-09 15:32   ` Andreas Färber [this message]
2015-10-09 14:36 ` [Qemu-devel] [PULL v3 12/12] Revert "qdev: Use qdev_get_device_class() for -device <type>, help" Markus Armbruster
2015-10-12 12:13 ` [Qemu-devel] [PULL v3 00/12] Fix device introspection regressions Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5617DDFB.1070005@suse.de \
    --to=afaerber@suse.de \
    --cc=agraf@suse.de \
    --cc=armbru@redhat.com \
    --cc=aurelien@aurel32.net \
    --cc=blauwirbel@gmail.com \
    --cc=edgar.iglesias@gmail.com \
    --cc=ehabkost@redhat.com \
    --cc=green@moxielogic.com \
    --cc=gxt@mprc.pku.edu.cn \
    --cc=jcmvbkbc@gmail.com \
    --cc=kbastian@mail.uni-paderborn.de \
    --cc=leon.alrae@imgtec.com \
    --cc=mark.cave-ayland@ilande.co.uk \
    --cc=michael@walle.cc \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=proljc@gmail.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    --cc=rth@twiddle.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.