From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v7 RFC 1/2] libxl: Introduce functions to add and remove USB devices to an HVM guest Date: Wed, 18 Jun 2014 15:30:39 +0100 Message-ID: <1403101839.6568.53.camel@kazak.uk.xensource.com> References: <1401716658-22393-1-git-send-email-george.dunlap@eu.citrix.com> <1401716658-22393-2-git-send-email-george.dunlap@eu.citrix.com> <1403096886.32540.22.camel@kazak.uk.xensource.com> <53A19284.5070505@eu.citrix.com> <1403099364.6568.22.camel@kazak.uk.xensource.com> <53A19AF4.1040502@eu.citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <53A19AF4.1040502@eu.citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: George Dunlap Cc: xen-devel@lists.xen.org, sstanisi@cbnco.com, Fabio Fantoni , Anthony Perard , Ian Jackson , "Simon (Bo) Cao" , Roger Pau Monne List-Id: xen-devel@lists.xenproject.org On Wed, 2014-06-18 at 14:58 +0100, George Dunlap wrote: > On 06/18/2014 02:49 PM, Ian Campbell wrote: > >>>> + ("backend_domid", libxl_domid), > >>>> + ("backend_domname", string), > >>>> + ("u", KeyedUnion(None, libxl_device_usb_type, "type", > >>>> + [("hostdev", Struct(None, [ > >>>> + ("hostbus", integer), > >>>> + ("hostaddr", integer) ])) > >>> No need to express the host topology I think (because you can build that > >>> from the bus,addr tuples)? > >> I don't really follow. You mean, we can drop 'host' from the last two > >> elements, and just call them "bus" and "addr"? > > Gah, I started writing one thing and then reunderstood usb and wrote > > half another. > > > > What I was trying to say is that you don't need hostaddr to describe the > > full USB topology path to the device because the (bus,addr) tuple you've > > given already does so (because each hub effectively creates a new bus > > number, so they all look like toplevel buses in this representation). > > You seem to be saying that something is redundant, or that there's extra > information somewhere; but as there are only two bits of data (bus and > addr), and agree that I need both, I'm having a hard time telling what > you think is redundant / could be removed.. I'm actually saying the opposite (or rather asking for confirmation that this is the case). i.e. one might expect that *more* information would be needed here (i.e. the full bus topology/path to a device via multiple hubs etc) but in fact this is not needed because the topology is not visible in this numbering scheme (each potential fork in the path results in a whole new bus number). Sorry, I could have been clearer about this. > "hostdev" is an element of the union; so the structure should unpack > like this: > struct { > libxl_domid backend_domid; > char * backend_domname; > libxl_usb_device_type type; > union u { > struct { > int hostbus, hostaddr; > } hostdev; > }; > }; > > At the moment, "type" can only be "hostdev"; but I'm envisioning in the > future that "type" might be "tablet", "mouse", "keyboard", maybe > "mass-storage", and that the union would have more elements. This seems sensible. You brought up dropping the hostdev prefix from the bus and addr (because you thought I was ;-)), which seemed sensible when you think of the code which accesses these fields i.e. dev->u.hostdev.hostbus vs dev->u.hostdev.bus. Up to you whether you want to repeat "host" in that. > Does that clear things up? Or am I totally confused? :-) I think you are remarkably unconfused given how confusing I'm being... Ian.