Re: [PATCH RFC server v2 03/11] vfio-user: instantiate vfio-user context

[Jag Raman]

> On Sep 8, 2021, at 8:40 AM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> 
> On Fri, Aug 27, 2021 at 01:53:22PM -0400, Jagannathan Raman wrote:
>> create a context with the vfio-user library to run a 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 | 29 +++++++++++++++++++++++++++++
>> 1 file changed, 29 insertions(+)
>> 
>> diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c
>> index 4a1e297..99d3dd1 100644
>> --- a/hw/remote/vfio-user-obj.c
>> +++ b/hw/remote/vfio-user-obj.c
>> @@ -27,11 +27,17 @@
>> #include "qemu/osdep.h"
>> #include "qemu-common.h"
>> 
>> +#include <errno.h>
> 
> qemu/osdep.h already includes <errno.h>
> 
>> +
>> #include "qom/object.h"
>> #include "qom/object_interfaces.h"
>> #include "qemu/error-report.h"
>> #include "trace.h"
>> #include "sysemu/runstate.h"
>> +#include "qemu/notify.h"
>> +#include "qapi/error.h"
>> +#include "sysemu/sysemu.h"
>> +#include "libvfio-user.h"
>> 
>> #define TYPE_VFU_OBJECT "vfio-user"
>> OBJECT_DECLARE_TYPE(VfuObject, VfuObjectClass, VFU_OBJECT)
>> @@ -51,6 +57,10 @@ struct VfuObject {
>> 
>>     char *socket;
>>     char *devid;
>> +
>> +    Notifier machine_done;
>> +
>> +    vfu_ctx_t *vfu_ctx;
>> };
>> 
>> static void vfu_object_set_socket(Object *obj, const char *str, Error **errp)
>> @@ -75,9 +85,23 @@ static void vfu_object_set_devid(Object *obj, const char 
>> *str, Error **errp)
>>     trace_vfu_prop("devid", str);
>> }
>> 
>> +static void vfu_object_machine_done(Notifier *notifier, void *data)
> 
> Please document the reason for using a machine init done notifier.

OK, will do.

> 
>> +{
>> +    VfuObject *o = container_of(notifier, VfuObject, machine_done);
>> +
>> +    o->vfu_ctx = vfu_create_ctx(VFU_TRANS_SOCK, o->socket, 0,
>> +                                o, VFU_DEV_TYPE_PCI);
>> +    if (o->vfu_ctx == NULL) {
>> +        error_setg(&error_abort, "vfu: Failed to create context - %s",
>> +                   strerror(errno));
>> +        return;
>> +    }
>> +}
>> +
>> static void vfu_object_init(Object *obj)
>> {
>>     VfuObjectClass *k = VFU_OBJECT_GET_CLASS(obj);
>> +    VfuObject *o = VFU_OBJECT(obj);
>> 
>>     if (!object_dynamic_cast(OBJECT(current_machine), TYPE_REMOTE_MACHINE)) {
>>         error_report("vfu: %s only compatible with %s machine",
>> @@ -92,6 +116,9 @@ static void vfu_object_init(Object *obj)
>>     }
>> 
>>     k->nr_devs++;
>> +
>> +    o->machine_done.notify = vfu_object_machine_done;
>> +    qemu_add_machine_init_done_notifier(&o->machine_done);
>> }
>> 
>> static void vfu_object_finalize(Object *obj)
>> @@ -101,6 +128,8 @@ static void vfu_object_finalize(Object *obj)
>> 
>>     k->nr_devs--;
>> 
>> +    vfu_destroy_ctx(o->vfu_ctx);
> 
> Will this function ever be called before vfu_object_machine_done() is
> called? In that case vfu_ctx isn't initialized.

There are some case where vfu_object_finalize() could be called before
vfu_object_machine_done() executes. In that case o->vfu_ctx would be
NULL - we didn’t account for that before.

vfu_destroy_ctx() does check for NULL - however, we’ll add a check
here as well in case vfu_destroy_ctx() changes in the future.

--
Jag