Re: [Qemu-devel] [PATCH RFC] pci: call PCIDeviceClass->exit on its .realize failure

[Cao jin <caoj.fnst@cn.fujitsu.com>]

Hi,
     sorry for replying late(was in vacation).

On 09/14/2016 07:59 PM, Marcel Apfelbaum wrote:
> On 09/14/2016 12:14 PM, Cao jin wrote:
>> It cannot guarantee all pci devices will free the allocated resource in
>> its .realize function on realize failure.
>>
>> CC: Michael S. Tsirkin <mst@redhat.com>
>> CC: Marcel Apfelbaum <marcel@redhat.com>
>> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
>> ---
>>
>> I found not all the devices will free the allocated resources on .realize
>> failure, and .exit function is the one who take responsibility to free
>> all
>> the resource. In theory, I think it should be PCIDeviceClass->exit who
>> does
>> the cleanup on realize failure, with appropriate check whether certain
>> resources is allocated.
>> It passed make check, but maybe need more confirmation, so, RFC.
>>

>
> Hi,
>
> I think the call to pc->exit is not necessary here.
>
> The call to pc->realize failed, so it is the callee (pc)
> responsibility to clean the resources.
>
> Please see in the same method:
>
>       pci_add_option_rom(pci_dev, is_default_rom, &local_err);
>          if (local_err) {
>              error_propagate(errp, local_err);
>              pci_qdev_unrealize(DEVICE(pci_dev), NULL);
>
>               ^^^^^^^^^^^^^^^^^^^
>
> This time we call pci_qdev_unrealize (that will call pc->exit)
> because pc->realize succeeded and it is now the caller responsibility
> to clean the resources.
>
>
> Have you found a specific scenario that causes problems?

No, it came to my mind when doing the patch "convert to error". Look 
usb_xhci_realize(), it will call usb_xhci_init to do some resource 
allocation, but won't free the resource once return because of 
msi_init's failure(that may be my fault), so the following patch[*] move 
it down.

I agree your view above. We can forget this patch:)

[*]http://lists.nongnu.org/archive/html/qemu-devel/2016-09/msg03063.html

-- 
Yours Sincerely,

Cao jin