|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] libxl: Add missing libxl__virtio_devtype to device_type_tbl array
On 27.07.23 16:45, Anthony PERARD wrote: Hello Anthony > On Thu, Jul 27, 2023 at 10:38:03AM +0000, Oleksandr Tyshchenko wrote: >> >> >> 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)." > > Yes, that would be fine. With that addition: > Acked-by: Anthony PERARD <anthony.perard@xxxxxxxxxx> Thanks for the clarification and A-b. > > Thanks, >
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |