Re: [Qemu-devel] [Xen-devel] [PATCH v2] libxl: usb2 and usb3 controller support for upstream qemu

[Dario Faggioli]

On ven, 2013-07-12 at 10:43 +0200, Fabio Fantoni wrote:
> Il 11/07/2013 17:56, Dario Faggioli ha scritto:
> >> Signed-off-by: Fabio Fantoni <address@hidden>
> >>
> >> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> >> index d218a2d..b4c6921 100644
> >> --- a/tools/libxl/libxl_types.idl
> >> +++ b/tools/libxl/libxl_types.idl
> >> @@ -325,6 +325,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
> >>                                          ("serial",           string),
> >>                                          ("boot",             string),
> >>                                          ("usb",              
> >> libxl_defbool),
> >> +                                       ("usbversion",              
> >> integer),
> >>                                          # usbdevice:
> >>                                          # - "tablet" for absolute mouse,
> >>                                          # - "mouse" for PS/2 protocol 
> >> relative mouse
> >>
> > I believe this calls for a `#define LIBXL_HAVE_USBVERSION' (or something
> > like that) in libxl.h, doesn't it?
>
> Is it necessary even if I just want to apply it to xen 4.4 (as a new 
> features) and not backport it to older versions?
>
Although API stability is something I'm not sure I fully master, I would
say, yes it is necessary. :-)

> Or probably I don't understand exactly what is the function of LIBXL_HAVE_*.
>
The point is to allow people to write code suitable for _all_ versions
of libxl. So, imagine, in the super cool toolstack I'm writing on top of
libxl, I want to use your new parameter, if it's there, i.e., if
compiling on top of 4.4, but I also want the code to compile against
libxl 4.3.

With the LIBXL_HAVE_*, I can do something like this:

    ...
    binfo.usb = true;
    #ifdef LIBXL_HAVE_BUILDINFO_USBVERSION
    binfo.usbversion = 3;
    #endif
    ...

Does that make sense?

> I tried to grep all code for LIBXL_HAVE_* to see an example of use but I 
> found only one an I don't understand.
>
Try harder! :-)

http://xenbits.xen.org/gitweb/?p=xen.git&a=search&h=refs%2Fheads%2Fstaging&st=commit&s=LIBXL_HAVE_
http://xenbits.xen.org/gitweb/?p=xen.git&a=search&h=HEAD&st=grep&s=LIBXL_HAVE_

Actually, while at it, given LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST is
added in ac16730d0339d41fd7d1, I'd go for
LIBXL_HAVE_BUILDINFO_USBVERSION for yours, it looks more consistent.

Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

signature.asc
Description: This is a digitally signed message part