Re: [PATCH v9 10/17] vfio-user: run vfio-user context

[Markus Armbruster]

Jagannathan Raman <jag.raman@oracle.com> writes:

> Setup a handler to run vfio-user context. The context is driven by
> messages to the file descriptor associated with it - get the fd for
> the context and hook up the handler with it
>
> 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>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  qapi/misc.json            |  30 +++++++++++
>  hw/remote/vfio-user-obj.c | 102 +++++++++++++++++++++++++++++++++++++-
>  2 files changed, 131 insertions(+), 1 deletion(-)
>
> diff --git a/qapi/misc.json b/qapi/misc.json
> index b83cc39029..fa49f2876a 100644
> --- a/qapi/misc.json
> +++ b/qapi/misc.json
> @@ -553,3 +553,33 @@
>  ##
>  { 'event': 'RTC_CHANGE',
>    'data': { 'offset': 'int', 'qom-path': 'str' } }
> +
> +##
> +# @VFU_CLIENT_HANGUP:
> +#
> +# Emitted when the client of a TYPE_VFIO_USER_SERVER closes the
> +# communication channel
> +#
> +# @vfu-id: ID of the TYPE_VFIO_USER_SERVER object
> +#
> +# @vfu-qom-path: path to the TYPE_VFIO_USER_SERVER object in the QOM tree
> +#
> +# @dev-id: ID of attached PCI device
> +#
> +# @dev-qom-path: path to attached PCI device in the QOM tree

I'm still unsure what kind(s) of ID @vfu-id and @dev-id are.  See below.

> +#
> +# Since: 7.1
> +#
> +# Example:
> +#
> +# <- { "event": "VFU_CLIENT_HANGUP",
> +#      "data": { "vfu-id": "vfu1",
> +#                "vfu-qom-path": "/objects/vfu1",
> +#                "dev-id": "sas1",
> +#                "dev-qom-path": "/machine/peripheral/sas1" },
> +#      "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
> +#
> +##
> +{ 'event': 'VFU_CLIENT_HANGUP',
> +  'data': { 'vfu-id': 'str', 'vfu-qom-path': 'str',
> +            'dev-id': 'str', 'dev-qom-path': 'str' } }
> diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c
> index 3ca6aa2b45..3a4c6a9fa0 100644
> --- a/hw/remote/vfio-user-obj.c
> +++ b/hw/remote/vfio-user-obj.c
> @@ -27,6 +27,9 @@
>   *
>   * device - id of a device on the server, a required option. PCI devices
>   *          alone are supported presently.
> + *
> + * notes - x-vfio-user-server could block IO and monitor during the
> + *         initialization phase.
>   */
>  
>  #include "qemu/osdep.h"
> @@ -40,11 +43,14 @@
>  #include "hw/remote/machine.h"
>  #include "qapi/error.h"
>  #include "qapi/qapi-visit-sockets.h"
> +#include "qapi/qapi-events-misc.h"
>  #include "qemu/notify.h"
> +#include "qemu/thread.h"
>  #include "sysemu/sysemu.h"
>  #include "libvfio-user.h"
>  #include "hw/qdev-core.h"
>  #include "hw/pci/pci.h"
> +#include "qemu/timer.h"
>  
>  #define TYPE_VFU_OBJECT "x-vfio-user-server"
>  OBJECT_DECLARE_TYPE(VfuObject, VfuObjectClass, VFU_OBJECT)
> @@ -86,6 +92,8 @@ struct VfuObject {
>      PCIDevice *pci_dev;
>  
>      Error *unplug_blocker;
> +
> +    int vfu_poll_fd;
>  };
>  
>  static void vfu_object_init_ctx(VfuObject *o, Error **errp);
> @@ -164,6 +172,76 @@ static void vfu_object_set_device(Object *obj, const 
> char *str, Error **errp)
>      vfu_object_init_ctx(o, errp);
>  }
>  
> +static void vfu_object_ctx_run(void *opaque)
> +{
> +    VfuObject *o = opaque;
> +    const char *vfu_id;
> +    char *vfu_path, *pci_dev_path;
> +    int ret = -1;
> +
> +    while (ret != 0) {
> +        ret = vfu_run_ctx(o->vfu_ctx);
> +        if (ret < 0) {
> +            if (errno == EINTR) {
> +                continue;
> +            } else if (errno == ENOTCONN) {
> +                vfu_id = object_get_canonical_path_component(OBJECT(o));
> +                vfu_path = object_get_canonical_path(OBJECT(o));

Hmm.  @vfu_id is always the last component of @vfu_path.  Why do we need
to send both?

> +                g_assert(o->pci_dev);
> +                pci_dev_path = object_get_canonical_path(OBJECT(o->pci_dev));
> +                qapi_event_send_vfu_client_hangup(vfu_id, vfu_path,
> +                                                  o->device, pci_dev_path);

Where is o->device set?  I'm asking because I it must not be null here,
and that's not locally obvious.

> +                qemu_set_fd_handler(o->vfu_poll_fd, NULL, NULL, NULL);
> +                o->vfu_poll_fd = -1;
> +                object_unparent(OBJECT(o));
> +                g_free(vfu_path);
> +                g_free(pci_dev_path);
> +                break;
> +            } else {
> +                VFU_OBJECT_ERROR(o, "vfu: Failed to run device %s - %s",
> +                                 o->device, strerror(errno));
> +                break;
> +            }
> +        }
> +    }
> +}

[...]