On Thu, May 07, 2015 at 11:15:55AM -0400, Cole Robinson wrote:
> My commit 747761a79 (v1.2.15 only) dropped this bit of logic when filling
> in a default arch in the XML:
>
> - /* First try to find one matching host arch */
> - for (i = 0; i < caps->nguests; i++) {
> - if (caps->guests[i]->ostype == ostype) {
> - for (j = 0; j < caps->guests[i]->arch.ndomains; j++) {
> - if (caps->guests[i]->arch.domains[j]->type == domain &&
> - caps->guests[i]->arch.id == caps->host.arch)
> - return caps->guests[i]->arch.id;
> - }
> - }
> - }
>
> That attempt to match host.arch is important, otherwise we end up
> defaulting to i686 on x86_64 host for KVM, which is not intended.
> Duplicate it in the centralized CapsLookup function.
>
> Additionally add some testcases that would have caught this.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1219191
> ---
> v2:
> Tweak subject
> add test case for type=qemu as well
I didn't specifically test this patch, but I compared it to the
previous patch and all it does is to add / change the test suite.
Since the code is the same as before, and I tested the code in V1:
ACK.
Rich.
> src/conf/capabilities.c | 63 +++++++++++++++-------
> .../qemuxml2argv-default-kvm-host-arch.args | 4 ++
> .../qemuxml2argv-default-kvm-host-arch.xml | 11 ++++
> .../qemuxml2argv-default-qemu-host-arch.args | 4 ++
> .../qemuxml2argv-default-qemu-host-arch.xml | 11 ++++
> tests/qemuxml2argvtest.c | 2 +
> .../qemuxml2xmlout-default-kvm-host-arch.xml | 21 ++++++++
> .../qemuxml2xmlout-default-qemu-host-arch.xml | 21 ++++++++
> tests/qemuxml2xmltest.c | 2 +
> tests/testutilsqemu.c | 12 +++++
> 10 files changed, 132 insertions(+), 19 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-default-kvm-host-arch.args
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-default-kvm-host-arch.xml
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-default-qemu-host-arch.args
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-default-qemu-host-arch.xml
> create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-default-kvm-host-arch.xml
> create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-default-qemu-host-arch.xml
>
> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> index 922741f..c43bfb3 100644
> --- a/src/conf/capabilities.c
> +++ b/src/conf/capabilities.c
> @@ -607,25 +607,13 @@ virCapsDomainDataCompare(virCapsGuestPtr guest,
> return true;
> }
>
> -/**
> - * virCapabilitiesDomainDataLookup:
> - * @caps: capabilities to query
> - * @ostype: guest operating system type, of enum VIR_DOMAIN_OSTYPE
> - * @arch: Architecture to search for
> - * @domaintype: domain type to search for, of enum VIR_DOMAIN_VIRT
> - * @emulator: Emulator path to search for
> - * @machinetype: Machine type to search for
> - *
> - * Search capabilities for the passed values, and if found return
> - * virCapabilitiesDomainDataLookup filled in with the default values
> - */
> -virCapsDomainDataPtr
> -virCapabilitiesDomainDataLookup(virCapsPtr caps,
> - int ostype,
> - virArch arch,
> - int domaintype,
> - const char *emulator,
> - const char *machinetype)
> +static virCapsDomainDataPtr
> +virCapabilitiesDomainDataLookupInternal(virCapsPtr caps,
> + int ostype,
> + virArch arch,
> + int domaintype,
> + const char *emulator,
> + const char *machinetype)
> {
> virCapsGuestPtr foundguest = NULL;
> virCapsGuestDomainPtr founddomain = NULL;
> @@ -730,6 +718,43 @@ virCapabilitiesDomainDataLookup(virCapsPtr caps,
> return ret;
> }
>
> +/**
> + * virCapabilitiesDomainDataLookup:
> + * @caps: capabilities to query
> + * @ostype: guest operating system type, of enum VIR_DOMAIN_OSTYPE
> + * @arch: Architecture to search for
> + * @domaintype: domain type to search for, of enum VIR_DOMAIN_VIRT
> + * @emulator: Emulator path to search for
> + * @machinetype: Machine type to search for
> + *
> + * Search capabilities for the passed values, and if found return
> + * virCapabilitiesDomainDataLookup filled in with the default values
> + */
> +virCapsDomainDataPtr
> +virCapabilitiesDomainDataLookup(virCapsPtr caps,
> + int ostype,
> + virArch arch,
> + int domaintype,
> + const char *emulator,
> + const char *machinetype)
> +{
> + virCapsDomainDataPtr ret;
> +
> + if (arch == VIR_ARCH_NONE) {
> + /* Prefer host arch if its available */
> + ret = virCapabilitiesDomainDataLookupInternal(caps, ostype,
> + caps->host.arch,
> + domaintype,
> + emulator, machinetype);
> + if (ret)
> + return ret;
> + }
> +
> + return virCapabilitiesDomainDataLookupInternal(caps, ostype,
> + arch, domaintype,
> + emulator, machinetype);
> +}
> +
> static int
> virCapabilitiesFormatNUMATopology(virBufferPtr buf,
> size_t ncells,
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-default-kvm-host-arch.args b/tests/qemuxml2argvdata/qemuxml2argv-default-kvm-host-arch.args
> new file mode 100644
> index 0000000..102691f
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-default-kvm-host-arch.args
> @@ -0,0 +1,4 @@
> +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
> +/usr/bin/kvm -S -machine pc,accel=kvm -m 4096 -smp 4 -nographic \
> +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -net none \
> +-serial none -parallel none
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-default-kvm-host-arch.xml b/tests/qemuxml2argvdata/qemuxml2argv-default-kvm-host-arch.xml
> new file mode 100644
> index 0000000..66dead0
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-default-kvm-host-arch.xml
> @@ -0,0 +1,11 @@
> +<domain type='kvm'>
> + <name>kvm</name>
> + <uuid>d091ea82-29e6-2e34-3005-f02617b36e87</uuid>
> + <memory unit='KiB'>4194304</memory>
> + <currentMemory unit='KiB'>4194304</currentMemory>
> + <vcpu placement='static'>4</vcpu>
> + <os>
> + <type>hvm</type>
> + <boot dev='hd'/>
> + </os>
> +</domain>
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-default-qemu-host-arch.args b/tests/qemuxml2argvdata/qemuxml2argv-default-qemu-host-arch.args
> new file mode 100644
> index 0000000..5bd404c
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-default-qemu-host-arch.args
> @@ -0,0 +1,4 @@
> +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu-system-x86_64 -S -machine pc-0.11,accel=tcg -m 4096 -smp 4 \
> +-nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \
> +-usb -net none -serial none -parallel none
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-default-qemu-host-arch.xml b/tests/qemuxml2argvdata/qemuxml2argv-default-qemu-host-arch.xml
> new file mode 100644
> index 0000000..85ddec5
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-default-qemu-host-arch.xml
> @@ -0,0 +1,11 @@
> +<domain type='qemu'>
> + <name>qemu-host</name>
> + <uuid>d091ea82-29e6-2e34-3005-f02617b36e87</uuid>
> + <memory unit='KiB'>4194304</memory>
> + <currentMemory unit='KiB'>4194304</currentMemory>
> + <vcpu placement='static'>4</vcpu>
> + <os>
> + <type>hvm</type>
> + <boot dev='hd'/>
> + </os>
> +</domain>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 97c7fba..d317c9b 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -599,6 +599,8 @@ mymain(void)
> DO_TEST_FAILURE("machine-xen-vmport-opt", QEMU_CAPS_MACHINE_OPT,
> QEMU_CAPS_MACHINE_VMPORT_OPT);
> 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("boot-cdrom", NONE);
> DO_TEST("boot-network", NONE);
> DO_TEST("boot-floppy", NONE);
> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-default-kvm-host-arch.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-default-kvm-host-arch.xml
> new file mode 100644
> index 0000000..30fa66d
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-default-kvm-host-arch.xml
> @@ -0,0 +1,21 @@
> +<domain type='kvm'>
> + <name>kvm</name>
> + <uuid>d091ea82-29e6-2e34-3005-f02617b36e87</uuid>
> + <memory unit='KiB'>4194304</memory>
> + <currentMemory unit='KiB'>4194304</currentMemory>
> + <vcpu placement='static'>4</vcpu>
> + <os>
> + <type arch='x86_64' machine='pc'>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/kvm</emulator>
> + <controller type='usb' index='0'/>
> + <controller type='pci' index='0' model='pci-root'/>
> + <memballoon model='virtio'/>
> + </devices>
> +</domain>
> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-default-qemu-host-arch.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-default-qemu-host-arch.xml
> new file mode 100644
> index 0000000..3e65b97
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-default-qemu-host-arch.xml
> @@ -0,0 +1,21 @@
> +<domain type='qemu'>
> + <name>qemu-host</name>
> + <uuid>d091ea82-29e6-2e34-3005-f02617b36e87</uuid>
> + <memory unit='KiB'>4194304</memory>
> + <currentMemory unit='KiB'>4194304</currentMemory>
> + <vcpu placement='static'>4</vcpu>
> + <os>
> + <type arch='x86_64' machine='pc-0.11'>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-system-x86_64</emulator>
> + <controller type='usb' index='0'/>
> + <controller type='pci' index='0' model='pci-root'/>
> + <memballoon model='virtio'/>
> + </devices>
> +</domain>
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index b611afd..99f402c 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -346,6 +346,8 @@ mymain(void)
> DO_TEST("minimal");
> DO_TEST("machine-core-on");
> DO_TEST("machine-core-off");
> + DO_TEST_DIFFERENT("default-kvm-host-arch");
> + DO_TEST_DIFFERENT("default-qemu-host-arch");
> DO_TEST("boot-cdrom");
> DO_TEST("boot-network");
> DO_TEST("boot-floppy");
> diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
> index 14743be..d067bca 100644
> --- a/tests/testutilsqemu.c
> +++ b/tests/testutilsqemu.c
> @@ -354,6 +354,18 @@ virCapsPtr testQemuCapsInit(void)
> NULL) == NULL)
> goto cleanup;
>
> + if ((machines = testQemuAllocMachines(&nmachines)) == NULL)
> + goto cleanup;
> +
> + if (virCapabilitiesAddGuestDomain(guest,
> + VIR_DOMAIN_VIRT_KVM,
> + "/usr/bin/qemu-kvm",
> + NULL,
> + nmachines,
> + machines) == NULL)
> + goto cleanup;
> + machines = NULL;
> +
> if ((machines = testQemuAllocNewerMachines(&nmachines)) == NULL)
> goto cleanup;
>
> --
> 2.4.0
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list