Re: [PATCH 06.1/10] xenconfig: add support for multiple USB devices syntax

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Marek Marczykowski-Górecki wrote:
> On Fri, Feb 20, 2015 at 04:10:47PM -0700, Jim Fehlig wrote:
>   
>> Marek Marczykowski-Górecki wrote:
>>     
>>> In Xen>=4.3, libxl supports new syntax for USB devices:
>>> usbdevice=[ "DEVICE", "DEVICE", ... ]
>>> Add support for that in xenconfig driver. When only one device is
>>> defined, keep using old syntax for backward compatibility.
>>>   
>>>       
>> Cool, thanks for doing this!  But I have a "small" nit...
>>
>>     
>>> Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
>>> ---
>>>  src/xenconfig/xen_common.c | 108 +++++++++++++++++++++++++++++++--------------
>>>   
>>>       
>> xm only supported the usbdevice=DEVICE syntax, but the code in
>> xen_common is used to parse/format both xm and xl config.  I think
>> parsing/formating of these devices should be moved to xen-xm and xen-xl.
>>     
>
> Also multiple devices are supported only in Xen >= 4.3. I haven't seen
> any constants for 4.x versions so haven't added new for that. Also names
> (XEND_CONFIG_VERSION_*) and its place (xenconfig/xen_sxpr.h) suggests it
> shouldn't be used for xl...
>   

For the xen-xl config format, I think it is fine to use the LIBXL_HAVE_*
pattern for detecting libxl features.  My suggestion was to move the
usbdevice formating/parsing from xen_common.c to xen_xm.c and xen_xl.c,
similar to disk formating/parsing.  The impl in xen_xm.c would provide
the existing formating/parsing functionality for xen-xm config.  The
impl in xen_xl.c would format/parse usbdevice as a list if
LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST.

Regards,
Jim

> Should I add a new version somewhere there anyway?
>
> I guess it applies also to other code there (which also doesn't bother
> with different libxl versions), so I guess I can simply ignore this
> issue.
>
>   

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]