6:43 p.m.
Just some minor patches to do with encoding a default CPU on the
qemu command line.
Patch 1 adds a test case
Patch 2 simplifies the code
Patch 3 throws an explicit error for a confusing situation
Cole Robinson (3):
tests: add qemu x86 kvm 32-on-64 test
qemu: command: rework adding of default cpu model
qemu: command: explicitly error for non-x86 default CPU
src/qemu/qemu_command.c | 95 +++++++++-------------
.../qemuxml2argvdata/qemuxml2argv-hyperv-off.args | 1 -
.../qemuxml2argv-kvm-features-off.args | 1 -
.../qemuxml2argv-x86-kvm-32-on-64.args | 21 +++++
.../qemuxml2argv-x86-kvm-32-on-64.xml | 13 +++
tests/qemuxml2argvtest.c | 1 +
tests/testutilsqemu.c | 18 +++-
7 files changed, 90 insertions(+), 60 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
--
2.13.3
6:43 p.m.
New subject: [libvirt] [PATCH 1/3] tests: add qemu x86 kvm 32-on-64 test
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(a)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",
};
@@ -215,6 +217,18 @@ testQemuAddI686Guest(virCapsPtr caps)
machines))
goto error;
+ machines = NULL;
+ if (!(machines = testQemuAllocMachines(&nmachines)))
+ goto error;
+
+ if (!virCapabilitiesAddGuestDomain(guest,
+ VIR_DOMAIN_VIRT_KVM,
+ QEMUBinList[TEST_UTILS_QEMU_BIN_KVM],
+ NULL,
+ nmachines,
+ machines))
+ goto error;
+
return 0;
error:
--
2.13.3
6:40 a.m.
New subject: [libvirt] [PATCH 1/3] tests: add qemu x86 kvm 32-on-64 test
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(a)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.
Pavel
};
@@ -215,6 +217,18 @@ testQemuAddI686Guest(virCapsPtr caps)
machines))
goto error;
+ machines = NULL;
+ if (!(machines = testQemuAllocMachines(&nmachines)))
+ goto error;
+
+ if (!virCapabilitiesAddGuestDomain(guest,
+ VIR_DOMAIN_VIRT_KVM,
+ QEMUBinList[TEST_UTILS_QEMU_BIN_KVM],
+ NULL,
+ nmachines,
+ machines))
+ goto error;
+
return 0;
error:
--
2.13.3
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
10:59 a.m.
New subject: [libvirt] [PATCH 1/3] tests: add qemu x86 kvm 32-on-64 test
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(a)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
6:43 p.m.
New subject: [libvirt] [PATCH 2/3] qemu: command: rework adding of default cpu model
Certain XML features that aren't in the <cpu> block map to -cpu
flags on the qemu cli. If one of these is specified but the user
didn't explicitly pass an XML <cpu> model, we need to format a
default model on the command line.
The current code handles this by sprinkling this default cpu handling
among all the different flag string formatting. Instead, switch it
to do this just once.
This alters some test output slightly: the previous code would
write the default -cpu in some cases when no flags were actually
added, so the output was redundant.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/qemu/qemu_command.c | 72 +++++++---------------
.../qemuxml2argvdata/qemuxml2argv-hyperv-off.args | 1 -
.../qemuxml2argv-kvm-features-off.args | 1 -
3 files changed, 22 insertions(+), 52 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b83261246..aa12479f7 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6714,11 +6714,11 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
virQEMUCapsPtr qemuCaps)
{
virArch hostarch = virArchFromHost();
- char *cpu = NULL;
+ char *cpu = NULL, *cpu_flags = NULL;
bool hasHwVirt = false;
const char *default_model;
- bool have_cpu = false;
int ret = -1;
+ virBuffer cpu_buf = VIR_BUFFER_INITIALIZER;
virBuffer buf = VIR_BUFFER_INITIALIZER;
size_t i;
@@ -6729,9 +6729,8 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
if (def->cpu &&
(def->cpu->mode != VIR_CPU_MODE_CUSTOM || def->cpu->model)) {
- if (qemuBuildCpuModelArgStr(driver, def, &buf, qemuCaps) < 0)
+ if (qemuBuildCpuModelArgStr(driver, def, &cpu_buf, qemuCaps) < 0)
goto cleanup;
- have_cpu = true;
/* Only 'svm' requires --enable-nesting. The nested 'vmx' patches
now
* simply hook off the CPU features. */
@@ -6769,8 +6768,7 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
((hostarch == VIR_ARCH_X86_64 &&
strstr(def->emulator, "kvm")) ||
strstr(def->emulator, "x86_64"))) {
- virBufferAdd(&buf, default_model, -1);
- have_cpu = true;
+ virBufferAdd(&cpu_buf, default_model, -1);
}
}
@@ -6780,21 +6778,14 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
if (timer->name == VIR_DOMAIN_TIMER_NAME_KVMCLOCK &&
timer->present != -1) {
- virBufferAsprintf(&buf, "%s,%ckvmclock",
- have_cpu ? "" : default_model,
+ virBufferAsprintf(&buf, ",%ckvmclock",
timer->present ? '+' : '-');
- have_cpu = true;
} else if (timer->name == VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK &&
timer->present == 1) {
- virBufferAsprintf(&buf, "%s,hv_time",
- have_cpu ? "" : default_model);
- have_cpu = true;
+ virBufferAddLit(&buf, ",hv_time");
} else if (timer->name == VIR_DOMAIN_TIMER_NAME_TSC &&
timer->frequency > 0) {
- virBufferAsprintf(&buf, "%s,tsc-frequency=%lu",
- have_cpu ? "" : default_model,
- timer->frequency);
- have_cpu = true;
+ virBufferAsprintf(&buf, ",tsc-frequency=%lu",
timer->frequency);
}
}
@@ -6805,10 +6796,7 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
else
sign = '-';
- virBufferAsprintf(&buf, "%s,%ckvm_pv_eoi",
- have_cpu ? "" : default_model,
- sign);
- have_cpu = true;
+ virBufferAsprintf(&buf, ",%ckvm_pv_eoi", sign);
}
if (def->features[VIR_DOMAIN_FEATURE_PVSPINLOCK]) {
@@ -6819,18 +6807,10 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
else
sign = '-';
- virBufferAsprintf(&buf, "%s,%ckvm_pv_unhalt",
- have_cpu ? "" : default_model,
- sign);
- have_cpu = true;
+ virBufferAsprintf(&buf, ",%ckvm_pv_unhalt", sign);
}
if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_TRISTATE_SWITCH_ON) {
- if (!have_cpu) {
- virBufferAdd(&buf, default_model, -1);
- have_cpu = true;
- }
-
for (i = 0; i < VIR_DOMAIN_HYPERV_LAST; i++) {
switch ((virDomainHyperv) i) {
case VIR_DOMAIN_HYPERV_RELAXED:
@@ -6866,22 +6846,12 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
for (i = 0; i < def->npanics; i++) {
if (def->panics[i]->model == VIR_DOMAIN_PANIC_MODEL_HYPERV) {
- if (!have_cpu) {
- virBufferAdd(&buf, default_model, -1);
- have_cpu = true;
- }
-
virBufferAddLit(&buf, ",hv_crash");
break;
}
}
if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) {
- if (!have_cpu) {
- virBufferAdd(&buf, default_model, -1);
- have_cpu = true;
- }
-
for (i = 0; i < VIR_DOMAIN_KVM_LAST; i++) {
switch ((virDomainKVM) i) {
case VIR_DOMAIN_KVM_HIDDEN:
@@ -6898,12 +6868,8 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
if (def->features[VIR_DOMAIN_FEATURE_PMU]) {
virTristateSwitch pmu = def->features[VIR_DOMAIN_FEATURE_PMU];
- if (!have_cpu)
- virBufferAdd(&buf, default_model, -1);
-
virBufferAsprintf(&buf, ",pmu=%s",
virTristateSwitchTypeToString(pmu));
- have_cpu = true;
}
if (def->cpu && def->cpu->cache) {
@@ -6911,11 +6877,6 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
bool hostOff = false;
bool l3Off = false;
- if (!have_cpu) {
- virBufferAdd(&buf, default_model, -1);
- have_cpu = true;
- }
-
switch (cache->mode) {
case VIR_CPU_CACHE_MODE_EMULATE:
virBufferAddLit(&buf, ",l3-cache=on");
@@ -6945,13 +6906,22 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
virBufferAddLit(&buf, ",l3-cache=off");
}
+ if (virBufferCheckError(&cpu_buf) < 0)
+ goto cleanup;
if (virBufferCheckError(&buf) < 0)
goto cleanup;
- cpu = virBufferContentAndReset(&buf);
+ cpu = virBufferContentAndReset(&cpu_buf);
+ cpu_flags = virBufferContentAndReset(&buf);
+
+ if (cpu_flags && !cpu) {
+ if (VIR_STRDUP(cpu, default_model) < 0)
+ goto cleanup;
+ }
if (cpu) {
- virCommandAddArgList(cmd, "-cpu", cpu, NULL);
+ virCommandAddArg(cmd, "-cpu");
+ virCommandAddArgFormat(cmd, "%s%s", cpu, cpu_flags ? cpu_flags :
"");
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NESTING) && hasHwVirt)
virCommandAddArg(cmd, "-enable-nesting");
@@ -6961,7 +6931,9 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
cleanup:
VIR_FREE(cpu);
+ VIR_FREE(cpu_flags);
virBufferFreeAndReset(&buf);
+ virBufferFreeAndReset(&cpu_buf);
return ret;
}
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hyperv-off.args
b/tests/qemuxml2argvdata/qemuxml2argv-hyperv-off.args
index e708feece..d1718d1f9 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-hyperv-off.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hyperv-off.args
@@ -8,7 +8,6 @@ QEMU_AUDIO_DRV=none \
-name QEMUGuest1 \
-S \
-M pc \
--cpu qemu32 \
-m 214 \
-smp 6,sockets=6,cores=1,threads=1 \
-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-kvm-features-off.args
b/tests/qemuxml2argvdata/qemuxml2argv-kvm-features-off.args
index e708feece..d1718d1f9 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-kvm-features-off.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-kvm-features-off.args
@@ -8,7 +8,6 @@ QEMU_AUDIO_DRV=none \
-name QEMUGuest1 \
-S \
-M pc \
--cpu qemu32 \
-m 214 \
-smp 6,sockets=6,cores=1,threads=1 \
-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
--
2.13.3
9:25 a.m.
New subject: [libvirt] [PATCH 2/3] qemu: command: rework adding of default cpu model
On Fri, Jul 14, 2017 at 07:43:05PM -0400, Cole Robinson wrote:
Certain XML features that aren't in the <cpu> block map to
-cpu
flags on the qemu cli. If one of these is specified but the user
didn't explicitly pass an XML <cpu> model, we need to format a
default model on the command line.
The current code handles this by sprinkling this default cpu handling
among all the different flag string formatting. Instead, switch it
to do this just once.
This alters some test output slightly: the previous code would
write the default -cpu in some cases when no flags were actually
added, so the output was redundant.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/qemu/qemu_command.c | 72 +++++++---------------
.../qemuxml2argvdata/qemuxml2argv-hyperv-off.args | 1 -
.../qemuxml2argv-kvm-features-off.args | 1 -
3 files changed, 22 insertions(+), 52 deletions(-)
ACK
Jan
6:43 p.m.
New subject: [libvirt] [PATCH 3/3] qemu: command: explicitly error for non-x86 default CPU
The code only currently handles writing an x86 default -cpu
argument, and doesn't know anything about other architectures.
Let's make this explicit rather than leaving ex. qemu ppc64 to
throw an error about -cpu qemu64
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/qemu/qemu_command.c | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index aa12479f7..f727e3d30 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6716,17 +6716,11 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
virArch hostarch = virArchFromHost();
char *cpu = NULL, *cpu_flags = NULL;
bool hasHwVirt = false;
- const char *default_model;
int ret = -1;
virBuffer cpu_buf = VIR_BUFFER_INITIALIZER;
virBuffer buf = VIR_BUFFER_INITIALIZER;
size_t i;
- if (def->os.arch == VIR_ARCH_I686)
- default_model = "qemu32";
- else
- default_model = "qemu64";
-
if (def->cpu &&
(def->cpu->mode != VIR_CPU_MODE_CUSTOM || def->cpu->model)) {
if (qemuBuildCpuModelArgStr(driver, def, &cpu_buf, qemuCaps) < 0)
@@ -6768,7 +6762,7 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
((hostarch == VIR_ARCH_X86_64 &&
strstr(def->emulator, "kvm")) ||
strstr(def->emulator, "x86_64"))) {
- virBufferAdd(&cpu_buf, default_model, -1);
+ virBufferAddLit(&cpu_buf, "qemu32");
}
}
@@ -6915,6 +6909,23 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
cpu_flags = virBufferContentAndReset(&buf);
if (cpu_flags && !cpu) {
+ const char *default_model;
+
+ switch (def->os.arch) {
+ case VIR_ARCH_I686:
+ default_model = "qemu32";
+ break;
+ case VIR_ARCH_X86_64:
+ default_model = "qemu64";
+ break;
+ default:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("CPU flags requested but can't determine "
+ "default CPU for arch %s"),
+ virArchToString(def->os.arch));
+ goto cleanup;
+ }
+
if (VIR_STRDUP(cpu, default_model) < 0)
goto cleanup;
}
--
2.13.3
11:40 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: command: explicitly error for non-x86 default CPU
On Fri, 2017-07-14 at 19:43 -0400, Cole Robinson wrote:
The code only currently handles writing an x86 default -cpu
argument, and doesn't know anything about other architectures.
Let's make this explicit rather than leaving ex. qemu ppc64 to
throw an error about -cpu qemu64
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
This seems to also fix
https://bugzilla.redhat.com/show_bug.cgi?id=1235511
I'll review the series early next week.
--
Andrea Bolognani / Red Hat / Virtualization
4:32 p.m.
New subject: [libvirt] [PATCH 3/3] qemu: command: explicitly error for non-x86 default CPU
On 07/15/2017 12:40 PM, Andrea Bolognani wrote:
On Fri, 2017-07-14 at 19:43 -0400, Cole Robinson wrote:
> The code only currently handles writing an x86 default -cpu
> argument, and doesn't know anything about other architectures.
> Let's make this explicit rather than leaving ex. qemu ppc64 to
> throw an error about -cpu qemu64
>
> Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
This seems to also fix
https://bugzilla.redhat.com/show_bug.cgi?id=1235511
I'll review the series early next week.
Thanks, but I don't think it will fix pmu=on for ppc64, just improve the error
scenario. We will still need to extend the code to hardcode the default CPU
model for PPC64 (would be nice if qemu had a generic 'default' value to use
here, or some way to get around specifying a cpu model)
- Cole
9:25 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: command: explicitly error for non-x86 default CPU
On Fri, Jul 14, 2017 at 07:43:06PM -0400, Cole Robinson wrote:
The code only currently handles writing an x86 default -cpu
argument, and doesn't know anything about other architectures.
Let's make this explicit rather than leaving ex. qemu ppc64 to
throw an error about -cpu qemu64
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/qemu/qemu_command.c | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
ACK
Jan
10:56 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: command: explicitly error for non-x86 default CPU
On 07/21/2017 10:25 AM, Ján Tomko wrote:
On Fri, Jul 14, 2017 at 07:43:06PM -0400, Cole Robinson wrote:
> The code only currently handles writing an x86 default -cpu
> argument, and doesn't know anything about other architectures.
> Let's make this explicit rather than leaving ex. qemu ppc64 to
> throw an error about -cpu qemu64
>
> Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
> ---
> src/qemu/qemu_command.c | 25 ++++++++++++++++++-------
> 1 file changed, 18 insertions(+), 7 deletions(-)
>
ACK
Jan
Thanks, I pushed patch 2 + 3 now which you reviewed, patch 1 was independent
and has some comments.
- Cole
2668
days inactive
2688
days old
10 comments
4 participants
participants (4)
-
Andrea Bolognani -
Cole Robinson -
Ján Tomko -
Pavel Hrdina