Re: [PATCH v3 05/12] vfio-user: find and init PCI device

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2975 bytes --]

On Mon, Oct 11, 2021 at 01:31:10AM -0400, Jagannathan Raman wrote:
> Find the PCI device with specified id. Initialize the device context
> with the QEMU PCI device
> 
> Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
> Signed-off-by: John G Johnson <john.g.johnson@oracle.com>
> Signed-off-by: Jagannathan Raman <jag.raman@oracle.com>
> ---
>  hw/remote/vfio-user-obj.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c
> index d26e5ec9e9..7ce4e5b256 100644
> --- a/hw/remote/vfio-user-obj.c
> +++ b/hw/remote/vfio-user-obj.c
> @@ -44,6 +44,8 @@
>  #include "qemu/notify.h"
>  #include "sysemu/sysemu.h"
>  #include "libvfio-user.h"
> +#include "hw/qdev-core.h"
> +#include "hw/pci/pci.h"
>  
>  #define TYPE_VFU_OBJECT "vfio-user-server"
>  OBJECT_DECLARE_TYPE(VfuObject, VfuObjectClass, VFU_OBJECT)
> @@ -68,6 +70,8 @@ struct VfuObject {
>      Notifier machine_done;
>  
>      vfu_ctx_t *vfu_ctx;
> +
> +    PCIDevice *pci_dev;
>  };
>  
>  static void vfu_object_set_socket(Object *obj, Visitor *v, const char *name,
> @@ -112,6 +116,9 @@ static void vfu_object_set_device(Object *obj, const char *str, Error **errp)
>  static void vfu_object_machine_done(Notifier *notifier, void *data)
>  {
>      VfuObject *o = container_of(notifier, VfuObject, machine_done);
> +    DeviceState *dev = NULL;
> +    vfu_pci_type_t pci_type = VFU_PCI_TYPE_CONVENTIONAL;
> +    int ret;
>  
>      o->vfu_ctx = vfu_create_ctx(VFU_TRANS_SOCK, o->socket->u.q_unix.path, 0,
>                                  o, VFU_DEV_TYPE_PCI);
> @@ -120,6 +127,31 @@ static void vfu_object_machine_done(Notifier *notifier, void *data)
>                     strerror(errno));
>          return;
>      }
> +
> +    dev = qdev_find_recursive(sysbus_get_default(), o->device);
> +    if (dev == NULL) {
> +        error_setg(&error_abort, "vfu: Device %s not found", o->device);
> +        return;
> +    }
> +
> +    if (!object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
> +        error_setg(&error_abort, "vfu: %s not a PCI device", o->device);
> +        return;
> +    }
> +
> +    o->pci_dev = PCI_DEVICE(dev);
> +
> +    if (pci_is_express(o->pci_dev)) {
> +        pci_type = VFU_PCI_TYPE_EXPRESS;
> +    }
> +
> +    ret = vfu_pci_init(o->vfu_ctx, pci_type, PCI_HEADER_TYPE_NORMAL, 0);
> +    if (ret < 0) {
> +        error_setg(&error_abort,
> +                   "vfu: Failed to attach PCI device %s to context - %s",
> +                   o->device, strerror(errno));
> +        return;
> +    }

It's unclear what happens when one of these error code paths is taken.
o->vfu_ctx and o->pci_dev might both be initialized, so how does the
code know not to service the vfio-user connection? It would be easy to
tell that this is correct if o->pci_dev and o->vfu_ctx were destroyed
when an error occurs.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]