|
From: | Cao jin |
Subject: | Re: [Qemu-devel] [PATCH RFC] pci: call PCIDeviceClass->exit on its .realize failure |
Date: | Mon, 19 Sep 2016 14:16:54 +0800 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 |
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 <address@hidden> CC: Marcel Apfelbaum <address@hidden> Signed-off-by: Cao jin <address@hidden> --- 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
[Prev in Thread] | Current Thread | [Next in Thread] |