[libvirt] [PATCH 0/6] CPU selection fixes and tests

4 out of the 8 tests added by patch 2/6 fail with current libvirt. After 5/6 some of them pass and some of them fail in a different way. After 6/6 all of them pass. Jirka Jiri Denemark (6): Fake host CPU for qemu tests Tests for CPU selection in qemu driver Deal with CPU models in [] Move MIN macro to util.h so that others can use it Support removing features when converting data to CPU Use configured CPU model if possible src/cpu/cpu.c | 8 +- src/cpu/cpu.h | 6 +- src/cpu/cpu_x86.c | 82 +++++++++++++++----- src/qemu/qemu_conf.c | 39 ++++++++-- src/util/util.c | 4 - src/util/util.h | 4 + .../qemuxml2argvdata/qemuxml2argv-cpu-exact1.args | 1 + tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.xml | 28 +++++++ .../qemuxml2argvdata/qemuxml2argv-cpu-exact2.args | 1 + tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.xml | 35 ++++++++ .../qemuxml2argv-cpu-minimum1.args | 1 + .../qemuxml2argvdata/qemuxml2argv-cpu-minimum1.xml | 21 +++++ .../qemuxml2argv-cpu-minimum2.args | 1 + .../qemuxml2argvdata/qemuxml2argv-cpu-minimum2.xml | 25 ++++++ .../qemuxml2argvdata/qemuxml2argv-cpu-strict1.args | 1 + .../qemuxml2argvdata/qemuxml2argv-cpu-strict1.xml | 38 +++++++++ .../qemuxml2argv-cpu-topology1.args | 1 + .../qemuxml2argv-cpu-topology1.xml | 21 +++++ .../qemuxml2argv-cpu-topology2.args | 1 + .../qemuxml2argv-cpu-topology2.xml | 22 +++++ .../qemuxml2argv-cpu-topology3.args | 1 + .../qemuxml2argv-cpu-topology3.xml | 21 +++++ tests/qemuxml2argvtest.c | 9 ++ tests/testutilsqemu.c | 30 +++++++- 24 files changed, 362 insertions(+), 39 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum1.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum1.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum2.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum2.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-strict1.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-strict1.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-topology1.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-topology1.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-topology2.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-topology2.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-topology3.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-topology3.xml

--- tests/testutilsqemu.c | 30 +++++++++++++++++++++++++++++- 1 files changed, 29 insertions(+), 1 deletions(-) diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 8dd26d4..e0e5e14 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -6,6 +6,7 @@ # include "testutilsqemu.h" # include "testutils.h" # include "memory.h" +# include "cpu_conf.h" static virCapsGuestMachinePtr *testQemuAllocMachines(int *nmachines) { @@ -62,13 +63,40 @@ virCapsPtr testQemuCapsInit(void) { static const char *const xen_machines[] = { "xenner" }; + static virCPUFeatureDef host_cpu_features[] = { + { (char *) "lahf_lm", -1 }, + { (char *) "xtpr", -1 }, + { (char *) "cx16", -1 }, + { (char *) "tm2", -1 }, + { (char *) "est", -1 }, + { (char *) "vmx", -1 }, + { (char *) "ds_cpl", -1 }, + { (char *) "pbe", -1 }, + { (char *) "tm", -1 }, + { (char *) "ht", -1 }, + { (char *) "ss", -1 }, + { (char *) "acpi", -1 }, + { (char *) "ds", -1 } + }; + static virCPUDef host_cpu = { + VIR_CPU_TYPE_HOST, /* type */ + 0, /* match */ + (char *) "x86_64", /* arch */ + (char *) "core2duo", /* model */ + 1, /* sockets */ + 2, /* cores */ + 1, /* threads */ + ARRAY_CARDINALITY(host_cpu_features), /* nfeatures */ + host_cpu_features /* features */ + }; uname (&utsname); if ((caps = virCapabilitiesNew(utsname.machine, 0, 0)) == NULL) return NULL; - if ((machines = testQemuAllocMachines(&nmachines)) == NULL) + if ((caps->host.cpu = virCPUDefCopy(&host_cpu)) == NULL || + (machines = testQemuAllocMachines(&nmachines)) == NULL) goto cleanup; if ((guest = virCapabilitiesAddGuest(caps, "hvm", "i686", 32, -- 1.7.0.4

On 04/16/2010 11:01 AM, Jiri Denemark wrote:
--- tests/testutilsqemu.c | 30 +++++++++++++++++++++++++++++- 1 files changed, 29 insertions(+), 1 deletions(-)
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 8dd26d4..e0e5e14 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -6,6 +6,7 @@ # include "testutilsqemu.h" # include "testutils.h" # include "memory.h" +# include "cpu_conf.h"
...
- if ((machines = testQemuAllocMachines(&nmachines)) == NULL) + if ((caps->host.cpu = virCPUDefCopy(&host_cpu)) == NULL || + (machines = testQemuAllocMachines(&nmachines)) == NULL) goto cleanup;
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

--- .../qemuxml2argvdata/qemuxml2argv-cpu-exact1.args | 1 + tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.xml | 28 ++++++++++++++ .../qemuxml2argvdata/qemuxml2argv-cpu-exact2.args | 1 + tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.xml | 35 ++++++++++++++++++ .../qemuxml2argv-cpu-minimum1.args | 1 + .../qemuxml2argvdata/qemuxml2argv-cpu-minimum1.xml | 21 +++++++++++ .../qemuxml2argv-cpu-minimum2.args | 1 + .../qemuxml2argvdata/qemuxml2argv-cpu-minimum2.xml | 25 +++++++++++++ .../qemuxml2argvdata/qemuxml2argv-cpu-strict1.args | 1 + .../qemuxml2argvdata/qemuxml2argv-cpu-strict1.xml | 38 ++++++++++++++++++++ .../qemuxml2argv-cpu-topology1.args | 1 + .../qemuxml2argv-cpu-topology1.xml | 21 +++++++++++ .../qemuxml2argv-cpu-topology2.args | 1 + .../qemuxml2argv-cpu-topology2.xml | 22 +++++++++++ .../qemuxml2argv-cpu-topology3.args | 1 + .../qemuxml2argv-cpu-topology3.xml | 21 +++++++++++ tests/qemuxml2argvtest.c | 9 +++++ 17 files changed, 228 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum1.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum1.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum2.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum2.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-strict1.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-strict1.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-topology1.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-topology1.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-topology2.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-topology2.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-topology3.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-topology3.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.args new file mode 100644 index 0000000..e48672c --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -cpu qemu64,-svm,-lm,-nx,-syscall,-clflush,-pse36,-mca -m 214 -smp 6 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot n -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.xml new file mode 100644 index 0000000..2538b1d --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.xml @@ -0,0 +1,28 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu>6</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='network'/> + </os> + <cpu match='exact'> + <model>qemu64</model> + <feature policy='disable' name='svm'/> + <feature policy='disable' name='lm'/> + <feature policy='disable' name='nx'/> + <feature policy='disable' name='syscall'/> + <feature policy='disable' name='clflush'/> + <feature policy='disable' name='pse36'/> + <feature policy='disable' name='mca'/> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.args new file mode 100644 index 0000000..d418d51 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -cpu core2duo,+lahf_lm,+3dnowext,+xtpr,+ds_cpl,+tm,+ht,+ds,-nx -m 214 -smp 6 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot n -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.xml new file mode 100644 index 0000000..db25a72 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.xml @@ -0,0 +1,35 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu>6</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='network'/> + </os> + <cpu match='exact'> + <model>core2duo</model> + <feature name='lahf_lm' policy='require'/> + <feature name='xtpr' policy='require'/> + <feature name='cx16' policy='disable'/> + <feature name='tm2' policy='disable'/> + <feature name='ds_cpl' policy='require'/> + <feature name='pbe' policy='disable'/> + <feature name='tm' policy='optional'/> + <feature name='ht' policy='require'/> + <feature name='ss' policy='disable'/> + <feature name='ds' policy='require'/> + <feature name='nx' policy='disable'/> + <feature name='3dnowext' policy='force'/> + <feature name='sse4a' policy='optional'/> + <feature name='wdt' policy='forbid'/> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum1.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum1.args new file mode 100644 index 0000000..06009ae --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum1.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -cpu core2duo,+lahf_lm,+xtpr,+cx16,+tm2,+est,+vmx,+ds_cpl,+pbe,+tm,+ht,+ss,+acpi,+ds -m 214 -smp 6 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot n -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum1.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum1.xml new file mode 100644 index 0000000..db9ad2f --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum1.xml @@ -0,0 +1,21 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu>6</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='network'/> + </os> + <cpu match='minimum'> + <model>486</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum2.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum2.args new file mode 100644 index 0000000..e2bc2e9 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum2.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -cpu core2duo,+lahf_lm,+xtpr,+cx16,+tm2,+est,+vmx,+ds_cpl,+pbe,+tm,+ht,+ss,+acpi,+ds,-lm,-nx,-syscall -m 214 -smp 6 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot n -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum2.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum2.xml new file mode 100644 index 0000000..0c89a26 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum2.xml @@ -0,0 +1,25 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu>6</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='network'/> + </os> + <cpu match='minimum'> + <model>qemu64</model> + <feature policy='disable' name='svm'/> + <feature policy='disable' name='lm'/> + <feature policy='disable' name='nx'/> + <feature policy='disable' name='syscall'/> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-strict1.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-strict1.args new file mode 100644 index 0000000..07cc0d7 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-strict1.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -cpu core2duo,+lahf_lm,+3dnowext,+xtpr,+est,+vmx,+ds_cpl,+tm,+ht,+acpi,+ds,-nx -m 214 -smp 6 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot n -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-strict1.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-strict1.xml new file mode 100644 index 0000000..6dacd9e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-strict1.xml @@ -0,0 +1,38 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu>6</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='network'/> + </os> + <cpu match='strict'> + <model>core2duo</model> + <feature name='lahf_lm' policy='require'/> + <feature name='xtpr' policy='require'/> + <feature name='cx16' policy='disable'/> + <feature name='tm2' policy='disable'/> + <feature name='est' policy='optional'/> + <feature name='vmx' policy='optional'/> + <feature name='ds_cpl' policy='require'/> + <feature name='pbe' policy='disable'/> + <feature name='tm' policy='optional'/> + <feature name='ht' policy='require'/> + <feature name='ss' policy='disable'/> + <feature name='acpi' policy='optional'/> + <feature name='ds' policy='require'/> + <feature name='nx' policy='disable'/> + <feature name='3dnowext' policy='force'/> + <feature name='sse4a' policy='optional'/> + <feature name='wdt' policy='forbid'/> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology1.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology1.args new file mode 100644 index 0000000..43e1d43 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology1.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 6,sockets=3,cores=2,threads=1 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot n -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology1.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology1.xml new file mode 100644 index 0000000..e31ea4b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology1.xml @@ -0,0 +1,21 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu>6</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='network'/> + </os> + <cpu> + <topology sockets="3" cores="2" threads="1"/> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology2.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology2.args new file mode 100644 index 0000000..f3d8f01 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology2.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -cpu core2duo -m 214 -smp 6,sockets=1,cores=2,threads=3 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot n -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology2.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology2.xml new file mode 100644 index 0000000..8bdad2c --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology2.xml @@ -0,0 +1,22 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu>6</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='network'/> + </os> + <cpu match='exact'> + <model>core2duo</model> + <topology sockets="1" cores="2" threads="3"/> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology3.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology3.args new file mode 100644 index 0000000..210bcdf --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology3.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 6 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot n -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology3.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology3.xml new file mode 100644 index 0000000..e31ea4b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology3.xml @@ -0,0 +1,21 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu>6</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='network'/> + </os> + <cpu> + <topology sockets="3" cores="2" threads="1"/> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 9e4d5bf..beb5dcd 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -346,6 +346,15 @@ mymain(int argc, char **argv) DO_TEST_FULL("restore-v2", QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC, "exec:cat"); DO_TEST_FULL("migrate", QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP, "tcp:10.0.0.1:5000"); + DO_TEST("cpu-topology1", QEMUD_CMD_FLAG_SMP_TOPOLOGY); + DO_TEST("cpu-topology2", QEMUD_CMD_FLAG_SMP_TOPOLOGY); + DO_TEST("cpu-topology3", 0); + DO_TEST("cpu-minimum1", 0); + DO_TEST("cpu-minimum2", 0); + DO_TEST("cpu-exact1", 0); + DO_TEST("cpu-exact2", 0); + DO_TEST("cpu-strict1", 0); + free(driver.stateDir); virCapabilitiesFree(driver.caps); -- 1.7.0.4

On 04/16/2010 11:01 AM, Jiri Denemark wrote:
+++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.xml @@ -0,0 +1,28 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.xml @@ -0,0 +1,35 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
Is reusing the same UUID going to bite us later, especially when running tests in parallel?
+++ b/tests/qemuxml2argvtest.c @@ -346,6 +346,15 @@ mymain(int argc, char **argv) DO_TEST_FULL("restore-v2", QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC, "exec:cat"); DO_TEST_FULL("migrate", QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP, "tcp:10.0.0.1:5000");
+ DO_TEST("cpu-topology1", QEMUD_CMD_FLAG_SMP_TOPOLOGY); + DO_TEST("cpu-topology2", QEMUD_CMD_FLAG_SMP_TOPOLOGY); + DO_TEST("cpu-topology3", 0); + DO_TEST("cpu-minimum1", 0); + DO_TEST("cpu-minimum2", 0); + DO_TEST("cpu-exact1", 0); + DO_TEST("cpu-exact2", 0); + DO_TEST("cpu-strict1", 0);
I didn't spot anything blatantly unusual, although I'm not exactly sure what I would be looking for. Then again, I am assuming that since everything passes when the entire series is applied, it certainly isn't breaking anything to add new tests. So, ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

+++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.xml @@ -0,0 +1,28 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.xml @@ -0,0 +1,35 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
Is reusing the same UUID going to bite us later, especially when running tests in parallel?
Quite likely in case we start running tests in parallel. But now we don't start them in parallel and all XMLs in qemuxml2argvdata use the exact same UUID and name so reusing them seems to be consistent. Jirka

Qemu committed a patch which list some CPU names in [] when asked for supported CPUs (qemu -cpu ?). Yet, it needs such CPUs to be passed without those square braces. When probing for supported CPU models, we can just strip the square braces and pretend we have never seen them. --- src/qemu/qemu_conf.c | 17 +++++++++++++---- 1 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 0cbedf2..8577ff1 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -628,7 +628,9 @@ typedef int const char ***retcpus); /* Format: - * <arch> <model> + * <arch> <model> + * recent qemu encloses some model names in []: + * <arch> [<model>] */ static int qemudParseX86Models(const char *output, @@ -661,15 +663,22 @@ qemudParseX86Models(const char *output, continue; if (retcpus) { + unsigned int len; + if (VIR_REALLOC_N(cpus, count + 1) < 0) goto error; if (next) - cpus[count] = strndup(p, next - p - 1); + len = next - p - 1; else - cpus[count] = strdup(p); + len = strlen(p); + + if (len > 2 && *p == '[' && p[len - 1] == ']') { + p++; + len -= 2; + } - if (!cpus[count]) + if (!(cpus[count] = strndup(p, len))) goto error; } count++; -- 1.7.0.4

On 04/16/2010 11:01 AM, Jiri Denemark wrote:
Qemu committed a patch which list some CPU names in [] when asked for supported CPUs (qemu -cpu ?). Yet, it needs such CPUs to be passed without those square braces. When probing for supported CPU models, we can just strip the square braces and pretend we have never seen them. --- src/qemu/qemu_conf.c | 17 +++++++++++++---- 1 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 0cbedf2..8577ff1 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -628,7 +628,9 @@ typedef int const char ***retcpus);
/* Format: - * <arch> <model> + * <arch> <model> + * recent qemu encloses some model names in []:
This comment can go out of date. Is it better to write something a bit more time neutral, as in: qemu version x.y.z encloses some model names in []:
+ * <arch> [<model>] */ static int qemudParseX86Models(const char *output, @@ -661,15 +663,22 @@ qemudParseX86Models(const char *output, continue;
if (retcpus) { + unsigned int len; + if (VIR_REALLOC_N(cpus, count + 1) < 0) goto error;
if (next) - cpus[count] = strndup(p, next - p - 1); + len = next - p - 1; else - cpus[count] = strdup(p); + len = strlen(p); + + if (len > 2 && *p == '[' && p[len - 1] == ']') { + p++; + len -= 2; + }
- if (!cpus[count]) + if (!(cpus[count] = strndup(p, len))) goto error;
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

--- src/util/util.c | 4 ---- src/util/util.h | 4 ++++ 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index 268944d..99383d1 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -79,10 +79,6 @@ # define NSIG 32 #endif -#ifndef MIN -# define MIN(a, b) ((a) < (b) ? (a) : (b)) -#endif - #define VIR_FROM_THIS VIR_FROM_NONE #define virUtilError(code, ...) \ diff --git a/src/util/util.h b/src/util/util.h index 4f0b233..6bf6bcc 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -32,6 +32,10 @@ # include <sys/select.h> # include <sys/types.h> +# ifndef MIN +# define MIN(a, b) ((a) < (b) ? (a) : (b)) +# endif + int saferead(int fd, void *buf, size_t count) ATTRIBUTE_RETURN_CHECK; ssize_t safewrite(int fd, const void *buf, size_t count) ATTRIBUTE_RETURN_CHECK; -- 1.7.0.4

On 04/16/2010 11:01 AM, Jiri Denemark wrote:
--- src/util/util.c | 4 ---- src/util/util.h | 4 ++++ 2 files changed, 4 insertions(+), 4 deletions(-)
+++ b/src/util/util.h @@ -32,6 +32,10 @@ # include <sys/select.h> # include <sys/types.h>
+# ifndef MIN +# define MIN(a, b) ((a) < (b) ? (a) : (b)) +# endif
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

So far, when CPUID data were converted into CPU model and features, the features can only be added to the model. As a result, when a guest asked for something like "qemu64,-svm" it would get a qemu32 plus a bunch of additional features instead. This patch adds support for removing feature from the base model. Selection algorithm remains the same: the best CPU model is the model which requires lowest number of features to be added/removed from it. --- src/cpu/cpu_x86.c | 71 ++++++++++++++++++++++++++++++++++++------------- src/qemu/qemu_conf.c | 12 +++++++- 2 files changed, 62 insertions(+), 21 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 205528d..014fe7d 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -27,6 +27,7 @@ #include "logging.h" #include "memory.h" +#include "utils.h" #include "cpu.h" #include "cpu_map.h" #include "cpu_x86.h" @@ -211,6 +212,27 @@ x86DataCopy(const union cpuData *data) } +static void +x86DataSubtract(union cpuData *data1, + const union cpuData *data2) +{ + unsigned int i; + unsigned int len; + + len = MIN(data1->x86.basic_len, data2->x86.basic_len); + for (i = 0; i < len; i++) { + x86cpuidClearBits(data1->x86.basic + i, + data2->x86.basic + i); + } + + len = MIN(data1->x86.extended_len, data2->x86.extended_len); + for (i = 0; i < len; i++) { + x86cpuidClearBits(data1->x86.extended + i, + data2->x86.extended + i); + } +} + + static union cpuData * x86DataFromModel(const struct x86_model *model) { @@ -282,24 +304,28 @@ x86DataToCPU(const union cpuData *data, const struct x86_map *map) { virCPUDefPtr cpu; - union cpuData *tmp = NULL; - unsigned int i; + union cpuData *copy = NULL; + union cpuData *modelData = NULL; if (VIR_ALLOC(cpu) < 0 || - (cpu->model = strdup(model->name)) == NULL || - (tmp = x86DataCopy(data)) == NULL) + !(cpu->model = strdup(model->name)) || + !(copy = x86DataCopy(data)) || + !(modelData = x86DataFromModel(model))) goto no_memory; - for (i = 0; i < model->ncpuid; i++) { - x86cpuidClearBits(x86DataCpuid(tmp, model->cpuid[i].function), - model->cpuid + i); - } + x86DataSubtract(copy, modelData); + x86DataSubtract(modelData, data); + + /* because feature policy is ignored for host CPU */ + cpu->type = VIR_CPU_TYPE_GUEST; - if (x86DataToCPUFeatures(cpu, VIR_CPU_FEATURE_REQUIRE, tmp, map)) + if (x86DataToCPUFeatures(cpu, VIR_CPU_FEATURE_REQUIRE, copy, map) || + x86DataToCPUFeatures(cpu, VIR_CPU_FEATURE_DISABLE, modelData, map)) goto error; cleanup: - x86DataFree(tmp); + x86DataFree(modelData); + x86DataFree(copy); return cpu; no_memory: @@ -1045,8 +1071,7 @@ x86Decode(virCPUDefPtr cpu, const struct x86_model *candidate; virCPUDefPtr cpuCandidate; virCPUDefPtr cpuModel = NULL; - struct cpuX86cpuid *cpuid; - int i; + unsigned int i; if (data == NULL || (map = x86LoadMap()) == NULL) return -1; @@ -1055,13 +1080,6 @@ x86Decode(virCPUDefPtr cpu, while (candidate != NULL) { bool allowed = (models == NULL); - for (i = 0; i < candidate->ncpuid; i++) { - cpuid = x86DataCpuid(data, candidate->cpuid[i].function); - if (cpuid == NULL - || !x86cpuidMatchMasked(cpuid, candidate->cpuid + i)) - goto next; - } - for (i = 0; i < nmodels; i++) { if (models && models[i] && STREQ(models[i], candidate->name)) { allowed = true; @@ -1078,6 +1096,19 @@ x86Decode(virCPUDefPtr cpu, if (!(cpuCandidate = x86DataToCPU(data, candidate, map))) goto out; + if (cpu->type == VIR_CPU_TYPE_HOST) { + cpuCandidate->type = VIR_CPU_TYPE_HOST; + for (i = 0; i < cpuCandidate->nfeatures; i++) { + switch (cpuCandidate->features[i].policy) { + case VIR_CPU_FEATURE_DISABLE: + virCPUDefFree(cpuCandidate); + goto next; + default: + cpuCandidate->features[i].policy = -1; + } + } + } + if (cpuModel == NULL || cpuModel->nfeatures > cpuCandidate->nfeatures) { virCPUDefFree(cpuModel); @@ -1310,6 +1341,8 @@ x86Baseline(virCPUDefPtr *cpus, if (VIR_ALLOC(cpu) < 0 || !(cpu->arch = strdup(cpus[0]->arch))) goto no_memory; + cpu->type = VIR_CPU_TYPE_GUEST; + cpu->match = VIR_CPU_MATCH_EXACT; for (i = 1; i < ncpus; i++) { struct x86_model *model; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 8577ff1..a1392fa 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -3309,12 +3309,20 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, if (VIR_ALLOC(guest) < 0 || !(guest->arch = strdup(ut->machine))) goto no_memory; + guest->type = VIR_CPU_TYPE_GUEST; if (cpuDecode(guest, data, cpus, ncpus) < 0) goto cleanup; virBufferVSprintf(&buf, "%s", guest->model); - for (i = 0; i < guest->nfeatures; i++) - virBufferVSprintf(&buf, ",+%s", guest->features[i].name); + for (i = 0; i < guest->nfeatures; i++) { + char sign; + if (guest->features[i].policy == VIR_CPU_FEATURE_DISABLE) + sign = '-'; + else + sign = '+'; + + virBufferVSprintf(&buf, ",%c%s", sign, guest->features[i].name); + } } else { /* -- 1.7.0.4

On 04/16/2010 11:01 AM, Jiri Denemark wrote:
So far, when CPUID data were converted into CPU model and features, the features can only be added to the model. As a result, when a guest asked for something like "qemu64,-svm" it would get a qemu32 plus a bunch of additional features instead.
This patch adds support for removing feature from the base model. Selection algorithm remains the same: the best CPU model is the model which requires lowest number of features to be added/removed from it.
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

Adds ability to provide a preferred CPU model for CPUID data decoding. Such model would be considered as the best possible model (if it's supported by hypervisor) regardless on number of features which have to be added or removed for describing required CPU. --- src/cpu/cpu.c | 8 +++++--- src/cpu/cpu.h | 6 ++++-- src/cpu/cpu_x86.c | 11 +++++++++-- src/qemu/qemu_conf.c | 10 ++++++++-- 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 4a1588d..580b767 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -127,11 +127,13 @@ int cpuDecode(virCPUDefPtr cpu, const union cpuData *data, const char **models, - unsigned int nmodels) + unsigned int nmodels, + const char *preferred) { struct cpuArchDriver *driver; - VIR_DEBUG("cpu=%p, data=%p, nmodels=%u", cpu, data, nmodels); + VIR_DEBUG("cpu=%p, data=%p, nmodels=%u, preferred=%s", + cpu, data, nmodels, NULLSTR(preferred)); if (models) { unsigned int i; for (i = 0; i < nmodels; i++) @@ -160,7 +162,7 @@ cpuDecode(virCPUDefPtr cpu, return -1; } - return driver->decode(cpu, data, models, nmodels); + return driver->decode(cpu, data, models, nmodels, preferred); } diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index 1494a7f..40f2a7d 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -49,7 +49,8 @@ typedef int (*cpuArchDecode) (virCPUDefPtr cpu, const union cpuData *data, const char **models, - unsigned int nmodels); + unsigned int nmodels, + const char *preferred); typedef int (*cpuArchEncode) (const virCPUDefPtr cpu, @@ -108,7 +109,8 @@ extern int cpuDecode (virCPUDefPtr cpu, const union cpuData *data, const char **models, - unsigned int nmodels); + unsigned int nmodels, + const char *preferred); extern int cpuEncode (const char *arch, diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 014fe7d..90ea509 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1064,7 +1064,8 @@ static int x86Decode(virCPUDefPtr cpu, const union cpuData *data, const char **models, - unsigned int nmodels) + unsigned int nmodels, + const char *preferred) { int ret = -1; struct x86_map *map; @@ -1109,6 +1110,12 @@ x86Decode(virCPUDefPtr cpu, } } + if (preferred && STREQ(cpuCandidate->model, preferred)) { + virCPUDefFree(cpuModel); + cpuModel = cpuCandidate; + break; + } + if (cpuModel == NULL || cpuModel->nfeatures > cpuCandidate->nfeatures) { virCPUDefFree(cpuModel); @@ -1356,7 +1363,7 @@ x86Baseline(virCPUDefPtr *cpus, if (!(data = x86DataFromModel(base_model))) goto no_memory; - if (x86Decode(cpu, data, models, nmodels) < 0) + if (x86Decode(cpu, data, models, nmodels, NULL) < 0) goto error; cleanup: diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index a1392fa..e6e6490 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1029,7 +1029,7 @@ qemudCapsInitCPU(virCapsPtr caps, cpu->threads = nodeinfo.threads; if (!(data = cpuNodeData(arch)) - || cpuDecode(cpu, data, NULL, 0) < 0) + || cpuDecode(cpu, data, NULL, 0, NULL) < 0) goto error; caps->host.cpu = cpu; @@ -3292,6 +3292,7 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, if (ncpus > 0 && host) { virCPUCompareResult cmp; + const char *preferred; cmp = cpuGuestData(host, def->cpu, &data); switch (cmp) { @@ -3309,8 +3310,13 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, if (VIR_ALLOC(guest) < 0 || !(guest->arch = strdup(ut->machine))) goto no_memory; + if (def->cpu->match == VIR_CPU_MATCH_MINIMUM) + preferred = host->model; + else + preferred = def->cpu->model; + guest->type = VIR_CPU_TYPE_GUEST; - if (cpuDecode(guest, data, cpus, ncpus) < 0) + if (cpuDecode(guest, data, cpus, ncpus, preferred) < 0) goto cleanup; virBufferVSprintf(&buf, "%s", guest->model); -- 1.7.0.4

On 04/16/2010 11:01 AM, Jiri Denemark wrote:
Adds ability to provide a preferred CPU model for CPUID data decoding. Such model would be considered as the best possible model (if it's supported by hypervisor) regardless on number of features which have to be added or removed for describing required CPU.
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On 04/16/2010 11:01 AM, Jiri Denemark wrote:
4 out of the 8 tests added by patch 2/6 fail with current libvirt. After 5/6 some of them pass and some of them fail in a different way. After 6/6 all of them pass.
I would suggest that before pushing, you reorder the patches so that 'make check' passes for every stage of the patch series. This makes patch bisection easier in the future (if we are trying to hunt down a regression, it's better if every commit builds independently). Admittedly, it can look a bit odd seeing the commit that fixes the bug before the commit that introduces the test, even though they were developed in the opposite order, but you get used to it. And 'git rebase -i' makes it so easy to do. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

I would suggest that before pushing, you reorder the patches so that 'make check' passes for every stage of the patch series. This makes patch bisection easier in the future (if we are trying to hunt down a regression, it's better if every commit builds independently). Admittedly, it can look a bit odd seeing the commit that fixes the bug before the commit that introduces the test, even though they were developed in the opposite order, but you get used to it. And 'git rebase -i' makes it so easy to do.
Well, to be honest, I first fixed the bug and then created the tests to check I don't regress somewhere else, to check the bug is really fix and for the future :-) I reordered the patches before posting as they seemed more logical this way. But I can reorder them back, no problem. Jirka

Eric, Thanks for reviewing this patchset but I have to respin it as make check would not pass on RHEL (basically on any system where there is no /usr/bin/qemu). I'll send version 2 soon. Jirka
participants (2)
-
Eric Blake
-
Jiri Denemark