Re: [Xen-devel] [PATCH 0/2] libxl: add PV display device driver interface

["al1img ." <al1img@xxxxxxxxx>]

This example is clear. But still wrapper macro is required to make it
visible for libxen client (xl):

#define LIBXL_DEFINE_DEVICE_LIST_FREE(type)
    void libxl_device_##type##_list_free(libxl_device_##type* list, int nr)
    {
        libxl__device_list_free(libxl__##type##_devtype, list, nr);
    }

Also where should this function (libxl__device_list_free) be located?
libxl_device.c?

Another case is getting this list:

From one side each device should have its own implementation, from
other side for PV devices
there is common part like getting devId and backend domId and unique
part like getting
device specific parameters (uuid for vtpm). In this case I can do following:

Implement void *libxl__device_list(struct libxl_device_type *dt,
libxl_ctx *ctx, uint32_t domid, int *num)
where I will get devId and backend domId. Then add get_params callback
to libxl_device_type to get device specific
parameters:

void *libxl__device_pv_list(struct libxl_device_type *dt, libxl_ctx
*ctx, uint32_t domid, int *num)
{
    ...
    if (dt->get_patams) {
         dt->get_params(...);
    }
}

And vtpm get list may look like:

libxl_device_vtpm *libxl_device_vtpm_list(libxl_ctx *ctx, uint32_t
domid, int *num)
{
    return libxl__device_list(&libxl__vtpm_devtype, ctx, domid, num);
}

DEFINE_DEVICE_TYPE_STRUCT(vtpm,
    .update_config = libxl_device_vtpm_update_config
    .get_params = libxl_device_vtpm_get_params
);

Correct?


2017-03-23 14:08 GMT+02:00 Juergen Gross <jgross@xxxxxxxx>:
> On 23/03/17 12:32, al1img . wrote:
>> Hi Juergen,
>>
>> I've checked the mentioned commits. And I don't see how it can be done.
>> The duplication I see it is in libxl_device_type.add and
>> libxl_device_type.list functions
>> for different PV devices. These functions have a lot of common code
>> which I've tried
>> to move to macros in my patches.
>
> Okay, here an example for replacement of LIBXL_DEFINE_DEVICE_LIST_FREE()
>
> void libxl_device_list_free(struct libxl_device_type *dt,
>                             void *list, int nr)
> {
>     int i;
>
>     for (i = 0; i < nr; i++)
>         dt->dispose(list + i * dt->dev_elem_size);
>     free(list);
> }
>
> BTW: exactly this pattern is to be found at the very end of
> libxl_retrieve_domain_configuration().
>
> The others should be doable, too.
>
>
> Juergen
>
>>
>> 2017-03-23 12:21 GMT+02:00 Juergen Gross <jgross@xxxxxxxx>:
>>> On 23/03/17 11:10, Oleksandr Grytsov wrote:
>>>> From: Oleksandr Grytsov <oleksandr_grytsov@xxxxxxxx>
>>>>
>>>> Hi all,
>>>>
>>>> We are working on series of PV drivers (display, sound, input etc.) and
>>>> would like to add their support to libxl and xl. These patches add PV 
>>>> display
>>>> device. PV display is based on [1] protocol.
>>>>
>>>> During implementation I see a lot of code duplication. This problem was
>>>> mentioned in [2]. One of the solutions was to implement common parts in IDL
>>>> and make them autogenerated. On my side, to minimize the copy/pasting
>>>> I've moved common parts into macro functions: LIBXL_DEFINE_DEVICE_COMMIT,
>>>> LIBXL_DEFINE_DEVICE_LIST_GET, LIBXL_DEFINE_DEVICE_GETINFO etc.
>>>> Existing PV devices implementations can be reworked to use these macros as
>>>> well. Any other proposals to avoid the duplications are welcome.
>>>
>>> Did you look into the device type framework I introduced with commit
>>> 74e857c6c7f9 and some followups? Maybe it is possible to expand this
>>> framework by adding more callbacks to struct libxl_device_type and
>>> have some common function(s) in libxl_device.c?
>>>
>>> Juergen
>>
>>
>>
>



-- 
Best Regards,
Oleksandr Grytsov.

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