>-----Original Message-----
>From: Duan, Zhenzhong <zhenzhong.d...@intel.com>
>Sent: Monday, July 3, 2023 3:15 PM
>To: qemu-devel@nongnu.org
>Cc: alex.william...@redhat.com; c...@redhat.com; Martins, Joao
><joao.m.mart...@oracle.com>; avih...@nvidia.com; Peng, Chao P
><chao.p.p...@intel.com>
>Subject: [PATCH v6 5/7] vfio/migration: Free resources when
>vfio_migration_realize fails
>
>When vfio_realize() succeeds, hot unplug will call vfio_exitfn() to free
>resources allocated in vfio_realize(); when vfio_realize() fails,
>vfio_exitfn() is
>never called and we need to free resources in vfio_realize().
>
>In the case that vfio_migration_realize() fails,
>e.g: with -only-migratable & enable-migration=off, we see below:
>
>(qemu) device_add vfio-pci,host=81:11.1,id=vfio1,bus=root1,enable-
>migration=off
>0000:81:11.1: Migration disabled
>Error: disallowing migration blocker (--only-migratable) for: 0000:81:11.1:
>Migration is disabled for VFIO device
>
>If we hotplug again we should see same log as above, but we see:
>(qemu) device_add vfio-pci,host=81:11.1,id=vfio1,bus=root1,enable-
>migration=off
>Error: vfio 0000:81:11.1: device is already attached
>
>That's because some references to VFIO device isn't released.
>For resources allocated in vfio_migration_realize(), free them by jumping to
>out_deinit path with calling a new function vfio_migration_deinit(). For
>resources allocated in vfio_realize(), free them by jumping to de-register path
>in vfio_realize().
>
Forgot fixes tag:
Fixes: a22651053b59 ("vfio: Make vfio-pci device migration capable")
Thanks
Zhenzhong
>Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
>---
> hw/vfio/migration.c | 33 +++++++++++++++++++++++----------
> hw/vfio/pci.c | 1 +
> 2 files changed, 24 insertions(+), 10 deletions(-)
>
>diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c index
>e6e5e85f7580..e3954570c853 100644
>--- a/hw/vfio/migration.c
>+++ b/hw/vfio/migration.c
>@@ -802,6 +802,17 @@ static int vfio_migration_init(VFIODevice *vbasedev)
> return 0;
> }
>
>+static void vfio_migration_deinit(VFIODevice *vbasedev) {
>+ VFIOMigration *migration = vbasedev->migration;
>+
>+ remove_migration_state_change_notifier(&migration->migration_state);
>+ qemu_del_vm_change_state_handler(migration->vm_state);
>+ unregister_savevm(VMSTATE_IF(vbasedev->dev), "vfio", vbasedev);
>+ vfio_migration_free(vbasedev);
>+ vfio_unblock_multiple_devices_migration();
>+}
>+
> static int vfio_block_migration(VFIODevice *vbasedev, Error *err, Error
>**errp) {
> int ret;
>@@ -866,7 +877,7 @@ int vfio_migration_realize(VFIODevice *vbasedev,
>Error **errp)
> error_setg(&err,
> "%s: VFIO device doesn't support device dirty
> tracking",
> vbasedev->name);
>- return vfio_block_migration(vbasedev, err, errp);
>+ goto add_blocker;
> }
>
> warn_report("%s: VFIO device doesn't support device dirty tracking",
>@@ -875,29 +886,31 @@ int vfio_migration_realize(VFIODevice *vbasedev,
>Error **errp)
>
> ret = vfio_block_multiple_devices_migration(vbasedev, errp);
> if (ret) {
>- return ret;
>+ goto out_deinit;
> }
>
> if (vfio_viommu_preset(vbasedev)) {
> error_setg(&err, "%s: Migration is currently not supported "
> "with vIOMMU enabled", vbasedev->name);
>- return vfio_block_migration(vbasedev, err, errp);
>+ goto add_blocker;
> }
>
> trace_vfio_migration_realize(vbasedev->name);
> return 0;
>+
>+add_blocker:
>+ ret = vfio_block_migration(vbasedev, err, errp);
>+out_deinit:
>+ if (ret) {
>+ vfio_migration_deinit(vbasedev);
>+ }
>+ return ret;
> }
>
> void vfio_migration_exit(VFIODevice *vbasedev) {
> if (vbasedev->migration) {
>- VFIOMigration *migration = vbasedev->migration;
>-
>- remove_migration_state_change_notifier(&migration->migration_state);
>- qemu_del_vm_change_state_handler(migration->vm_state);
>- unregister_savevm(VMSTATE_IF(vbasedev->dev), "vfio", vbasedev);
>- vfio_migration_free(vbasedev);
>- vfio_unblock_multiple_devices_migration();
>+ vfio_migration_deinit(vbasedev);
> }
>
> if (vbasedev->migration_blocker) {
>diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index c2cf7454ece6..9154dd929d07
>100644
>--- a/hw/vfio/pci.c
>+++ b/hw/vfio/pci.c
>@@ -3210,6 +3210,7 @@ static void vfio_realize(PCIDevice *pdev, Error
>**errp)
> ret = vfio_migration_realize(vbasedev, errp);
> if (ret) {
> error_report("%s: Migration disabled", vbasedev->name);
>+ goto out_deregister;
> }
> }
>
>--
>2.34.1
