From 2b43b2995b02e73b70d10b5be2f2b29b68c7e2fa Mon Sep 17 00:00:00 2001 From: Zhenzhong Duan Date: Mon, 3 Jul 2023 15:15:08 +0800 Subject: vfio/migration: Free resources when vfio_migration_realize fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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(). Signed-off-by: Zhenzhong Duan Fixes: a22651053b59 ("vfio: Make vfio-pci device migration capable") Reviewed-by: Cédric Le Goater Reviewed-by: Joao Martins Signed-off-by: Cédric Le Goater --- hw/vfio/migration.c | 33 +++++++++++++++++++++++---------- hw/vfio/pci.c | 1 + 2 files changed, 24 insertions(+), 10 deletions(-) (limited to 'hw/vfio') diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c index e6e5e85..e395457 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 c2cf745..9154dd9 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; } } -- cgit v1.1