[libvirt] [PATCH] Set a sensible default master start port for ehci companion controllers

From: "Daniel P. Berrange" <berrange@redhat.com> The uhci1, uhci2, uhci3 companion controllers for ehci1 must have a master start port set. Since this value is predictable we should set it automatically if the app does not supply it --- src/conf/domain_conf.c | 22 ++++++++++++++++++ .../qemuxml2argv-usb-ich9-ehci-addr.xml | 24 +++++--------------- 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 54ac1db..f4775be 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4066,6 +4066,28 @@ virDomainControllerDefParseXML(xmlNodePtr node, VIR_FREE(vectors); break; } + case VIR_DOMAIN_CONTROLLER_TYPE_USB: { + /* If the XML has a uhci1, uhci2, uhci3 controller and no + * master port was given, we should set a sensible one */ + int masterPort = -1; + switch (def->model) { + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI1: + masterPort = 0; + break; + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI2: + masterPort = 2; + break; + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI3: + masterPort = 4; + break; + } + if (masterPort != -1 && + def->info.mastertype == VIR_DOMAIN_CONTROLLER_MASTER_NONE) { + def->info.mastertype = VIR_DOMAIN_CONTROLLER_MASTER_USB; + def->info.master.usb.startport = masterPort; + } + break; + } default: break; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.xml index 8eff1d7..ad85d63 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.xml @@ -19,19 +19,13 @@ <controller type='usb' index='1' model='ich9-ehci1'> </controller> - <controller type='usb' index='0' model='ich9-uhci1'> - <master startport='0'/> - </controller> - <controller type='usb' index='1' model='ich9-uhci1'> - <master startport='0'/> - </controller> + <controller type='usb' index='0' model='ich9-uhci1'/> + <controller type='usb' index='1' model='ich9-uhci1'/> <controller type='usb' index='2' model='ich9-uhci1'> <master startport='0'/> </controller> - <controller type='usb' index='0' model='ich9-uhci3'> - <master startport='4'/> - </controller> + <controller type='usb' index='0' model='ich9-uhci3'/> <controller type='usb' index='1' model='ich9-uhci3'> <master startport='4'/> </controller> @@ -39,15 +33,9 @@ <master startport='4'/> </controller> - <controller type='usb' index='2' model='ich9-uhci2'> - <master startport='2'/> - </controller> - <controller type='usb' index='1' model='ich9-uhci2'> - <master startport='2'/> - </controller> - <controller type='usb' index='0' model='ich9-uhci2'> - <master startport='2'/> - </controller> + <controller type='usb' index='2' model='ich9-uhci2'/> + <controller type='usb' index='1' model='ich9-uhci2'/> + <controller type='usb' index='0' model='ich9-uhci2'/> <memballoon model='virtio'/> </devices> </domain> -- 1.7.10.1

On 05/14/2012 06:24 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The uhci1, uhci2, uhci3 companion controllers for ehci1 must have a master start port set. Since this value is predictable we should set it automatically if the app does not supply it --- src/conf/domain_conf.c | 22 ++++++++++++++++++ .../qemuxml2argv-usb-ich9-ehci-addr.xml | 24 +++++--------------- 2 files changed, 28 insertions(+), 18 deletions(-)
ACK. But shouldn't we also have a test in qemuxml2xmltest, by using a file in qemuxml2xmloutdata/, that shows that we generate the correct start port on output in response to a user omitting information that we can determine by default? -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Mon, May 14, 2012 at 03:47:57PM -0600, Eric Blake wrote:
On 05/14/2012 06:24 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The uhci1, uhci2, uhci3 companion controllers for ehci1 must have a master start port set. Since this value is predictable we should set it automatically if the app does not supply it --- src/conf/domain_conf.c | 22 ++++++++++++++++++ .../qemuxml2argv-usb-ich9-ehci-addr.xml | 24 +++++--------------- 2 files changed, 28 insertions(+), 18 deletions(-)
ACK. But shouldn't we also have a test in qemuxml2xmltest, by using a file in qemuxml2xmloutdata/, that shows that we generate the correct start port on output in response to a user omitting information that we can determine by default?
Now I added a test case for the previous USB patch it automatically also tested the scenario you describe here. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Hey, Did some more testing. Looks like qemu is sensitive to the order in which the controllers are passed. Looks like they should be passed in a specific order (unless there is something else which is wrong here). Tried with two VMs, one in which the order was: <controller index="0" model="ich9-uhci3" type="usb"/> <controller index="0" model="ich9-ehci1" type="usb"/> <controller index="0" model="ich9-uhci1" type="usb"/> <controller index="0" model="ich9-uhci2" type="usb"/> Failed on: qemu-kvm: -device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x5.0x2: Parameter 'masterbus' expects an USB masterbus qemu-kvm: -device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x5.0x2: Device 'ich9-usb-uhci3' could not be initialized
From the qemu log file: LC_ALL=C PATH=/sbin:/usr/sbin:/bin:/usr/bin HOME=/root USER=root LOGNAME=root QEMU_AUDIO_DRV=spice /usr/libexec/qemu-kvm -S -M pc-0.14 -cpu kvm64,+lahf_lm,+ssse3,-cx16 -enable-kvm -m 512 -smp 1,sockets=1,cores=1,threads=1 -name usb5 -uuid d15a09b9-dde9-4b0e-8b76-5f7bc22be73a -smbios type=1,manufacturer=Red Hat,product=RHEV Hypervisor,version=6Server-6.3.0.2.el6,serial=44454C4C-4200-104C-8036-B9C04F30354A_84:2b:2b:bf:60:b6,uuid=d15a09b9-dde9-4b0e-8b76-5f7bc22be73a -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/usb5.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=2012-05-16T06:52:23,driftfix=slew -device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x5.0x2 -device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x5.0x7 -device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x5 -device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x5.0x1 -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x6 -drive if=none,id=drive-ide0-1-0,readonly=on,format=raw,serial= -device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 -drive file=/rhev/data-center/189b1e74-7421-11e1-ae9c-bf3b1f8a7b5b/690bf6ec-75e9-40b3-b754-8c79ee84e5e5/images/fa990a35-dcea-489c-a515-3e22cff48c17/d0111f10-1034-4911-af2a-d19a2804dfc1,if=none,id=drive-virtio-disk0,format=raw,serial=fa990a35-dcea-489c-a515-3e22cff48c17,cache=writeback,werror=stop,rerror=stop,aio=threads -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x7,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 -netdev tap,fd=26,id=hostnet0 -device virtio-net-pci,netdev=hostnet0,id=net0,mac=00:1a:4a:16:01:52,bus=pci.0,addr=0x3 -chardev socket,id=charchannel0,path=/var/lib/libvirt/qemu/channels/usb5.com.redhat.rhevm.vdsm,server,nowait -device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=com.redhat.rhevm.vdsm -chardev spicevmc,id=charchannel1,name=vdagent -device virtserialport,bus=virtio-serial0.0,nr=2,chardev=charchannel1,id=channel1,name=com.redhat.spice.0 -chardev pty,id=charconsole0 -device virtconsole,chardev=charconsole0,id=console0 -spice port=5901,addr=0 -k en-us -vga qxl -global qxl-vga.vram_size=67108864 -device intel-hda,id=sound0,bus=pci.0,addr=0x4 -device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 -chardev spicevmc,id=charredir0,name=usbredir -device usb-redir,chardev=charredir0,id=redir0 -chardev spicevmc,id=charredir1,name=usbredir -device usb-redir,chardev=charredir1,id=redir1 char device redirected to /dev/pts/3 do_spice_init: starting 0.10.1 spice_server_add_interface: SPICE_INTERFACE_KEYBOARD spice_server_add_interface: SPICE_INTERFACE_MOUSE spice_server_add_interface: SPICE_INTERFACE_QXL red_worker_main: begin display_channel_create: create display channel cursor_channel_create: create cursor channel qemu-kvm: -device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x5.0x2: Parameter 'masterbus' expects an USB masterbus qemu-kvm: -device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x5.0x2: Device 'ich9-usb-uhci3' could not be initialized
In the other VM the order was: <controller index="0" model="ich9-ehci1" type="usb"/> <controller index="0" model="ich9-uhci1" type="usb"/> <controller index="0" model="ich9-uhci2" type="usb"/> <controller index="0" model="ich9-uhci3" type="usb"/> And it succeeded.
From the qemu log: LC_ALL=C PATH=/sbin:/usr/sbin:/bin:/usr/bin HOME=/root USER=root LOGNAME=root QEMU_AUDIO_DRV=spice /usr/libexec/qemu-kvm -S -M pc-0.14 -cpu kvm64,+lahf_lm,+ssse3,-cx16 -enable-kvm -m 512 -smp 1,sockets=1,cores=1,threads=1 -name usb3 -uuid bfadbe4b-71e1-477c-ba65-e93a08fb8130 -smbios type=1,manufacturer=Red Hat,product=RHEV Hypervisor,version=6Server-6.3.0.2.el6,serial=44454C4C-4200-104C-8036-B9C04F30354A_84:2b:2b:bf:60:b6,uuid=bfadbe4b-71e1-477c-ba65-e93a08fb8130 -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/usb3.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=2012-05-15T12:21:46,driftfix=slew -device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x4.0x7 -device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x4 -device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x4.0x1 -device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x4.0x2 -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x5 -drive if=none,id=drive-ide0-1-0,readonly=on,format=raw,serial=11111111-1111-1111-1111-111111111111 -device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 -drive file=/rhev/data-center/189b1e74-7421-11e1-ae9c-bf3b1f8a7b5b/690bf6ec-75e9-40b3-b754-8c79ee84e5e5/images/c6e2e4f8-cc19-43bc-8c6e-ea0b4ece0eb5/154e2704-4d49-433f-8282-440809107424,if=none,id=drive-virtio-disk0,format=raw,serial=c6e2e4f8-cc19-43bc-8c6e-ea0b4ece0eb5,cache=writeback,werror=stop,rerror=stop,aio=threads -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x6,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 -chardev socket,id=charchannel0,path=/var/lib/libvirt/qemu/channels/usb3.com.redhat.rhevm.vdsm,server,nowait -device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=com.redhat.rhevm.vdsm -chardev spicevmc,id=charchannel1,name=vdagent -device virtserialport,bus=virtio-serial0.0,nr=2,chardev=charchannel1,id=channel1,name=com.redhat.spice.0 -chardev pty,id=charconsole0 -device virtconsole,chardev=charconsole0,id=console0 -spice port=5900,addr=0 -k en-us -vga qxl -global qxl-vga.vram_size=67108864 -device intel-hda,id=sound0,bus=pci.0,addr=0x3 -device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 -chardev spicevmc,id=charredir0,name=usbredir -device usb-redir,chardev=charredir0,id=redir0 -chardev spicevmc,id=charredir1,name=usbredir -device usb-redir,chardev=charredir1,id=redir1
Thank you, Oved ----- Original Message -----
From: "Daniel P. Berrange" <berrange@redhat.com> To: "Eric Blake" <eblake@redhat.com> Cc: libvir-list@redhat.com Sent: Tuesday, May 15, 2012 6:22:13 PM Subject: Re: [libvirt] [PATCH] Set a sensible default master start port for ehci companion controllers
On Mon, May 14, 2012 at 03:47:57PM -0600, Eric Blake wrote:
On 05/14/2012 06:24 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The uhci1, uhci2, uhci3 companion controllers for ehci1 must have a master start port set. Since this value is predictable we should set it automatically if the app does not supply it --- src/conf/domain_conf.c | 22 ++++++++++++++++++ .../qemuxml2argv-usb-ich9-ehci-addr.xml | 24 +++++--------------- 2 files changed, 28 insertions(+), 18 deletions(-)
ACK. But shouldn't we also have a test in qemuxml2xmltest, by using a file in qemuxml2xmloutdata/, that shows that we generate the correct start port on output in response to a user omitting information that we can determine by default?
Now I added a test case for the previous USB patch it automatically also tested the scenario you describe here.
Daniel -- |: http://berrange.com -o- | http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- | http://virt-manager.org :| |: http://autobuild.org -o- | http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- | http://live.gnome.org/gtk-vnc :|
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

Hey, I built libvirt RPM with this fix (and the previous addresses fix), and I get the following error when running a VM with USB support: internal error Process exited while reading console log output: qemu-kvm: -device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x4.0x7: Duplicate ID 'usb' for device This error happened also without these fixes (I thought these patches will address that, so I waited, but looks like the problem might not be related to it). Did you encounter such a problem when you tested it? The relevant devices I pass are: <controller index="0" model="ich9-ehci1" type="usb"/> <controller index="0" model="ich9-uhci1" type="usb"> <master startport="0"/> </controller> <controller index="0" model="ich9-uhci2" type="usb"> <master startport="2"/> </controller> <controller index="0" model="ich9-uhci3" type="usb"> <master startport="4"/> </controller> <redirdev bus="usb" type="spicevmc"/> <redirdev bus="usb" type="spicevmc"/> Thank you, Oved ----- Original Message -----
From: "Daniel P. Berrange" <berrange@redhat.com> To: libvir-list@redhat.com Cc: "Oved Ourfalli" <ovedo@redhat.com>, "Hans de Goede" <hdegoede@redhat.com>, "Daniel P. Berrange" <berrange@redhat.com> Sent: Monday, May 14, 2012 3:24:40 PM Subject: [PATCH] Set a sensible default master start port for ehci companion controllers
From: "Daniel P. Berrange" <berrange@redhat.com>
The uhci1, uhci2, uhci3 companion controllers for ehci1 must have a master start port set. Since this value is predictable we should set it automatically if the app does not supply it --- src/conf/domain_conf.c | 22 ++++++++++++++++++ .../qemuxml2argv-usb-ich9-ehci-addr.xml | 24 +++++--------------- 2 files changed, 28 insertions(+), 18 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 54ac1db..f4775be 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4066,6 +4066,28 @@ virDomainControllerDefParseXML(xmlNodePtr node, VIR_FREE(vectors); break; } + case VIR_DOMAIN_CONTROLLER_TYPE_USB: { + /* If the XML has a uhci1, uhci2, uhci3 controller and no + * master port was given, we should set a sensible one */ + int masterPort = -1; + switch (def->model) { + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI1: + masterPort = 0; + break; + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI2: + masterPort = 2; + break; + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI3: + masterPort = 4; + break; + } + if (masterPort != -1 && + def->info.mastertype == VIR_DOMAIN_CONTROLLER_MASTER_NONE) { + def->info.mastertype = VIR_DOMAIN_CONTROLLER_MASTER_USB; + def->info.master.usb.startport = masterPort; + } + break; + }
default: break; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.xml index 8eff1d7..ad85d63 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.xml @@ -19,19 +19,13 @@ <controller type='usb' index='1' model='ich9-ehci1'> </controller>
- <controller type='usb' index='0' model='ich9-uhci1'> - <master startport='0'/> - </controller> - <controller type='usb' index='1' model='ich9-uhci1'> - <master startport='0'/> - </controller> + <controller type='usb' index='0' model='ich9-uhci1'/> + <controller type='usb' index='1' model='ich9-uhci1'/> <controller type='usb' index='2' model='ich9-uhci1'> <master startport='0'/> </controller>
- <controller type='usb' index='0' model='ich9-uhci3'> - <master startport='4'/> - </controller> + <controller type='usb' index='0' model='ich9-uhci3'/> <controller type='usb' index='1' model='ich9-uhci3'> <master startport='4'/> </controller> @@ -39,15 +33,9 @@ <master startport='4'/> </controller>
- <controller type='usb' index='2' model='ich9-uhci2'> - <master startport='2'/> - </controller> - <controller type='usb' index='1' model='ich9-uhci2'> - <master startport='2'/> - </controller> - <controller type='usb' index='0' model='ich9-uhci2'> - <master startport='2'/> - </controller> + <controller type='usb' index='2' model='ich9-uhci2'/> + <controller type='usb' index='1' model='ich9-uhci2'/> + <controller type='usb' index='0' model='ich9-uhci2'/> <memballoon model='virtio'/> </devices> </domain> -- 1.7.10.1

Did some more digging on that. The id is constructed in the function qemuUsbId: static void qemuUsbId(virBufferPtr buf, int idx) { if (idx == 0) virBufferAsprintf(buf, "usb"); else virBufferAsprintf(buf, "usb%d", idx); } So, as you can see it is either usb, or usbX... However, according to https://bugzilla.redhat.com/show_bug.cgi?id=820869, all controllers should contain the same index, resulting in an ID duplication (as, IIUC, the idx which is passed is the controller index). I don't know the importance of the ID to qemu-kvm, but perhaps it should be a concatenation of both the qemu name and the index, i.e., ich9-usb-ehci1-0 instead of usb, ich9-usb-ehci1-1 instead of usb1 and etc. What do you think? ----- Original Message -----
From: "Oved Ourfalli" <ovedo@redhat.com> To: "Daniel P. Berrange" <berrange@redhat.com> Cc: libvir-list@redhat.com Sent: Tuesday, May 15, 2012 8:41:32 AM Subject: Re: [libvirt] [PATCH] Set a sensible default master start port for ehci companion controllers
Hey,
I built libvirt RPM with this fix (and the previous addresses fix), and I get the following error when running a VM with USB support: internal error Process exited while reading console log output: qemu-kvm: -device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x4.0x7: Duplicate ID 'usb' for device
This error happened also without these fixes (I thought these patches will address that, so I waited, but looks like the problem might not be related to it).
Did you encounter such a problem when you tested it?
The relevant devices I pass are: <controller index="0" model="ich9-ehci1" type="usb"/> <controller index="0" model="ich9-uhci1" type="usb"> <master startport="0"/> </controller> <controller index="0" model="ich9-uhci2" type="usb"> <master startport="2"/> </controller> <controller index="0" model="ich9-uhci3" type="usb"> <master startport="4"/> </controller> <redirdev bus="usb" type="spicevmc"/> <redirdev bus="usb" type="spicevmc"/>
Thank you, Oved ----- Original Message -----
From: "Daniel P. Berrange" <berrange@redhat.com> To: libvir-list@redhat.com Cc: "Oved Ourfalli" <ovedo@redhat.com>, "Hans de Goede" <hdegoede@redhat.com>, "Daniel P. Berrange" <berrange@redhat.com> Sent: Monday, May 14, 2012 3:24:40 PM Subject: [PATCH] Set a sensible default master start port for ehci companion controllers
From: "Daniel P. Berrange" <berrange@redhat.com>
The uhci1, uhci2, uhci3 companion controllers for ehci1 must have a master start port set. Since this value is predictable we should set it automatically if the app does not supply it --- src/conf/domain_conf.c | 22 ++++++++++++++++++ .../qemuxml2argv-usb-ich9-ehci-addr.xml | 24 +++++--------------- 2 files changed, 28 insertions(+), 18 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 54ac1db..f4775be 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4066,6 +4066,28 @@ virDomainControllerDefParseXML(xmlNodePtr node, VIR_FREE(vectors); break; } + case VIR_DOMAIN_CONTROLLER_TYPE_USB: { + /* If the XML has a uhci1, uhci2, uhci3 controller and no + * master port was given, we should set a sensible one */ + int masterPort = -1; + switch (def->model) { + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI1: + masterPort = 0; + break; + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI2: + masterPort = 2; + break; + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI3: + masterPort = 4; + break; + } + if (masterPort != -1 && + def->info.mastertype == VIR_DOMAIN_CONTROLLER_MASTER_NONE) { + def->info.mastertype = VIR_DOMAIN_CONTROLLER_MASTER_USB; + def->info.master.usb.startport = masterPort; + } + break; + }
default: break; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.xml index 8eff1d7..ad85d63 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.xml @@ -19,19 +19,13 @@ <controller type='usb' index='1' model='ich9-ehci1'> </controller>
- <controller type='usb' index='0' model='ich9-uhci1'> - <master startport='0'/> - </controller> - <controller type='usb' index='1' model='ich9-uhci1'> - <master startport='0'/> - </controller> + <controller type='usb' index='0' model='ich9-uhci1'/> + <controller type='usb' index='1' model='ich9-uhci1'/> <controller type='usb' index='2' model='ich9-uhci1'> <master startport='0'/> </controller>
- <controller type='usb' index='0' model='ich9-uhci3'> - <master startport='4'/> - </controller> + <controller type='usb' index='0' model='ich9-uhci3'/> <controller type='usb' index='1' model='ich9-uhci3'> <master startport='4'/> </controller> @@ -39,15 +33,9 @@ <master startport='4'/> </controller>
- <controller type='usb' index='2' model='ich9-uhci2'> - <master startport='2'/> - </controller> - <controller type='usb' index='1' model='ich9-uhci2'> - <master startport='2'/> - </controller> - <controller type='usb' index='0' model='ich9-uhci2'> - <master startport='2'/> - </controller> + <controller type='usb' index='2' model='ich9-uhci2'/> + <controller type='usb' index='1' model='ich9-uhci2'/> + <controller type='usb' index='0' model='ich9-uhci2'/> <memballoon model='virtio'/> </devices> </domain> -- 1.7.10.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, May 15, 2012 at 01:41:32AM -0400, Oved Ourfalli wrote:
Hey,
I built libvirt RPM with this fix (and the previous addresses fix), and I get the following error when running a VM with USB support: internal error Process exited while reading console log output: qemu-kvm: -device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x4.0x7: Duplicate ID 'usb' for device
This error happened also without these fixes (I thought these patches will address that, so I waited, but looks like the problem might not be related to it).
Did you encounter such a problem when you tested it?
No, can you provide the full XML document you generate, along with the contents of /var/log/libvirt/qemu/$DOMAIN.log so I can see the corresponding CLI argv Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

----- Original Message -----
From: "Daniel P. Berrange" <berrange@redhat.com> To: "Oved Ourfalli" <ovedo@redhat.com> Cc: "Hans de Goede" <hdegoede@redhat.com>, libvir-list@redhat.com Sent: Tuesday, May 15, 2012 11:49:39 AM Subject: Re: [PATCH] Set a sensible default master start port for ehci companion controllers
On Tue, May 15, 2012 at 01:41:32AM -0400, Oved Ourfalli wrote:
Hey,
I built libvirt RPM with this fix (and the previous addresses fix), and I get the following error when running a VM with USB support: internal error Process exited while reading console log output: qemu-kvm: -device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x4.0x7: Duplicate ID 'usb' for device
This error happened also without these fixes (I thought these patches will address that, so I waited, but looks like the problem might not be related to it).
Did you encounter such a problem when you tested it?
No, can you provide the full XML document you generate, along with the contents of /var/log/libvirt/qemu/$DOMAIN.log so I can see the corresponding CLI argv
Attached. Just to make sure, did you see my other E-mail, specifying why I think the issue happens? (problem in qemuUsbId function). Let me know if you need more information, Oved
Daniel -- |: http://berrange.com -o- | http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- | http://virt-manager.org :| |: http://autobuild.org -o- | http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- | http://live.gnome.org/gtk-vnc :|

On Tue, May 15, 2012 at 05:25:23AM -0400, Oved Ourfalli wrote:
-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x4.0x7 -device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x4 -device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x4.0x1 -device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x4.0x2
Here we see the problem - we have a USB1 controller (piix3-usb0-uhci) and the USB2 controller + companions. Both the USB1 & USB2 controller are getting the same name.
<controller type="usb"> <address domain="0x0000" function="0x2" slot="0x01" type="pci" bus="0x00"/> </controller>
Here you have requested the USB1 controller (defaults to piix3-usb-uhci) but ommitted the index= attribute, so it defaults to 0.
<controller index="0" model="ich9-ehci1" type="usb"/>
And here you have requested USB2 controller also with index=0.
<controller index="0" model="ich9-uhci1" type="usb"> <master startport="0"/> </controller> <controller index="0" model="ich9-uhci2" type="usb"> <master startport="2"/> </controller> <controller index="0" model="ich9-uhci3" type="usb"> <master startport="4"/> </controller>
And these are the companions - it is correct that these have the same index as the master controller. Libvirt really ought to have raised an error about your XML with the duplicated index for USB1 and USB2 controllers. Of course you can just remove the USB1 controller from your XML and avoid the issue, since the companions provide USB1 compatibility Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

You're right. I used an old VM and it didn't work well (VM from early dev phase for this feature). I created a new one. First time I run it, it works. The second time, in which we pass the address (as we store it in the ovirt engine database), I get the following XML: <controller index="0" model="ich9-uhci1" type="usb"> <master startport="0"/> <address domain="0x0000" function="0x2" slot="0x04" type="pci" bus="0x00"/> </controller> <controller index="0" model="ich9-ehci1" type="usb"> <address domain="0x0000" function="0x2" slot="0x04" type="pci" bus="0x00"/> </controller> <controller index="0" model="ich9-uhci3" type="usb"> <master startport="4"/> <address domain="0x0000" function="0x2" slot="0x04" type="pci" bus="0x00"/> </controller> <controller index="0" model="ich9-uhci2" type="usb"> <master startport="2"/> <address domain="0x0000" function="0x2" slot="0x04" type="pci" bus="0x00"/> </controller> And I get the following error: XML error: Attempted double use of PCI Address '0:0:4.2' (may need "multifunction='on'" for device on function 0) It is weird that the functions above are not as expected from your source code. Also, the multifunction isn't set to on. But, they were okay in the first time, otherwise it wouldn't have worked. Any suggestions? Thank you, Oved ----- Original Message -----
From: "Daniel P. Berrange" <berrange@redhat.com> To: "Oved Ourfalli" <ovedo@redhat.com> Cc: "Hans de Goede" <hdegoede@redhat.com>, libvir-list@redhat.com Sent: Tuesday, May 15, 2012 12:40:12 PM Subject: Re: [PATCH] Set a sensible default master start port for ehci companion controllers
On Tue, May 15, 2012 at 05:25:23AM -0400, Oved Ourfalli wrote:
-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x4.0x7 -device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x4 -device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x4.0x1 -device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x4.0x2
Here we see the problem - we have a USB1 controller (piix3-usb0-uhci) and the USB2 controller + companions.
Both the USB1 & USB2 controller are getting the same name.
<controller type="usb"> <address domain="0x0000" function="0x2" slot="0x01" type="pci" bus="0x00"/> </controller>
Here you have requested the USB1 controller (defaults to piix3-usb-uhci) but ommitted the index= attribute, so it defaults to 0.
<controller index="0" model="ich9-ehci1" type="usb"/>
And here you have requested USB2 controller also with index=0.
<controller index="0" model="ich9-uhci1" type="usb"> <master startport="0"/> </controller> <controller index="0" model="ich9-uhci2" type="usb"> <master startport="2"/> </controller> <controller index="0" model="ich9-uhci3" type="usb"> <master startport="4"/> </controller>
And these are the companions - it is correct that these have the same index as the master controller.
Libvirt really ought to have raised an error about your XML with the duplicated index for USB1 and USB2 controllers. Of course you can just remove the USB1 controller from your XML and avoid the issue, since the companions provide USB1 compatibility
Daniel -- |: http://berrange.com -o- | http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- | http://virt-manager.org :| |: http://autobuild.org -o- | http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- | http://live.gnome.org/gtk-vnc :|

On Tue, May 15, 2012 at 07:22:52AM -0400, Oved Ourfalli wrote:
You're right. I used an old VM and it didn't work well (VM from early dev phase for this feature).
I created a new one. First time I run it, it works. The second time, in which we pass the address (as we store it in the ovirt engine database), I get the following XML: <controller index="0" model="ich9-uhci1" type="usb"> <master startport="0"/> <address domain="0x0000" function="0x2" slot="0x04" type="pci" bus="0x00"/> </controller> <controller index="0" model="ich9-ehci1" type="usb"> <address domain="0x0000" function="0x2" slot="0x04" type="pci" bus="0x00"/> </controller> <controller index="0" model="ich9-uhci3" type="usb"> <master startport="4"/> <address domain="0x0000" function="0x2" slot="0x04" type="pci" bus="0x00"/> </controller> <controller index="0" model="ich9-uhci2" type="usb"> <master startport="2"/> <address domain="0x0000" function="0x2" slot="0x04" type="pci" bus="0x00"/> </controller>
It looks like you're not recording address correctly. You have put all devices on the same function number here. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (3)
-
Daniel P. Berrange
-
Eric Blake
-
Oved Ourfalli