[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] hw/arm/bcm283x: Fix crash with device_add bcm28

From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH] hw/arm/bcm283x: Fix crash with device_add bcm2837 on unsupported machines
Date: Thu, 12 Jul 2018 15:19:50 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)

Peter Maydell <address@hidden> writes:

> On 12 July 2018 at 13:06, Markus Armbruster <address@hidden> wrote:
>> Peter Maydell <address@hidden> writes:
>>> On 11 July 2018 at 17:12, Eduardo Habkost <address@hidden> 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.
>>>> Moving to realize is not the best solution, but I would prefer to
>>>> do that in 3.0 instead of leaving the device-list-properties
>>>> crash unfixed.
>>> I would like to see the crash fixed too. But I'd like to
>>> see it fixed:
>>>  (a) by having clear documentation about how the QOM
>>> system works, what you should do in init and what you
>>> should do in realize, when and why you need to manually
>>> parent objects, etc
>>>  (b) as far as possible making our APIs for doing this
>>> easy to use correctly and difficult to use wrongly. At
>>> the moment we have APIs that are far too easy to misuse,
>>> which means we will continue to get bugs like this and spend
>>> a lot of time on one-off fixes for them.
>>> In particular I don't understand why we need to manually
>>> parent these objects at all.
>> I want both (a) and (b) as badly as anyone, but we should not hold any
>> particular crash bug hostage to get them.
> Without at least (a) I can't review this patch or any other
> patch that fixes this kind of crash bug...

Fair point.  But if Paolo can, and vouches for a fix, then we shouldn't
hold the fix hostage.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]