PCI_DEVICE has overwritten DeviceState::unrealize (pci_qdev_unrealize).
However, LSI53C895A, which is a subclass of PCI_DEVICE, overwrites it
again and doesn't save the parent's implementation so the PCI_DEVICE's
implementation of DeviceState::unrealize will never be called when
unrealize a LSI53C895A device.  And it will lead to memory leak and
unplug failure.

For a PCI device, it's better to implement PCIDevice::exit instead of
DeviceState::unrealize.  So let's change to use PCIDevice::exit.

Fixes: a8632434c7e9 ("lsi: implement I/O memory space for Memory Move instructions")
Cc: qemu-stable@nongnu.org
Signed-off-by: Peng Liang <liangpeng10@huawei.com>
---
 hw/scsi/lsi53c895a.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 7d13c7dc1c46..af46c62f0d78 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -2312,7 +2312,7 @@ static void lsi_scsi_realize(PCIDevice *dev, Error **errp)
     scsi_bus_new(&s->bus, sizeof(s->bus), d, &lsi_scsi_info, NULL);
 }
 
-static void lsi_scsi_unrealize(DeviceState *dev)
+static void lsi_scsi_exit(PCIDevice *dev)
 {
     LSIState *s = LSI53C895A(dev);
 
@@ -2325,11 +2325,11 @@ static void lsi_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
     k->realize = lsi_scsi_realize;
+    k->exit = lsi_scsi_exit;
     k->vendor_id = PCI_VENDOR_ID_LSI_LOGIC;
     k->device_id = PCI_DEVICE_ID_LSI_53C895A;
     k->class_id = PCI_CLASS_STORAGE_SCSI;
     k->subsystem_id = 0x1000;
-    dc->unrealize = lsi_scsi_unrealize;
     dc->reset = lsi_scsi_reset;
     dc->vmsd = &vmstate_lsi_scsi;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
-- 
2.29.2

On 02/03/21 14:30, Peng Liang wrote:
> PCI_DEVICE has overwritten DeviceState::unrealize (pci_qdev_unrealize).
> However, LSI53C895A, which is a subclass of PCI_DEVICE, overwrites it
> again and doesn't save the parent's implementation so the PCI_DEVICE's
> implementation of DeviceState::unrealize will never be called when
> unrealize a LSI53C895A device.  And it will lead to memory leak and
> unplug failure.
> 
> For a PCI device, it's better to implement PCIDevice::exit instead of
> DeviceState::unrealize.  So let's change to use PCIDevice::exit.
> 
> Fixes: a8632434c7e9 ("lsi: implement I/O memory space for Memory Move instructions")
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Peng Liang <liangpeng10@huawei.com>
> ---
>   hw/scsi/lsi53c895a.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> index 7d13c7dc1c46..af46c62f0d78 100644
> --- a/hw/scsi/lsi53c895a.c
> +++ b/hw/scsi/lsi53c895a.c
> @@ -2312,7 +2312,7 @@ static void lsi_scsi_realize(PCIDevice *dev, Error **errp)
>       scsi_bus_new(&s->bus, sizeof(s->bus), d, &lsi_scsi_info, NULL);
>   }
>   
> -static void lsi_scsi_unrealize(DeviceState *dev)
> +static void lsi_scsi_exit(PCIDevice *dev)
>   {
>       LSIState *s = LSI53C895A(dev);
>   
> @@ -2325,11 +2325,11 @@ static void lsi_class_init(ObjectClass *klass, void *data)
>       PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>   
>       k->realize = lsi_scsi_realize;
> +    k->exit = lsi_scsi_exit;
>       k->vendor_id = PCI_VENDOR_ID_LSI_LOGIC;
>       k->device_id = PCI_DEVICE_ID_LSI_53C895A;
>       k->class_id = PCI_CLASS_STORAGE_SCSI;
>       k->subsystem_id = 0x1000;
> -    dc->unrealize = lsi_scsi_unrealize;
>       dc->reset = lsi_scsi_reset;
>       dc->vmsd = &vmstate_lsi_scsi;
>       set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
> 

Queued, thanks.

Paolo