[Engine-devel] Supporting native USB in oVirt

[Eli Mesika]

----- Original Message -----
> From: "Dan Kenigsberg" <danken at redhat.com>
> To: "Oved Ourfalli" <ovedo at redhat.com>, "Dave Allan" <dallan at redhat.com>
> Cc: "Dave Allan" <dallan at redhat.com>, "Jiri Denemark" <jdenemar at redhat.com>, "Michal Privoznik"
> <mprivozn at redhat.com>, "Itamar Heim" <iheim at redhat.com>, "Igor Lvovsky" <ilvovsky at redhat.com>, "Eli Mesika"
> <emesika at redhat.com>, "Hans de Goede" <hdegoede at redhat.com>, "Andrew Cathrow" <acathrow at redhat.com>
> Sent: Tuesday, May 8, 2012 11:41:25 AM
> Subject: Re: Supporting native USB in oVirt
> 
> On Tue, May 08, 2012 at 03:19:32AM -0400, Oved Ourfalli wrote:
> > Hi,
> > 
> > We are now working on adding the support for native USB devices on
> > oVirt.
> > As far as we understand, in order to use it we need to pass the
> > following devices with the following restrictions (according to
> > the EHCI spec):
> > 1. EHCI USB controller - with the highest function number, #7.
> > 
> > devices='{type:controller,device:usb,model:ich9-ehci1,address:{type:pci,domain:0x0000,bus:0x00,slot:0x04,function:0x7}}'
> > 
> > 2. 3 UHCI USB controllers, on the same bus and PCI slot as the EHCI
> > controller. Set the multifunction bit to on, on the controller
> > with function #0.
> > 
> > devices='{type:controller,device:usb,model:ich9-uhci1,master:{startport:0},address:{type:pci,domain:0x0000,bus:0x00,slot:0x04,function:0x0,multifunction:on}}'
> > devices='{type:controller,device:usb,model:ich9-uhci2,master:{startport:2},address:{type:pci,domain:0x0000,bus:0x00,slot:0x04,function:0x1}}'
> > devices='{type:controller,device:usb,model:ich9-uhci3,master:{startport:4},address:{type:pci,domain:0x0000,bus:0x00,slot:0x04,function:0x2}}'
> 
> I'd like to make it clear that the suggested API request Engine to
> pass
> a "master" attribute for the controller device, with a nested
> dictionary
> within. This makes sense, as I am certain that libvirt's modeling of
> the
> device has merits. However, AFAIK Engine does not do stuff like that
> at
> the momemnt.

And should not do in future IMHO, this should be transparent to the engine core that should know only the USB device.

> 
> > 
> > 3. USB redirect devices (according to the needed number of USB
> > slots, maximum 6) on the same bus, each one having a different
> > port.
> > 
> > devices='{type:redir,device:spicevmc,bus:usb,address:{type:usb,bus:0,port:1}}'
> > devices='{type:redir,device:spicevmc,bus:usb,address:{type:usb,bus:0,port:2}}'
> > devices='{type:redir,device:spicevmc,bus:usb,address:{type:usb,bus:0,port:3}}'
> > devices='{type:redir,device:spicevmc,bus:usb,address:{type:usb,bus:0,port:4}}'
> > devices='{type:redir,device:spicevmc,bus:usb,address:{type:usb,bus:0,port:5}}'
> > devices='{type:redir,device:spicevmc,bus:usb,address:{type:usb,bus:0,port:6}}'
> > 
> > 4. If we want more than 6 USB slots, we need to have 2 EHCI
> > controllers, and 6 UHCI controllers, that are consistent with the
> > restrictions above, on different bus.
> > (we need them to be on different bus, since the connection between
> > the redirect devices and the controllers is the bus).
> > 
> > According to Hans (cc-ed), if we let libvirt pick its own
> > addresses, it will put each controller of the composite USB
> > controller device on its own pci slot, which is a violation of the
> > EHCI spec.
> 
> Is there an open libvirt bug for this?
> 
> > 
> > The problem is that the oVirt engine is not aware of addresses.
> > They aren't managed by it.
> > They are chosen automatically by libvirt, returned to the engine,
> > and they saved in the engine database in order to provide the
> > ability to retain the same PCI addresses after VM restart.
> 
> Yes, we should be able to trust libvirt's assignement, certainly for
> the
> masses of VMs. Few VMs of certain customer may want to tweak the
> location of their devices. Maybe Engine does not need to expose this
> feature at the moment (I think it is better that each device address
> is
> considered a "blob" by Engine), but the Engine/Vdsm API should
> support
> that.

I know that in future we may need to tweak the device location.
But still, the general principle of address allocation should be kept simple:
If you send an address, this address is honored by libvirt
If you don't, you will get a valid one from libvirt.
I think that engine should not handle any addresses apart from persisting the address in DB.

> 
> > 
> > So, in order to support the EHCI spec, oVirt will have to be aware
> > of addresses, manage them (allocate them, check for collisions,
> > update on every libvirt change regarding that, etc...). Obviously,
> > it doesn't feel right, and in fact it is also not feasible, to
> > manage these addresses in the oVirt domain.
> 
> That's not obvious to me. Engine could have a hard-coded addresses
> for
> the usb device gang. Libvirt could arrange the rest of the devices
> around it, so Engine should not worry about collisions. Libvirt's
> stable
> API relieves Engine from worrying about "every libvirt change
> regarding
> that" - it is libvirt responsibility to make sure that every future
> libvirt version would handle the USB addresses properly.

A big hole for future bugs that will fall between libvirt & engine responsibility.
I don't like that idea of spreading responsibilities between separate components 
and prefer that addresses will have one manager that is obviously libvirt.

> 
> > 
> > Is all the above correct, or are we missing something?
> > If so, can you address the issues above, and provide the ability to
> > manage these devices in libvirt?
> > 
> > Regards,
> > Oved
>