Re: [Xen-devel] [PATCH RFC V2 1/5] libxl: add pvusb definitions

["Chun Yan Liu" <cyliu@xxxxxxxx>]

>>> On 3/4/2015 at 08:33 PM, in message <1425472438.25940.147.camel@xxxxxxxxxx>,
Ian Campbell <ian.campbell@xxxxxxxxxx> wrote: 
> On Wed, 2015-03-04 at 12:26 +0000, George Dunlap wrote: 
> > On 03/04/2015 10:00 AM, Ian Campbell wrote: 
> > > On Wed, 2015-03-04 at 00:26 -0700, Chun Yan Liu wrote: 
> > >> 
> > >>>>> On 3/3/2015 at 07:10 PM, in message 
> > >>>>> <1425381019.24959.87.camel@xxxxxxxxxx>,  
> Ian 
> > >> Campbell <ian.campbell@xxxxxxxxxx> wrote:  
> > >>> On Mon, 2015-01-19 at 16:28 +0800, Chunyan Liu wrote:  
> > >>>   
> > >>> Sorry for the long delay in replying.  
> > >>>   
> > >>>> To attach a usb device, a virtual usb controller should be created 
> > >>>> first.  
> > >>>> This patch defines usbctrl and usbdevice related structs.  
> > >>>   
> > >>> Per <54CA17DF0200006600095E3D@xxxxxxxxxxxxxxxxxxxxxxx> please could you 
> > >>>  
> > >>> mention here that the HVM guest related parts (i.e.  
> > >>> LIBXL_USBCTRL_TYPE_DEVICEMODEL) and libxl_usb_type are placeholders for 
> > >>>  
> > >>> emulated HVM support.  
> > >> 
> > >> Yes, I agree it's better placed in libxl_usb_type rather than ctrl_type. 
> > >> 
> > >>>   
> > >>> In fact I wonder if it should just be omitted, we will need a 
> > >>> LIBXL_HAVE  
> > >>> for HVM USB support anyway once it is implemented so we can add the 
> > >>> enum  
> > >>> then.  
> > >> 
> > >> It won't harm to omit it for current pvusb work. Acceptable to me to 
> > >> add enum later when adding HVM qemu emulated usb device implementation. 
> > >  
> > > I suppose users of libxl would like to be able to expose to their users 
> > > whether or not HVM USB passthrough will work (i.e. to hide UI options). 
> > > So I think we will want the #define eventually so they can know at 
> > > compile time if HVM USB will work. 
> > >  
> > > We could add a negative one now (LIBXC_NO_HVM_USB_PASSTHROUGH) and 
> > > remove it later, but that's icky I think. 
> > >  
> > > So I think omit the HVM stuff for now, it's less confusing overall that 
> > > way. 
> > >  
> > > George, is that OK with you? 
> >  
> > Yes; particularly as I'm hoping that having the PVUSB stuff in will make 
> > it easier for me to add my HVM usb hot-plug stuff before the feature 
> > freeze. :-) 
>  
> Great. 
>  
> >  
> > >> To usb controller index. 
> > >> A usb device should be connected to a usb port of a usb controller. 
> > >> e.g.: there is 2 usb controllers in system, each with 8 ports, then: 
> > >> 1st usb controller index will be 0, port will be 1~8. 
> > >> 2nd usb controller index will be 1, port will be 1~8. 
> > >> To attach a usb device through pvusb way, it should be pointed to 
> > >> connect to which controller and which port. 
> > >  
> > > I guess what I'm missing is how do I create this controller? I saw 
> > > nothing in the guest cfg which would allow me to create one. 
> > >  
> > > Is there some way to say "I don't care, find a controller and use it"? 
> >  
> > This isn't documented, but if you set "ctrl" to -1, the code as written 
> > will automatically: 
> >  * find an empty port on a controller, if there is one 
> >  * create a controller if there isn't one. 
> >  
> > I meant to mention this in my mail yesterday though -- I think probably 
> > there should be a defined constant in the IDL (LIBXL_USBCTRL_AUTO or 
> > something) you should use for that, rather than just remembering a magic 
> > value. 
>  
> Yes, and it should be the init_val in the idl I think so that the 
> default is to do something useful after _init is called. 

Got it. Will update.

>  
> Can we arrange for the default/auto value to be 0, or is that too 
> confusing because it is expected that controllers will be zero based? 

Yeah, controller index is zero based, so it might be confusing if setting
default/auto to be 0.

- Chunyan

>  
> > >>>> +  
> > >>>> @@ -547,6 +578,7 @@ libxl_domain_config = Struct("domain_config", [  
> > >>>>      ("disks", Array(libxl_device_disk, "num_disks")),  
> > >>>>      ("nics", Array(libxl_device_nic, "num_nics")),  
> > >>>>      ("pcidevs", Array(libxl_device_pci, "num_pcidevs")),  
> > >>>> +    ("usbs", Array(libxl_device_usb, "num_usbs")),  
> > >>>   
> > >>> So, I'm unsure how this interacts with the controllers, which it 
> > >>> doesn't  
> > >>> seem to be possible to specify at domain build time. 
> > >> 
> > >> In domain config, user only needs to specify usb=['2-1.6'], by default, 
> > >> it  
> will 
> > >> create a default usb contoller, and probe the 1st available  
> controller:port for 
> > >> the usb device to attach. So, it can work to specify usbs here only. 
> > >> 
> > >> Reason didn't include controller in libxl_domain_config: for HVM qemu  
> emulated 
> > >> usb device, all work is done in qemu (create usb controller and attach 
> > >> usb  
> device), 
> > >> no controller exists in libxl in that case. 
> > >  
> > > OK, so it's an HVM only thing. I think that makes sense, but then how 
> > > does the libxl_device_usb.ctrl field make sense or how do I use it? 
> >  
> > Well for one, you can use libxl_device_usbctrl_add() to make a new one 
> > on a running VM; then you can use libxl_device_usb_add() to attach it. 
> > (These are exposed in xl as usb-ctrl-attach and usb-attach.) 
>  
> I was thinking in the context of the domain_config struct above, so 
> runtime xl commands other than create aren't usable. 
>  
> Ian. 
>  
>  
>  


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