| Message ID | 285fd539cd389286ba6278f4acb7ffebba134c2f.1455032128.git.crobinso@redhat.com |
|---|---|
| State | Accepted |
| Commit | e9394d699c091d81edfb24735e69cdee707aabbc |
| Headers | show |
On 02/09/2016 01:31 PM, Laine Stump wrote: > On 02/09/2016 10:58 AM, Cole Robinson wrote: >> When we unconditionally enable QEMU_CAPS_DEVICE, these tests need >> some massaging, so do it ahead of time to not mix it in with the >> big test refresh. >> >> - minimal-s390 is not a real world working config, so drop it >> - disk-usb was testing for an old code path that will be removed. >> instead use it to test lack of USB disk support, and rename it >> to disk-usb-nosupport. Switch xml2xml to use disk-usb-device for >> input. >> - cputune-numatune was needlessly using q35, switch it to an older >> machine type >> --- >> .../qemuxml2argv-cputune-numatune.args | 7 +++---- >> .../qemuxml2argv-cputune-numatune.xml | 12 +---------- >> .../qemuxml2argv-disk-usb-nosupport.xml} | 0 >> tests/qemuxml2argvdata/qemuxml2argv-disk-usb.args | 23 >> ---------------------- >> .../qemuxml2argv-minimal-s390.args | 21 -------------------- >> .../qemuxml2argvdata/qemuxml2argv-minimal-s390.xml | 21 -------------------- >> tests/qemuxml2argvtest.c | 5 ++--- >> .../qemuxml2xmlout-cputune-numatune.xml | 14 +++---------- >> .../qemuxml2xmlout-disk-usb-device.xml} | 6 ++---- >> tests/qemuxml2xmltest.c | 2 +- >> 10 files changed, 12 insertions(+), 99 deletions(-) >> rename tests/{qemuxml2xmloutdata/qemuxml2xmlout-disk-usb.xml => >> qemuxml2argvdata/qemuxml2argv-disk-usb-nosupport.xml} (100%) >> delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-usb.args >> delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-minimal-s390.args >> delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-minimal-s390.xml >> rename tests/{qemuxml2argvdata/qemuxml2argv-disk-usb.xml => >> qemuxml2xmloutdata/qemuxml2xmlout-disk-usb-device.xml} (90%) >> >> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.args >> b/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.args >> index 7e5678d..bde6338 100644 >> --- a/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.args >> +++ b/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.args >> @@ -7,16 +7,15 @@ QEMU_AUDIO_DRV=none \ >> /usr/bin/qemu-system-x86_64 \ >> -name dummy2 \ >> -S \ >> --M pc-q35-2.3 \ >> +-M pc-0.11 \ >> -m 128 \ >> -smp 2,maxcpus=6,sockets=6,cores=1,threads=1 \ >> -object iothread,id=iothread1 \ >> -object iothread,id=iothread2 \ >> -uuid 4d92ec27-9ebf-400b-ae91-20c71c647c19 \ >> -nographic \ >> +-nodefaults \ >> -monitor unix:/tmp/test-monitor,server,nowait \ >> -no-acpi \ >> -boot c \ >> --net none \ >> --serial none \ >> --parallel none >> +-usb > > Why did I think that we had eliminated all use of "-usb"? Is it that we're > currently only using it in the tests? If so, that's another thing to eliminate > - we should never be using -usb unless that's the only way to get a usb device > for a particular qemu (and I don't know that that is *ever* the case). > The qemu_command.c chunk is: if (usbcontroller == 0 && !qemuDomainMachineIsQ35(def) && !ARCH_IS_S390(def->os.arch)) virCommandAddArg(cmd, "-usb") Where usbcontroller == 0 if we didn't build a -device string for a USB controller. I'd need to dig into qemu to figure out what -usb actually maps to to determine if that makes sense anymore > That's not the problem of this patch, though. ACK. > Thanks, pushed now - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 02/09/2016 02:28 PM, Laine Stump wrote: > On 02/09/2016 01:46 PM, Cole Robinson wrote: >> On 02/09/2016 01:31 PM, Laine Stump wrote: >>> On 02/09/2016 10:58 AM, Cole Robinson wrote: >>>> When we unconditionally enable QEMU_CAPS_DEVICE, these tests need >>>> some massaging, so do it ahead of time to not mix it in with the >>>> big test refresh. >>>> >>>> - minimal-s390 is not a real world working config, so drop it >>>> - disk-usb was testing for an old code path that will be removed. >>>> instead use it to test lack of USB disk support, and rename it >>>> to disk-usb-nosupport. Switch xml2xml to use disk-usb-device for >>>> input. >>>> - cputune-numatune was needlessly using q35, switch it to an older >>>> machine type >>>> --- >>>> .../qemuxml2argv-cputune-numatune.args | 7 +++---- >>>> .../qemuxml2argv-cputune-numatune.xml | 12 +---------- >>>> .../qemuxml2argv-disk-usb-nosupport.xml} | 0 >>>> tests/qemuxml2argvdata/qemuxml2argv-disk-usb.args | 23 >>>> ---------------------- >>>> .../qemuxml2argv-minimal-s390.args | 21 >>>> -------------------- >>>> .../qemuxml2argvdata/qemuxml2argv-minimal-s390.xml | 21 >>>> -------------------- >>>> tests/qemuxml2argvtest.c | 5 ++--- >>>> .../qemuxml2xmlout-cputune-numatune.xml | 14 +++---------- >>>> .../qemuxml2xmlout-disk-usb-device.xml} | 6 ++---- >>>> tests/qemuxml2xmltest.c | 2 +- >>>> 10 files changed, 12 insertions(+), 99 deletions(-) >>>> rename tests/{qemuxml2xmloutdata/qemuxml2xmlout-disk-usb.xml => >>>> qemuxml2argvdata/qemuxml2argv-disk-usb-nosupport.xml} (100%) >>>> delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-usb.args >>>> delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-minimal-s390.args >>>> delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-minimal-s390.xml >>>> rename tests/{qemuxml2argvdata/qemuxml2argv-disk-usb.xml => >>>> qemuxml2xmloutdata/qemuxml2xmlout-disk-usb-device.xml} (90%) >>>> >>>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.args >>>> b/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.args >>>> index 7e5678d..bde6338 100644 >>>> --- a/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.args >>>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.args >>>> @@ -7,16 +7,15 @@ QEMU_AUDIO_DRV=none \ >>>> /usr/bin/qemu-system-x86_64 \ >>>> -name dummy2 \ >>>> -S \ >>>> --M pc-q35-2.3 \ >>>> +-M pc-0.11 \ >>>> -m 128 \ >>>> -smp 2,maxcpus=6,sockets=6,cores=1,threads=1 \ >>>> -object iothread,id=iothread1 \ >>>> -object iothread,id=iothread2 \ >>>> -uuid 4d92ec27-9ebf-400b-ae91-20c71c647c19 \ >>>> -nographic \ >>>> +-nodefaults \ >>>> -monitor unix:/tmp/test-monitor,server,nowait \ >>>> -no-acpi \ >>>> -boot c \ >>>> --net none \ >>>> --serial none \ >>>> --parallel none >>>> +-usb >>> Why did I think that we had eliminated all use of "-usb"? Is it that we're >>> currently only using it in the tests? If so, that's another thing to eliminate >>> - we should never be using -usb unless that's the only way to get a usb device >>> for a particular qemu (and I don't know that that is *ever* the case). >>> >> The qemu_command.c chunk is: >> >> if (usbcontroller == 0 && >> !qemuDomainMachineIsQ35(def) && >> !ARCH_IS_S390(def->os.arch)) >> virCommandAddArg(cmd, "-usb") >> >> Where usbcontroller == 0 if we didn't build a -device string for a USB >> controller. I'd need to dig into qemu to figure out what -usb actually maps to >> to determine if that makes sense anymore > > Okay, that's what I remember. usbcontroller will only be 0 if there was no > <controller type='usb'/> in the xml, and that shouldn't happen because the > post-parse will add a usb controller if there isn't any (except in a few cases > - see qemuDomainDefAddDefaultDevices). So is the post-parse not happening for > the qemuxml2argvtest? (there are dozens and dozens of "-usb" in the *.args files) > > PostParse is triggered automatically via DomainDefParse. You can see in the qemuxml2xml output that there's a USB controller added. Looking deeper, that usbcontroller bit is only set if QEMU_CAPS_PIIX3_USB_UHCI is available, otherwise it uses legacyusb aka -usb. Maybe that's another flag that we can assume is available unconditionally for the qemu we support, I'd need to dig a bit - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.args b/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.args index 7e5678d..bde6338 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.args @@ -7,16 +7,15 @@ QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-x86_64 \ -name dummy2 \ -S \ --M pc-q35-2.3 \ +-M pc-0.11 \ -m 128 \ -smp 2,maxcpus=6,sockets=6,cores=1,threads=1 \ -object iothread,id=iothread1 \ -object iothread,id=iothread2 \ -uuid 4d92ec27-9ebf-400b-ae91-20c71c647c19 \ -nographic \ +-nodefaults \ -monitor unix:/tmp/test-monitor,server,nowait \ -no-acpi \ -boot c \ --net none \ --serial none \ --parallel none +-usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml b/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml index 855e084..2258e96 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml @@ -13,7 +13,7 @@ <memory mode='strict' placement='auto'/> </numatune> <os> - <type arch='x86_64' machine='pc-q35-2.3'>hvm</type> + <type arch='x86_64' machine='pc'>hvm</type> <boot dev='hd'/> </os> <clock offset='utc'/> @@ -22,16 +22,6 @@ <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-x86_64</emulator> - <controller type='sata' index='0'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> - </controller> - <controller type='pci' index='0' model='pcie-root'/> - <controller type='pci' index='1' model='dmi-to-pci-bridge'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x1e' function='0x0'/> - </controller> - <controller type='pci' index='2' model='pci-bridge'> - <address type='pci' domain='0x0000' bus='0x01' slot='0x01' function='0x0'/> - </controller> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <memballoon model='none'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-usb.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-nosupport.xml similarity index 100% rename from tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-usb.xml rename to tests/qemuxml2argvdata/qemuxml2argv-disk-usb-nosupport.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-usb.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-usb.args deleted file mode 100644 index f41f8b4..0000000 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-usb.args +++ /dev/null @@ -1,23 +0,0 @@ -LC_ALL=C \ -PATH=/bin \ -HOME=/home/test \ -USER=test \ -LOGNAME=test \ -QEMU_AUDIO_DRV=none \ -/usr/bin/qemu \ --name QEMUGuest1 \ --S \ --M pc \ --m 214 \ --smp 1 \ --uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ --nographic \ --monitor unix:/tmp/test-monitor,server,nowait \ --no-acpi \ --boot c \ --usb \ --drive file=/dev/HostVG/QEMUGuest1,format=raw,if=ide,bus=0,unit=0 \ --usbdevice disk:/tmp/usbdisk.img \ --net none \ --serial none \ --parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-minimal-s390.args b/tests/qemuxml2argvdata/qemuxml2argv-minimal-s390.args deleted file mode 100644 index a51d1f0..0000000 --- a/tests/qemuxml2argvdata/qemuxml2argv-minimal-s390.args +++ /dev/null @@ -1,21 +0,0 @@ -LC_ALL=C \ -PATH=/bin \ -HOME=/home/test \ -USER=test \ -LOGNAME=test \ -QEMU_AUDIO_DRV=none \ -/usr/bin/qemu \ --name QEMUGuest1 \ --S \ --M s390-virtio \ --m 214 \ --smp 1 \ --uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ --nographic \ --monitor unix:/tmp/test-monitor,server,nowait \ --no-acpi \ --boot c \ --drive file=/dev/HostVG/QEMUGuest1,format=raw,if=virtio \ --net none \ --serial none \ --parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-minimal-s390.xml b/tests/qemuxml2argvdata/qemuxml2argv-minimal-s390.xml deleted file mode 100644 index 7378bd4..0000000 --- a/tests/qemuxml2argvdata/qemuxml2argv-minimal-s390.xml +++ /dev/null @@ -1,21 +0,0 @@ -<domain type='qemu'> - <name>QEMUGuest1</name> - <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> - <memory>219100</memory> - <currentMemory>219100</currentMemory> - <os> - <type arch='s390x' machine='s390-virtio'>hvm</type> - <boot dev='hd'/> - </os> - <clock offset='utc'/> - <on_poweroff>destroy</on_poweroff> - <on_reboot>restart</on_reboot> - <on_crash>destroy</on_crash> - <devices> - <emulator>/usr/bin/qemu</emulator> - <disk type='block' device='disk'> - <source dev='/dev/HostVG/QEMUGuest1'/> - <target dev='hda' bus='virtio'/> - </disk> - </devices> -</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 9a37fbc..0b46d4d 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -603,7 +603,6 @@ mymain(void) DO_TEST("minimal", NONE); DO_TEST_PARSE_ERROR("minimal-no-memory", NONE); DO_TEST("minimal-msg-timestamp", QEMU_CAPS_MSG_TIMESTAMP); - DO_TEST("minimal-s390", NONE); DO_TEST("machine-aliases1", NONE); DO_TEST("machine-aliases2", QEMU_CAPS_KVM); DO_TEST("machine-core-on", QEMU_CAPS_MACHINE_OPT, @@ -819,7 +818,7 @@ mymain(void) QEMU_CAPS_DEVICE, QEMU_CAPS_BOOTINDEX); DO_TEST_PARSE_ERROR("disk-device-lun-type-invalid", QEMU_CAPS_DEVICE, QEMU_CAPS_VIRTIO_SCSI); - DO_TEST("disk-usb", NONE); + DO_TEST_FAILURE("disk-usb-nosupport", QEMU_CAPS_DEVICE); DO_TEST("disk-usb-device", QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_USB_STORAGE, QEMU_CAPS_NODEFCONFIG); @@ -1337,7 +1336,7 @@ mymain(void) DO_TEST_PARSE_ERROR("cputune-iothreadsched-toomuch", NONE); DO_TEST_PARSE_ERROR("cputune-vcpusched-overlap", NONE); DO_TEST("cputune-numatune", QEMU_CAPS_SMP_TOPOLOGY, - QEMU_CAPS_KVM, + QEMU_CAPS_KVM, QEMU_CAPS_DEVICE, QEMU_CAPS_OBJECT_IOTHREAD, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE); diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-cputune-numatune.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-cputune-numatune.xml index 855e084..1278b00 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-cputune-numatune.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-cputune-numatune.xml @@ -13,7 +13,7 @@ <memory mode='strict' placement='auto'/> </numatune> <os> - <type arch='x86_64' machine='pc-q35-2.3'>hvm</type> + <type arch='x86_64' machine='pc'>hvm</type> <boot dev='hd'/> </os> <clock offset='utc'/> @@ -22,16 +22,8 @@ <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-x86_64</emulator> - <controller type='sata' index='0'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> - </controller> - <controller type='pci' index='0' model='pcie-root'/> - <controller type='pci' index='1' model='dmi-to-pci-bridge'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x1e' function='0x0'/> - </controller> - <controller type='pci' index='2' model='pci-bridge'> - <address type='pci' domain='0x0000' bus='0x01' slot='0x01' function='0x0'/> - </controller> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <memballoon model='none'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-usb.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-usb-device.xml similarity index 90% rename from tests/qemuxml2argvdata/qemuxml2argv-disk-usb.xml rename to tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-usb-device.xml index 06d75aa..6e132cb 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-usb.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-usb-device.xml @@ -15,21 +15,19 @@ <devices> <emulator>/usr/bin/qemu</emulator> <disk type='block' device='disk'> - <driver name='qemu' type='raw'/> <source dev='/dev/HostVG/QEMUGuest1'/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='disk'> - <driver name='qemu' type='raw'/> <source file='/tmp/usbdisk.img'/> <target dev='sda' bus='usb'/> </disk> <controller type='usb' index='0'/> - <controller type='ide' index='0'/> <controller type='pci' index='0' model='pci-root'/> + <controller type='ide' index='0'/> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> - <memballoon model='none'/> + <memballoon model='virtio'/> </devices> </domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 0c22f53..465de8e 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -339,7 +339,7 @@ mymain(void) DO_TEST("disk-floppy"); DO_TEST("disk-many"); DO_TEST("disk-xenvbd"); - DO_TEST("disk-usb"); + DO_TEST("disk-usb-device"); DO_TEST("disk-virtio"); DO_TEST("floppy-drive-fat"); DO_TEST("disk-drive-boot-disk");
When we unconditionally enable QEMU_CAPS_DEVICE, these tests need some massaging, so do it ahead of time to not mix it in with the big test refresh. - minimal-s390 is not a real world working config, so drop it - disk-usb was testing for an old code path that will be removed. instead use it to test lack of USB disk support, and rename it to disk-usb-nosupport. Switch xml2xml to use disk-usb-device for input. - cputune-numatune was needlessly using q35, switch it to an older machine type --- .../qemuxml2argv-cputune-numatune.args | 7 +++---- .../qemuxml2argv-cputune-numatune.xml | 12 +---------- .../qemuxml2argv-disk-usb-nosupport.xml} | 0 tests/qemuxml2argvdata/qemuxml2argv-disk-usb.args | 23 ---------------------- .../qemuxml2argv-minimal-s390.args | 21 -------------------- .../qemuxml2argvdata/qemuxml2argv-minimal-s390.xml | 21 -------------------- tests/qemuxml2argvtest.c | 5 ++--- .../qemuxml2xmlout-cputune-numatune.xml | 14 +++---------- .../qemuxml2xmlout-disk-usb-device.xml} | 6 ++---- tests/qemuxml2xmltest.c | 2 +- 10 files changed, 12 insertions(+), 99 deletions(-) rename tests/{qemuxml2xmloutdata/qemuxml2xmlout-disk-usb.xml => qemuxml2argvdata/qemuxml2argv-disk-usb-nosupport.xml} (100%) delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-usb.args delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-minimal-s390.args delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-minimal-s390.xml rename tests/{qemuxml2argvdata/qemuxml2argv-disk-usb.xml => qemuxml2xmloutdata/qemuxml2xmlout-disk-usb-device.xml} (90%) -- 2.5.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list