Re: [Qemu-devel] [PATCH for-4.0 v9 09/16] qemu_thread: supplement error handling for pci_edu_realize

[Markus Armbruster]

Peter Xu <address@hidden> writes:

> On Tue, Jan 08, 2019 at 07:14:11AM +0100, Jiri Slaby wrote:
>> On 07. 01. 19, 18:29, Markus Armbruster wrote:
>> >    static void pci_edu_uninit(PCIDevice *pdev)
>> >    {
>> >        EduState *edu = EDU(pdev);
>> > 
>> >        qemu_mutex_lock(&edu->thr_mutex);
>> >        edu->stopping = true;
>> >        qemu_mutex_unlock(&edu->thr_mutex);
>> >        qemu_cond_signal(&edu->thr_cond);
>> >        qemu_thread_join(&edu->thread);
>> > 
>> >        qemu_cond_destroy(&edu->thr_cond);
>> >        qemu_mutex_destroy(&edu->thr_mutex);
>> > 
>> >        timer_del(&edu->dma_timer);
>> >    }
>> > 
>> > Preexisting: pci_edu_uninit() neglects to call msi_uninit().  Jiri?\
>> 
>> I don't know, the MSI support was added in:
>> commit eabb5782f70b4a10975b24ccd7129929a05ac932
>> Author: Peter Xu <address@hidden>
>> Date:   Wed Sep 28 21:03:39 2016 +0800
>> 
>>     hw/misc/edu: support MSI interrupt
>> 
>> Hence CCing Peter.
>
> Hi, Jiri, Markus, Fei,
>
> IMHO msi_uninit() is optional since it only operates on the config
> space of the device to remove the capability or fix up the flags
> without really doing any real destruction of objects so nothing will
> be leaked (unlike msix_uninit, which should be required).

Michael, Marcel, is neglecting to call msi_uninit() okay, a harmless
bug, or a harmful bug?

>                                                            But I do
> agree that calling msi_uninit() could be even nicer here.
>
> Anyone would like to post a patch? Or should I?

Please coordinate fixing this with Fei Li.