Bug 1538637 - Enable USB controller by default on AArch64
Enable USB controller by default on AArch64
Status: CLOSED NOTABUG
Product: Virtualization Tools
Classification: Community
Component: libvirt (Show other bugs)
unspecified
aarch64 Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Andrea Bolognani
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2018-01-25 08:54 EST by Marcin Juszkiewicz
Modified: 2018-02-08 08:27 EST (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2018-02-08 08:27:32 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
patch to get AArch64 defaults saner (611 bytes, text/plain)
2018-01-25 08:54 EST, Marcin Juszkiewicz
no flags Details
failed domain xml (2.27 KB, text/plain)
2018-01-25 08:54 EST, Marcin Juszkiewicz
no flags Details


External Trackers
Tracker ID Priority Status Summary Last Updated
Launchpad 1745340 None None None 2018-01-25 08:54 EST

  None (edit)
Description Marcin Juszkiewicz 2018-01-25 08:54:03 EST
Created attachment 1386090 [details]
patch to get AArch64 defaults saner

Description of problem:

We live in a world where 99% of VM instances are x86 ones. And most of tools just assumes that when  you run VM then it's config is like it is on x86 (for example Nova does that).

And they fail on USB host.

Version-Release number of selected component (if applicable):

3.10.0

How reproducible:

always

Steps to Reproduce:
1. deploy OpenStack on aarch64 with some graphical console enabled
2. run a VM instance
3. watch it fail

Actual results:

libvirtError: internal error: No free USB ports

Expected results:

VM instance runs

Additional info:

Changing Nova code is one solution. Changing libvirt defaults is other - easier one and makes aarch64 closer to x86 defaults which are expected by different tools.
Comment 1 Marcin Juszkiewicz 2018-01-25 08:54 EST
Created attachment 1386091 [details]
failed domain xml
Comment 2 Andrea Bolognani 2018-01-25 11:47:06 EST
Patch posted upstream.

  https://www.redhat.com/archives/libvir-list/2018-January/msg00929.html
Comment 3 Marcin Juszkiewicz 2018-01-31 11:29:03 EST
https://review.openstack.org/#/c/538003/ solves the problem on Nova side.
Comment 4 Andrea Bolognani 2018-02-01 04:03:38 EST
(In reply to Marcin Juszkiewicz from comment #3)
> https://review.openstack.org/#/c/538003/ solves the problem on Nova side.

I'm going to comment here because I don't have an account on the
OpenStack and I don't feel like creating one :)

In the latest update, you say

> But then there are two ways and I do not know which one to choose:
> 
> 1. Just add usb host controller. On x86 they are present already
>    so one more should not do any harm.
> 
> 2. Check is pointer device usb one or not and then decide to add
>    usb host controller or not.

I would go with 1), especially if the libvirt patch I mentioned in
Comment 2 will be accepted, because you would then basically get
the same behavior even with older libvirt releases. Even if it's
not accepted, it makes sense to have a uniform guest configuration
across architecture at the Nova level.

On a side note, x86 guests would still get only a single USB
controller: the default USB controller is not added if one is
already present in the guest.
Comment 5 Marcin Juszkiewicz 2018-02-01 06:40:23 EST
And I went that way. If !x86 then add usb host controller.
Comment 6 Andrea Bolognani 2018-02-01 09:00:02 EST
(In reply to Marcin Juszkiewicz from comment #5)
> And I went that way. If !x86 then add usb host controller.

I noticed this exchange in

  https://review.openstack.org/#/c/538003/7/nova/virt/libvirt/driver.py@5039

> Stephen Finucane
> Is there any reason we need to limit this to non-x86 architectures?
> What happens if you add it for x86? Does it override the default,
> or does it add a second controller?
> 
> Marcin Juszkiewicz
> it would just add another one

Have you verified this is actually the case? Because as I mentioned
in Comment 4 I believe a user-provided USB controller being present
would prevent libvirt from adding the default one, and so the
architecture check shouldn't be needed.
Comment 7 Marcin Juszkiewicz 2018-02-01 09:15:35 EST
(Venv) 15:14 (s) root@puchatek:diskimage-builder# echo '<controller type="usb" />' >usb.xml
(Venv) 15:14 (s) root@puchatek:diskimage-builder# virsh dumpxml debian9|grep controller|grep hci
    <controller type='usb' index='0' model='ich9-ehci1'>
    <controller type='usb' index='0' model='ich9-uhci1'>
    <controller type='usb' index='0' model='ich9-uhci2'>
    <controller type='usb' index='0' model='ich9-uhci3'>
(Venv) 15:14 (s) root@puchatek:diskimage-builder# virsh attach-device debian9 usb.xml --config
Pomyślnie podłączono urządzenie

(Venv) 15:14 (s) root@puchatek:diskimage-builder# virsh dumpxml debian9|grep controller|grep hci
    <controller type='usb' index='0' model='ich9-ehci1'>
    <controller type='usb' index='0' model='ich9-uhci1'>
    <controller type='usb' index='0' model='ich9-uhci2'>
    <controller type='usb' index='0' model='ich9-uhci3'>
    <controller type='usb' index='1' model='piix3-uhci'>
(Venv) 15:14 (s) root@puchatek:diskimage-builder#
Comment 8 Andrea Bolognani 2018-02-01 09:49:41 EST
(In reply to Marcin Juszkiewicz from comment #7)
> (Venv) 15:14 (s) root@puchatek:diskimage-builder# echo '<controller
> type="usb" />' >usb.xml
> (Venv) 15:14 (s) root@puchatek:diskimage-builder# virsh dumpxml debian9|grep
> controller|grep hci
>     <controller type='usb' index='0' model='ich9-ehci1'>
>     <controller type='usb' index='0' model='ich9-uhci1'>
>     <controller type='usb' index='0' model='ich9-uhci2'>
>     <controller type='usb' index='0' model='ich9-uhci3'>
> (Venv) 15:14 (s) root@puchatek:diskimage-builder# virsh attach-device
> debian9 usb.xml --config
> Pomyślnie podłączono urządzenie
> 
> (Venv) 15:14 (s) root@puchatek:diskimage-builder# virsh dumpxml debian9|grep
> controller|grep hci
>     <controller type='usb' index='0' model='ich9-ehci1'>
>     <controller type='usb' index='0' model='ich9-uhci1'>
>     <controller type='usb' index='0' model='ich9-uhci2'>
>     <controller type='usb' index='0' model='ich9-uhci3'>
>     <controller type='usb' index='1' model='piix3-uhci'>
> (Venv) 15:14 (s) root@puchatek:diskimage-builder#

But here you're adding a USB controller *on top* of another, existing
USB controller! Of course libvirt wouldn't remove the existing one :)

I was thinking more along the lines of defining a guest with

  <devices>
    ...
    <controller type='usb'/>
    ...
  </device>

as opposed to one with

  <devices>
    ...
    <!-- No USB controllers at all -->
    ...
  </devices>

In both cases you would end up with a single USB controller in
the guest.

Then again, Nova is probably not using the default USB controller
for x86 guests - nor should it, since that's USB1. I probably just
misunderstood the context of your changes.
Comment 9 Marcin Juszkiewicz 2018-02-01 10:03:02 EST
My change has to do one simple thing: add usb host controller on non-x86 architectures. 

X86 has one, other archs (like aarch64) do not. So make sure that there is one. If there was already one then fine - it will have one more. But at least one will be present.

Checking is there a usb host controller in VM or not is possible but imho waste of time - show me guest OS which is unable to handle more than one usb host controller.
Comment 10 Andrea Bolognani 2018-02-01 10:27:28 EST
(In reply to Marcin Juszkiewicz from comment #9)
> My change has to do one simple thing: add usb host controller on non-x86
> architectures. 
> 
> X86 has one, other archs (like aarch64) do not.

Correction: both x86 and ppc64 guest get a USB controller by default;
aarch64 doesn't, and s390x *shouldn't*: USB is not supported on that
architecture, and any such controller will be ignored if present.

I expected there to be some part of Nova that would create an
explicit USB2 controller, but I couldn't find any reference to ich9
or EHCI with a quick 'git grep'. Do x86 guest in OpenStack use the
USB1 (piix3-uhci) controller by default?

> So make sure that there is
> one. If there was already one then fine - it will have one more. But at
> least one will be present.
> 
> Checking is there a usb host controller in VM or not is possible but imho
> waste of time - show me guest OS which is unable to handle more than one usb
> host controller.

I guess that would confuse the user more than it would the OS :)
Comment 11 Daniel Berrange 2018-02-01 10:31:37 EST
(In reply to Andrea Bolognani from comment #10)
> (In reply to Marcin Juszkiewicz from comment #9)
> > My change has to do one simple thing: add usb host controller on non-x86
> > architectures. 
> > 
> > X86 has one, other archs (like aarch64) do not.
> 
> Correction: both x86 and ppc64 guest get a USB controller by default;
> aarch64 doesn't, and s390x *shouldn't*: USB is not supported on that
> architecture, and any such controller will be ignored if present.
> 
> I expected there to be some part of Nova that would create an
> explicit USB2 controller, but I couldn't find any reference to ich9
> or EHCI with a quick 'git grep'. Do x86 guest in OpenStack use the
> USB1 (piix3-uhci) controller by default?

Yes, that is precisely the bug in Nova - its not doing anything with USB controllers right now, even on x86, so gets the USB1 default.
Comment 12 Marcin Juszkiewicz 2018-02-01 10:47:32 EST
Nova gets more complicated anyway. I just realized that once I have graphical console I need USB keyboard as well. 

Will rewrite patch as I now have to check do I have a keyboard already and add one if not. And once at it I will look at usb host controller situation again.
Comment 13 Andrea Bolognani 2018-02-01 10:54:04 EST
(In reply to Daniel Berrange from comment #11)
> > Correction: both x86 and ppc64 guest get a USB controller by default;
> > aarch64 doesn't, and s390x *shouldn't*: USB is not supported on that
> > architecture, and any such controller will be ignored if present.
> > 
> > I expected there to be some part of Nova that would create an
> > explicit USB2 controller, but I couldn't find any reference to ich9
> > or EHCI with a quick 'git grep'. Do x86 guest in OpenStack use the
> > USB1 (piix3-uhci) controller by default?
> 
> Yes, that is precisely the bug in Nova - its not doing anything with USB
> controllers right now, even on x86, so gets the USB1 default.

So wouldn't the right immediate fix explicitly adding the USB
controller everywhere, even x86 (but not s390x)?

Later on it can potentially be improved upon by explicitly using
USB2 on x86/i440fx - all other arches/machines already default
to USB3.
Comment 14 Laine Stump 2018-02-01 12:09:22 EST
Are you suggesting the USB2 controller on 440fx to support "really old" guest OSes that don't have a USB3 driver? Otherwise there isn't any reason to prefer a USB2 controller on i440fx, and it could even have a negative impact on CPU usage (Gerd? Is that just USB1, or is USB2 also problematic?)
Comment 15 Gerd Hoffmann 2018-02-01 14:12:01 EST
(In reply to Laine Stump from comment #14)
> Are you suggesting the USB2 controller on 440fx to support "really old"
> guest OSes that don't have a USB3 driver?

That (guest without xhci driver) is pretty much the only good reason to not use usb3.

> (Gerd? Is that just USB1, or is USB2 also problematic?)

xhci is the best choice, the hardware design is much more virtualization-friendly.  usb1 and usb2 both burn lots of cpu cycles for the emulation due to the way the hardware <=> os interface is designed.

usb2 (aka ehci) code has a few more tricks learned to reduce the cpu load, so it is slightly better than usb1, but still not good.
Comment 16 Marcin Juszkiewicz 2018-02-08 04:51:29 EST
Nova change got merged so on aarch64 it adds usb host controller and usb keyboard for all guests with graphical console.
Comment 17 Andrea Bolognani 2018-02-08 08:27:32 EST
Cool! Based on the upstream discussion, closing as NOTABUG.

Note You need to log in before you can comment on or make changes to this bug.