Re: [Qemu-devel] [PATCH v3 6/7] qdev: Protect device-list-properties against broken devices

2015-09-29 Thread Paolo Bonzini
On 2/09/2015 10:05, Markus Armbruster wrote: > > > 1. I made device-introspection-test run "info qom-tree", which has a > > >lovely propensity to crash when a crappy device left dangling pointer > > >behind. This led me to "cgthree", "cuda", "integrator_debug", > > >"macio-oldworld", "

Re: [Qemu-devel] [PATCH v3 6/7] qdev: Protect device-list-properties against broken devices

2015-09-29 Thread Markus Armbruster
Peter Maydell writes: > On 28 September 2015 at 20:36, Markus Armbruster wrote: >> 1. I made device-introspection-test run "info qom-tree", which has a >>lovely propensity to crash when a crappy device left dangling pointer >>behind. This led me to "cgthree", "cuda", "integrator_debug",

Re: [Qemu-devel] [PATCH v3 6/7] qdev: Protect device-list-properties against broken devices

2015-09-28 Thread Peter Maydell
On 28 September 2015 at 20:36, Markus Armbruster wrote: > 1. I made device-introspection-test run "info qom-tree", which has a >lovely propensity to crash when a crappy device left dangling pointer >behind. This led me to "cgthree", "cuda", "integrator_debug", >"macio-oldworld", "maci

Re: [Qemu-devel] [PATCH v3 6/7] qdev: Protect device-list-properties against broken devices

2015-09-28 Thread Markus Armbruster
Markus Armbruster writes: > Thomas Huth writes: > >> On 28/09/15 10:11, Markus Armbruster wrote: >>> Thomas Huth writes: >>> On 25/09/15 16:17, Markus Armbruster wrote: > Thomas Huth writes: > >> On 24/09/15 20:57, Markus Armbruster wrote: >>> Several devices don't surviv

Re: [Qemu-devel] [PATCH v3 6/7] qdev: Protect device-list-properties against broken devices

2015-09-28 Thread Peter Maydell
On 28 September 2015 at 15:35, Markus Armbruster wrote: > That's a memory_region_init_io(), so I should search for that pattern, > too. Any memory_region_init*() in fact, I guess. >300 hits :( You can ignore all the ones which are in board models or non-qdevified devices, though. -- PMM

Re: [Qemu-devel] [PATCH v3 6/7] qdev: Protect device-list-properties against broken devices

2015-09-28 Thread Markus Armbruster
Thomas Huth writes: > On 28/09/15 10:11, Markus Armbruster wrote: >> Thomas Huth writes: >> >>> On 25/09/15 16:17, Markus Armbruster wrote: Thomas Huth writes: > On 24/09/15 20:57, Markus Armbruster wrote: >> Several devices don't survive object_unref(object_new(T)): they cra

Re: [Qemu-devel] [PATCH v3 6/7] qdev: Protect device-list-properties against broken devices

2015-09-28 Thread Paolo Bonzini
On 28/09/2015 16:17, Markus Armbruster wrote: >> The reason why this particular call has a NULL owner is that the >> (non-qdevified) DBDMA_init object inside it is also passing a NULL >> owner. DBDMA_init object is also doing a few more non-idempotent things >> such as a malloc, a vmstate_register

Re: [Qemu-devel] [PATCH v3 6/7] qdev: Protect device-list-properties against broken devices

2015-09-28 Thread Markus Armbruster
Paolo Bonzini writes: > On 28/09/2015 10:11, Markus Armbruster wrote: >> >> For most of the devices my patch marks, we have a pretty good idea on >> what's wrong with them. spapr-rng is among the exceptions. You believe >> it's actually "the macio object". Which one? "macio" is abstract... >

Re: [Qemu-devel] [PATCH v3 6/7] qdev: Protect device-list-properties against broken devices

2015-09-28 Thread Peter Maydell
On 28 September 2015 at 10:17, Thomas Huth wrote: > By the way, there are some more spots like this in the code, e.g. in > pxa2xx_fir_instance_init() in hw/arm/pxa2xx.c ... That's an oversight from when it was converted to qdev, I think, and could be fixed. -- PMM

Re: [Qemu-devel] [PATCH v3 6/7] qdev: Protect device-list-properties against broken devices

2015-09-28 Thread Thomas Huth
On 28/09/15 10:11, Markus Armbruster wrote: > Thomas Huth writes: > >> On 25/09/15 16:17, Markus Armbruster wrote: >>> Thomas Huth writes: >>> 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

Re: [Qemu-devel] [PATCH v3 6/7] qdev: Protect device-list-properties against broken devices

2015-09-28 Thread Paolo Bonzini
On 28/09/2015 10:15, Andreas Färber wrote: > Am 28.09.2015 um 10:11 schrieb Markus Armbruster: >> Hmm, does creating and destroying a macio object leave the memory region >> behind? >> >> Paolo, is calling memory_region_init() in an instance_init() method >> okay? >> >> If yes, where should they

Re: [Qemu-devel] [PATCH v3 6/7] qdev: Protect device-list-properties against broken devices

2015-09-28 Thread Paolo Bonzini
On 28/09/2015 10:11, Markus Armbruster wrote: > > For most of the devices my patch marks, we have a pretty good idea on > what's wrong with them. spapr-rng is among the exceptions. You believe > it's actually "the macio object". Which one? "macio" is abstract... > > You report introspecting

Re: [Qemu-devel] [PATCH v3 6/7] qdev: Protect device-list-properties against broken devices

2015-09-28 Thread Andreas Färber
Am 28.09.2015 um 10:11 schrieb Markus Armbruster: > Hmm, does creating and destroying a macio object leave the memory region > behind? > > Paolo, is calling memory_region_init() in an instance_init() method > okay? > > If yes, where should they be destroyed, and how? The counterpart to .instance

Re: [Qemu-devel] [PATCH v3 6/7] qdev: Protect device-list-properties against broken devices

2015-09-28 Thread Markus Armbruster
Thomas Huth writes: > On 25/09/15 16:17, Markus Armbruster wrote: >> Thomas Huth writes: >> >>> 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. T

Re: [Qemu-devel] [PATCH v3 6/7] qdev: Protect device-list-properties against broken devices

2015-09-25 Thread Thomas Huth
On 25/09/15 16:17, Markus Armbruster wrote: > Thomas Huth writes: > >> 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. >>> >>> This breaks at least device-list

Re: [Qemu-devel] [PATCH v3 6/7] qdev: Protect device-list-properties against broken devices

2015-09-25 Thread Markus Armbruster
Thomas Huth writes: > 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. >> >> This breaks at least device-list-properties, because >> qmp_device_list_properties()

Re: [Qemu-devel] [PATCH v3 6/7] qdev: Protect device-list-properties against broken devices

2015-09-25 Thread Thomas Huth
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. > > This breaks at least device-list-properties, because > qmp_device_list_properties() needs to create a device to

Re: [Qemu-devel] [PATCH v3 6/7] qdev: Protect device-list-properties against broken devices

2015-09-24 Thread Markus Armbruster
Eduardo Habkost writes: > On Thu, Sep 24, 2015 at 08:57:21PM +0200, Markus Armbruster wrote: > [...] >> diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c >> index ff249af..7692090 100644 >> --- a/hw/arm/allwinner-a10.c >> +++ b/hw/arm/allwinner-a10.c >> @@ -103,6 +103,8 @@ static void

Re: [Qemu-devel] [PATCH v3 6/7] qdev: Protect device-list-properties against broken devices

2015-09-24 Thread Eduardo Habkost
On Thu, Sep 24, 2015 at 08:57:21PM +0200, Markus Armbruster wrote: [...] > diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c > index ff249af..7692090 100644 > --- a/hw/arm/allwinner-a10.c > +++ b/hw/arm/allwinner-a10.c > @@ -103,6 +103,8 @@ static void aw_a10_class_init(ObjectClass *oc,

[Qemu-devel] [PATCH v3 6/7] qdev: Protect device-list-properties against broken devices

2015-09-24 Thread 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: s