Re: [PATCH v3 04/11] libxl: add generic function to add device

[Oleksandr Grytsov <al1img@gmail.com>]

On Thu, Jul 6, 2017 at 6:51 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Tue, Jun 27, 2017 at 01:03:20PM +0300, Oleksandr Grytsov wrote:
>> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>>
>> Add libxl__device_add functio.
>> Almost all devices have similar libxl__device_xxxx_add function.
>> This generic function implements same functionality but
>> using the device handling framework. The device specific
>> part this is setting xen store configuration. This part
>> is moved to set_xenstore_config callback of the device framework.
>>
>> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> [...]
>> +
>> +void libxl__device_add(libxl__egc *egc, uint32_t domid,
>> +                       const struct libxl_device_type *dt, void *type,
>> +                       libxl__ao_device *aodev)
>> +{
>> +    STATE_AO_GC(aodev->ao);
>> +    libxl__device *device;
>> +    int rc;
>> +
>> +    rc = dt->set_default(gc, domid, type);
>> +    if (rc) goto out;
>> +
>> +    GCNEW(device);
>> +    rc = dt->to_device(gc, domid, type, device);
>> +    if ( rc != 0 ) goto out;
>> +
>> +    rc = libxl__device_exists(gc, XBT_NULL, device);
>> +    if (rc < 0) goto out;
>> +    if (rc == 1) {              /* already exists in xenstore */
>> +        LOGD(ERROR, domid, "device already exists in xenstore");
>> +        aodev->action = LIBXL__DEVICE_ACTION_ADD; /* for error message */
>> +        rc = ERROR_DEVICE_EXISTS;
>> +        goto out;
>> +    }
>> +
>> +    if (aodev->update_json) {
>> +        rc = device_add_domain_config(gc, domid, dt, type);
>> +        if (rc) goto out;
>> +    }
>> +
>> +    if (dt->set_xenstore_config) {
>> +        rc = dt->set_xenstore_config(gc, domid, type);
>> +        if (rc) goto out;
>> +    }
>> +
>
> This has changed the locking hierarchy we define in libxl_internal.h.
> See libxl_internal.h:L2592.
>
> Either you need to preserve the hierarchy or you need to prove the
> correctness of the new approach. The former is probably easier.

Actually my the first patch probably was done on the old codebase
which doesn't have locking in add function. So new approach is
definitely wrong and I will use former one.

-- 
Best Regards,
Oleksandr Grytsov.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel