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

[Ian Campbell <ian.campbell@citrix.com>]

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@citrix.com>, Ian
> Campbell <ian.campbell@citrix.com> 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@relay2.provo.novell.com> 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?

> >  
> > > +    ]) 
> > > + 
> > > +libxl_device_usb = Struct("device_usb", [ 
> > > +    ("ctrl", integer), 
> >  
> > Is this an index into something? If so what? 
> 
> 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"?

> >  
> > There seems to be no usbctrl array added to the domain_config struct, so 
> > I'm unsure how this is used. 
> >  
> > > +    ("port", integer), 
> >  
> > Port on the hub? 
> > > +    ("intf", string), 
> >  
> > What is this one? (This may just be my lack of usb knowledge)
> 
> It means sysfs interface for the usb device under /sys/bus/usb/devices/,
> like: 2-1.6.

Thanks. I think given Georges feedback we will be dropping this?
> ntly not used.
> 
> > > + 
> > > @@ -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?