Re: [PATCH v4 06/14] vfio-user: find and init PCI device

[Jag Raman <jag.raman@oracle.com>]

> On Dec 16, 2021, at 5:39 AM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> 
> On Wed, Dec 15, 2021 at 10:35:30AM -0500, Jagannathan Raman wrote:
>> @@ -150,6 +157,38 @@ static void vfu_object_init_ctx(VfuObject *o, Error **errp)
>> +    o->pci_dev = PCI_DEVICE(dev);
> ...
>> @@ -190,6 +229,8 @@ static void vfu_object_finalize(Object *obj)
>> 
>>     o->device = NULL;
>> 
>> +    o->pci_dev = NULL;
> 
> We need to consider how this interacts with device_del hot unplug.
> o->pci_dev is a pointer and we don't hold a refcount, so I think
> o->pci_dev could disappear at any moment.
> 
> A pair of object_ref/unref(OBJECT(o->pci_dev)) calls would not be enough
> because device_del will still unrealize the device that's in use by the
> vfio-user server.
> 
> I suggest adding a check to qdev_unplug() that prevents unplug when the
> device is in use by the vfio-user server. That's similar to the code in
> that function for preventing unplug during migration.
> 
> One way to do that is by adding a new API:
> 
>  /*
>   * Register an Error that is raised when unplug is attempted on a
>   * device. If another blocker has already been registered then that
>   * Error may be raised during unplug instead.
>   *
>   * qdev_del_unplug_blocker() must be called to remove this blocker.
>   */
>  void qdev_add_unplug_blocker(DeviceState *dev, Error *blocker);
> 
>  /*
>   * Deregister an Error that was previously registered with
>   * qdev_add_unplug_blocker().
>   */
>  void qdev_del_unplug_blocker(DeviceState *dev, Error *blocker);
> 
> The vfio-user server then needs to add an Error *unplug_blocker field to
> VfuObject and call qdev_add/del_unplug_blocker() on the PCI device.
> 
> From a user perspective this means that device_del fails with "Device
> currently in use by vfio-user server '%s'".

OK sounds good, will add the above unplug blocker API for PCI devices.

Thank you!
--
Jag

> 
> Stefan