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

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

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@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?
> 
> 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.

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?

> >>>> + 
> >>>> @@ -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.