Re: [Qemu-devel] [PATCHv3] dma/i82374: avoid double creation of i82374 device

[Thomas Huth <thuth@redhat.com>]

On 27.11.2017 09:40, Eduardo Otubo wrote:
> On Fri, Nov 24, 2017 at 06:44:59PM +0100, Thomas Huth wrote:
>>  Hi Eduardo,
>>
>> On 24.11.2017 14:46, Eduardo Otubo wrote:
>>> v3:
>>>  * Removed all unecessary local_err
>>>  * Change return of isa_bus_dma() and DMA_init() from void to int8_t,
>>>    returning -EBUSY on error and 0 on success
>>>  * Added qdev_cleanup_nofail() in case isa_bus_dma() returns error. The
>>>    cleanup looks safe, but please review if I didn't miss any detail
>>>
>>> v2:
>>>  * Removed user_creatable=false and replaced by error handling using
>>>    Error **errp and error_propagate();
>>
>> Version changelog should go below the "---" separator, otherwise it will
>> be included in the git changelog as well, which is kind of ugly.
>>
>>> QEMU fails when used with the following command line:
>>>
>>>     ./ppc64-softmmu/qemu-system-ppc64 -S -machine 40p,accel=tcg -device i82374
>>>     qemu-system-ppc64: hw/isa/isa-bus.c:110: isa_bus_dma: Assertion `!bus->dma[0] && !bus->dma[1]' failed.
>>>     Aborted (core dumped)
>>>
>>> The 40p machine type already created the device i82374. If specified in the
>>> command line, it will try to create it again, hence generating the error. The
>>> function isa_bus_dma() isn't supposed to be called twice for the same bus. One
>>> way to avoid this problem is to set user_creatable=false.
>>
>> You don't do that user_creatable=false here anymore, so please remove it
>> from the description.
>>
>>> A possible fix in a near future would be making
>>> isa_bus_dma()/DMA_init()/i82374_realize() return an error instead of asserting
>>> as well.
>>
>> You should rephrase that sentence as well.
>>
> 
> Well, I think one mistake lead to another here. I've always put the changelog
> before the --- and that's why the old commit message. For example:
> 2f668be77501c0232a84aafb6a066c9915987f0e. But I guess on that context it made
> sense to use the changelog since the commit message was too simplistic. I'm
> gonna fix this on the v3 then, among other thigs that I need to fix. Thanks for
> the heads up :)

 Hi,

did you ever send a v4? The problem seems still to persist...

 Thomas