Re: [PATCH v3 1/3] vfio/pci: Fix resource leak in vfio_realize

[Joao Martins]

On 21/06/2023 09:02, Zhenzhong Duan wrote:
> When adding migration blocker failed in vfio_migration_realize(),
> hotplug will fail and we see below:
> 
> (qemu) device_add 
> vfio-pci,host=81:11.1,id=vfio1,bus=root1,x-enable-migration=true
> 0000:81:11.1: VFIO migration is not supported in kernel
> Error: disallowing migration blocker (--only-migratable) for: VFIO device 
> doesn't support migration
> 
> If we hotplug again we should see same log as above, but we see:
> (qemu) device_add 
> vfio-pci,host=81:11.0,id=vfio0,bus=root0,x-enable-migration=true
> Error: vfio 0000:81:11.0: device is already attached
> 
> That's because some references to VFIO device isn't released,
> we should check return value of vfio_migration_realize() and
> release the references, then VFIO device will be truely
> released when hotplug failed.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>  hw/vfio/pci.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 73874a94de12..c71b0955d81c 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;
>          }
>      }
>  
This doesn't look right. This means that failure to support migration will
deregister the device. Migration "realize" should not condition as to whether
your device finishes the realize. Maybe the fix is elsewhere?