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 <m...@redhat.com>
CC: Marcel Apfelbaum <mar...@redhat.com>
Signed-off-by: Cao jin <caoj.f...@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
the resource. In theory, I think it should be PCIDeviceClass->exit who
the cleanup on realize failure, with appropriate check whether certain
resources is allocated.
It passed make check, but maybe need more confirmation, so, RFC.


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:)


Yours Sincerely,

Cao jin

Reply via email to