[libvirt] [PATCH 1/3] tests: add qemu x86 kvm 32-on-64 test

[Cole Robinson]

On 07/21/2017 07:40 AM, Pavel Hrdina wrote:
> On Fri, Jul 14, 2017 at 07:43:04PM -0400, Cole Robinson wrote:
>> There's some specific logic in qemuBuildCpuCommandLine to support
>> auto adding -cpu qemu 32 for arch=i686 with an x86_64 qemu-kvm binary.
>> Add a test case for it
>>
>> Signed-off-by: Cole Robinson <crobinso at redhat.com>
>> ---
>>  .../qemuxml2argv-x86-kvm-32-on-64.args              | 21 +++++++++++++++++++++
>>  .../qemuxml2argv-x86-kvm-32-on-64.xml               | 13 +++++++++++++
>>  tests/qemuxml2argvtest.c                            |  1 +
>>  tests/testutilsqemu.c                               | 18 ++++++++++++++++--
>>  4 files changed, 51 insertions(+), 2 deletions(-)
>>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-x86-kvm-32-on-64.args
>>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-x86-kvm-32-on-64.xml
>>
>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-x86-kvm-32-on-64.args b/tests/qemuxml2argvdata/qemuxml2argv-x86-kvm-32-on-64.args
>> new file mode 100644
>> index 000000000..5fdeaf843
>> --- /dev/null
>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-x86-kvm-32-on-64.args
>> @@ -0,0 +1,21 @@
>> +LC_ALL=C \
>> +PATH=/bin \
>> +HOME=/home/test \
>> +USER=test \
>> +LOGNAME=test \
>> +QEMU_AUDIO_DRV=none \
>> +/usr/bin/qemu-kvm \
>> +-name kvm \
>> +-S \
>> +-machine pc,accel=kvm \
>> +-cpu qemu32 \
>> +-m 4096 \
>> +-smp 1,sockets=1,cores=1,threads=1 \
>> +-uuid d091ea82-29e6-2e34-3005-f02617b36e87 \
>> +-nographic \
>> +-nodefaults \
>> +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-kvm/monitor.sock,server,\
>> +nowait \
>> +-mon chardev=charmonitor,id=monitor,mode=readline \
>> +-no-acpi \
>> +-boot c
>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-x86-kvm-32-on-64.xml b/tests/qemuxml2argvdata/qemuxml2argv-x86-kvm-32-on-64.xml
>> new file mode 100644
>> index 000000000..2939cec15
>> --- /dev/null
>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-x86-kvm-32-on-64.xml
>> @@ -0,0 +1,13 @@
>> +<domain type='kvm'>
>> +  <name>kvm</name>
>> +  <uuid>d091ea82-29e6-2e34-3005-f02617b36e87</uuid>
>> +  <memory unit='KiB'>4194304</memory>
>> +  <os>
>> +    <type arch='i686'>hvm</type>
>> +  </os>
>> +  <devices>
>> +    <emulator>/usr/bin/qemu-kvm</emulator>
>> +    <controller type='usb' model='none'/>
>> +    <memballoon model='none'/>
>> +  </devices>
>> +</domain>
>> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
>> index 302c9c892..ef5a9b0dc 100644
>> --- a/tests/qemuxml2argvtest.c
>> +++ b/tests/qemuxml2argvtest.c
>> @@ -690,6 +690,7 @@ mymain(void)
>>      DO_TEST("kvm", QEMU_CAPS_MACHINE_OPT);
>>      DO_TEST("default-kvm-host-arch", QEMU_CAPS_MACHINE_OPT);
>>      DO_TEST("default-qemu-host-arch", QEMU_CAPS_MACHINE_OPT);
>> +    DO_TEST("x86-kvm-32-on-64", QEMU_CAPS_MACHINE_OPT);
>>      DO_TEST("boot-cdrom", NONE);
>>      DO_TEST("boot-network", NONE);
>>      DO_TEST("boot-floppy", NONE);
>> diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
>> index ee4853841..d1290fdde 100644
>> --- a/tests/testutilsqemu.c
>> +++ b/tests/testutilsqemu.c
>> @@ -111,7 +111,8 @@ typedef enum {
>>      TEST_UTILS_QEMU_BIN_ARM,
>>      TEST_UTILS_QEMU_BIN_PPC64,
>>      TEST_UTILS_QEMU_BIN_PPC,
>> -    TEST_UTILS_QEMU_BIN_S390X
>> +    TEST_UTILS_QEMU_BIN_S390X,
>> +    TEST_UTILS_QEMU_BIN_KVM,
>>  } QEMUBinType;
>>  
>>  static const char *QEMUBinList[] = {
>> @@ -121,7 +122,8 @@ static const char *QEMUBinList[] = {
>>      "/usr/bin/qemu-system-arm",
>>      "/usr/bin/qemu-system-ppc64",
>>      "/usr/bin/qemu-system-ppc",
>> -    "/usr/bin/qemu-system-s390x"
>> +    "/usr/bin/qemu-system-s390x",
>> +    "/usr/bin/qemu-kvm",
> 
> I don't like this patch because it adds a binary that is specific to
> downstream releases.  The whole point of my previous patches was to
> remove all downstream binaries from our test suite.
> 
> This test can be easily done with the current x86_64 binary
> TEST_UTILS_QEMU_BIN_X86_64.
> 

I thought the qemu-kvm patch was required to trigger the logic, but indeed
it's not necessary. I'll send a v2

Thanks,
Cole