Re: [PATCH-for-5.1] hw/isa/isa-superio: Fix IDE controller realization

[Philippe Mathieu-Daudé]

On 7/21/20 11:57 AM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
> 
>> On 7/21/20 10:15 AM, Markus Armbruster wrote:
>>> Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
>>>
>>>> When realizing a Super I/O with IDE controller [*], we get:
>>>>
>>>>   qom/object.c:1684: object_property_try_add_child: Assertion 
>>>> `!child->parent' failed.
>>>>   Aborted (core dumped)
>>>>
>>>> This is because the device is already realized when we try to
>>>> add the QOM property to the parent. Fix by realizing *after*
>>>> adding the QOM relationship.
>>>>
>>>> [*] Set ISASuperIOClass::ide.count = N with N not zero
>>>
>>> Is this a latent bug, or can it bite in master?  If the latter, can you
>>> show how to reproduce?
>>
>> Latent bug for master:
>>
>> $ git grep ide.count
>> hw/isa/isa-superio.c:149:    if (k->ide.count && (!k->ide.is_enabled ||
>> k->ide.is_enabled(sio, 0))) {
>> hw/isa/isa-superio.c:197:    sc->ide.count = 0;
>> hw/isa/smc37c669-superio.c:100:    sc->ide.count = 0;
>> hw/isa/vt82c686.c:529:    sc->ide.count = 0;
>>
>> I hit it rebasing undergoing series for 5.2 and testing them.
> 
> Let's tweak the commit message like this
> 
>     [*] Set ISASuperIOClass::ide.count = N with N not zero (no such
>     thing currently exists; the bug is latent)
> 
> Preferably with that or something like it:
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

OK. I still consider it worthwhile fixing for 5.1, as while it
doesn't bite master, forks might be affected, since the bug is
in master.