Re: [Xen-devel] [PATCH 1 of 2] libxl: Allow multiple USB devices on HVM domain creation

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2012-11-29 at 12:01 +0000, George Dunlap wrote:
> On 29/11/12 11:16, Ian Campbell wrote:
> > On Wed, 2012-11-28 at 12:16 +0000, George Dunlap wrote:
> >> # HG changeset patch
> >> # User George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> >> # Date 1354101445 0
> >> # Node ID 538d9ffbd71b41e8cf6d7da0ded9e0a0b07f3c0d
> >> # Parent  ae6fb202b233af815466055d9f1a635802a50855
> >> libxl: Allow multiple USB devices on HVM domain creation
> >>
> >> This patch allows an HVM domain to be created with multiple USB
> >> devices.
> >>
> >> Since the previous interface only allowed the passing of a single
> >> device, this requires us to add a new element to the hvm struct of
> >> libxl_domain_build_info -- usbdevice_list.  For API compatibility, the
> >> old element, usbdevice, remains.
> >>
> >> If b_info->hvm.usbdevice is non-NULL, then it will be used exclusively;
> >> otherwise, libxl will check for b_info->hvm.usbdevice_list instead.
> > Is this the right way round? If the caller knows about usbdevice_list
> > enough to have set it to something then surely that's the one it wanted?
> 
>   Well, I had considered returning an error if both were set. :-)  I 
> suppose actually that's probably the way that is likely to flag up bugs 
> in the toolstack the best -- only doing one or the other will cause 
> weird buggy behavior that will be hard to track down.  Does that sound 
> good to you?
> 
> >> Each device listed will cause an extra "-usbdevice [foo]" to be appended
> >> to the qemu command line.
> >>
> >> In order to allow users of libxl to write software compatible with older
> >> versions of libxl, also define LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST.  If
> >> this is present, the caller should use b_info->hvm.usbdevice_list; 
> >> otherwise
> >> they should use b_info->hvm.usbdevice.
> > Actually it's a both stricter and looser than that, if
> > LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST is defined then the caller can
> > choose to use usbdevice_list but can also use usbdevice if they wish
> > (but not both). If it is not present then they must not use
> > usbdevice_list at all.
> 
> Hmm, I think I wrote this first and then changed it and forgot to update 
> it.  :-)  I also forgot that in this kind of "API spec" document, 
> "should" and "must" are technical terms with very specific definitions.  
> I'll revise it when I re-spin.

I don't think we actually use the RFC-whichever definitions of should
and must in general (and they would be SHOULD and MUST anyhow), it's
probably me being picky.

> > I wonder if this LIBXL_HAVE should also be contained in a #ifdef
> > LIBXL_API_VERSION >= 0x040300?
> 
> What would be the purpose of that?

LIBXL_API_VERSION and LIBXL_HAVE_* are two different mechanisms for
helping us to do API compatibility, I hadn't properly thought about the
interaction between them until now.

Adding the ifdef would expose the new feature only to those claiming
compatibility with the newer API.

More generally the purpose if LIBXL_API_VERSION generally is to allow us
to deprecate interfaces over time, which isn't quite the case here I
suppose.

Ian.


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