Re: [libvirt] [PATCH v2 1/6] tests: qemuxml2argv: remove some QEMU_CAPS_DEVICE problem cases

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)