Hi Philippe,
On 2025/2/28 17:24, Philippe Mathieu-Daudé wrote: > Hi Zheng, > > On 28/2/25 06:03, Zheng Huang wrote: >> This commit adds failback routine for `virtio_pci_realize` to >> fix the memory leak of an address space and the virtio-net device object. >> If the realization of the device failed, the address space should be >> destroyed too. >> >> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2845 >> >> Signed-off-by: Zheng Huang <hz1624917...@outlook.com> >> >> --- >> hw/virtio/virtio-pci.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c >> index c773a9130c..4b0d8cd90a 100644 >> --- a/hw/virtio/virtio-pci.c >> +++ b/hw/virtio/virtio-pci.c >> @@ -2266,6 +2266,9 @@ static void virtio_pci_realize(PCIDevice *pci_dev, >> Error **errp) >> virtio_pci_bus_new(&proxy->bus, sizeof(proxy->bus), proxy); >> if (k->realize) { >> k->realize(proxy, errp); >> + if (*errp) { >> + address_space_destroy(&proxy->modern_cfg_mem_as); >> + } >> } >> } >> > > I think instead we want to add an instance_init in virtio_pci_class_init > and move the address_space_init call from virtio_pci_realize there. > > Regards, > > Phil. I have reviewed the relevant code again and found that if address_space_init is moved into instance_init, it will not be able to take follow-up actions such as free the AS if device realization failed, thus failing to address the issue. Additionally, I referred to the code for AS initialization and destruction in other devices and found that they are managed in device realize and unrealize handlers. Therefore, I still believe the previous approach is a better choice. If there are other potential solutions or considerations that I might have missed, please let me know. I'm looking forward to hearing your thoughts! Best regards, Zheng.
