Re: [PATCH] libxl: Add missing libxl__virtio_devtype to device_type_tbl array

[Oleksandr Tyshchenko <Oleksandr_Tyshchenko@xxxxxxxx>]

On 27.07.23 12:50, Anthony PERARD wrote:

Hello Anthony

> On Wed, Jul 26, 2023 at 05:14:59PM +0300, Oleksandr Tyshchenko wrote:
>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
>>
>> Without it being present it won't be possible to use some
>> libxl__device_type's callbacks for virtio devices as the common code
>> can only invoke these callbacks (by dereferencing a pointer) for valid
>> libxl__device_type's elements when iterating over device_type_tbl[].
> 
> Did you notice an issue with it been missing from device_type_tbl[] ?
> Because to me it looks like all the functions that are using
> device_type_tbl will just skip over virtio devtype.
> 
> domcreate_attach_devices:
>      skip virtio because ".skip_attach = 1"
> 
> libxl__need_xenpv_qemu:
>      skip virtio because "dm_needed" is NULL
> 
> retrieve_domain_configuration_end:
>      skip because "compare" is "libxl_device_virtio_compare" which is NULL
> 
> libxl__update_domain_configuration:
>      skip because "update_config" is NULL.
> 
> So, I think the patch is fine, adding virtio to the device_type_tbl
> array is good for completeness, but the patch description may be
> misleading.
> 
> Did I miss something?

No, you didn't.

Just to be clear, there is no issue within *current* the code base, I am 
experimenting with using device-model bits, so I implemented 
libxl__device_virtio_dm_needed() locally and noticed that it didn't get 
called at all, the reason was in absence of libxl__virtio_devtype in the 
said array.

Do you agree with the following addition to the commit description?

"Please note, there is no issue within current the code base as virtio 
devices don't use callbacks that depend on libxl__virtio_devtype 
presence in device_type_tbl[]. The issue will appear as soon as we start
using these callbacks (for example, dm_needed)."




> 
> Thanks,
>