[libvirt] [PATCH 0/3] qemu: support disabling/enabling kvmclock with <timer> elements
QEMU supports a bunch of CPUID features that are tied to the kvm CPUID nodes rather than the processor's. They are "kvmclock", "kvm_nopiodelay", "kvm_mmu", "kvm_asyncpf". These are not known to libvirt and their CPUID leaf might move if (for example) the Hyper-V extensions are enabled. Hence their handling would anyway require some special-casing. However, among these the most useful is kvmclock; an additional "property" of this feature is that a <timer> element is a better model than a CPUID feature. This is what this series does. Patch 1 tweaks the -cpu parsing code so that it detects the "default" CPU names and (instead of creating a <cpu> element) sets the guest arch to either i686 or x86_64; this is useful and helped writing testcases. The next two patches add the new feature. Finally, patch 4 tweaks the default CPU model to be more precise when KVM is enabled. Paolo Bonzini (4): qemu: get arch name from <cpu> element conf: add kvmclock timer qemu: parse and create -cpu ...,-kvmclock qemu: default to kvm32/kvm64 when KVM is enabled docs/formatdomain.html.in | 4 +- docs/schemas/domaincommon.rng | 3 +- src/conf/domain_conf.c | 3 +- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 138 ++++++++++++++++---- tests/qemuargv2xmltest.c | 4 + tests/qemuxml2argvdata/qemu-lib.sh | 1 + .../qemuxml2argv-cpu-host-kvmclock.args | 4 + .../qemuxml2argv-cpu-host-kvmclock.xml | 23 ++++ .../qemuxml2argv-cpu-kvmclock.args | 4 + .../qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.xml | 24 ++++ tests/qemuxml2argvdata/qemuxml2argv-kvmclock.args | 4 + tests/qemuxml2argvdata/qemuxml2argv-kvmclock.xml | 21 +++ tests/qemuxml2argvtest.c | 3 + tests/qemuxml2xmltest.c | 3 + 15 files changed, 209 insertions(+), 31 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-host-kvmclock.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-host-kvmclock.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvmclock.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvmclock.xml -- 1.7.7.1
The qemu32 CPU model is chosen based on the <os arch=...> name when creating the QEMU command line. Reflect the kvm32/kvm64/qemu32/qemu64 CPU models in the <os> element when doing the opposite transformation. To do this, we need to not look at def->os.arch until after the command-line has been parsed. At the same time, avoid creating an empty <cpu> element when the QEMU command-line specifies the default CPU model for the guest arch. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- src/qemu/qemu_command.c | 70 +++++++++++++++++++++++++++++++---------------- 1 files changed, 46 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index aaccf62..7c4460e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6779,12 +6779,10 @@ static int qemuParseCommandLineCPU(virDomainDefPtr dom, const char *val) { - virCPUDefPtr cpu; + virCPUDefPtr cpu = NULL; const char *p = val; const char *next; - - if (!(cpu = qemuInitGuestCPU(dom))) - goto error; + char *model = NULL; do { if (*p == '\0' || *p == ',') @@ -6793,13 +6791,13 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, if ((next = strchr(p, ','))) next++; - if (!cpu->model) { + if (!model) { if (next) - cpu->model = strndup(p, next - p - 1); + model = strndup(p, next - p - 1); else - cpu->model = strdup(p); + model = strdup(p); - if (!cpu->model) + if (!model) goto no_memory; } else if (*p == '+' || *p == '-') { @@ -6824,6 +6822,9 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, if (!feature) goto no_memory; + if (!cpu && !(cpu = qemuInitGuestCPU(dom))) + goto error; + ret = virCPUDefAddFeature(cpu, feature, policy); VIR_FREE(feature); if (ret < 0) @@ -6831,6 +6832,21 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, } } while ((p = next)); + if (model) { + if (STREQ(model, "qemu32") || STREQ(model, "kvm32")) { + dom->os.arch = strdup("i686"); + VIR_FREE(model); + } else if (STREQ(model, "qemu64") || STREQ(model, "kvm64")) { + dom->os.arch = strdup("x86_64"); + VIR_FREE(model); + } else { + if (!cpu && !(cpu = qemuInitGuestCPU(dom))) + goto error; + + cpu->model = model; + } + } + return 0; syntax: @@ -6942,6 +6958,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, const char **nics = NULL; int video = VIR_DOMAIN_VIDEO_TYPE_CIRRUS; int nvirtiodisk = 0; + int disabled_features = 0; qemuDomainCmdlineDefPtr cmd = NULL; virDomainDiskDefPtr disk = NULL; const char *ceph_args = qemuFindEnv(progenv, "CEPH_ARGS"); @@ -7000,21 +7017,6 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, if (!def->os.type) goto no_memory; - if (STRPREFIX(def->emulator, "qemu")) - path = def->emulator; - else - path = strstr(def->emulator, "qemu"); - if (path && - STRPREFIX(path, "qemu-system-")) - def->os.arch = strdup(path + strlen("qemu-system-")); - else - def->os.arch = strdup("i686"); - if (!def->os.arch) - goto no_memory; - - if (STREQ(def->os.arch, "i686")||STREQ(def->os.arch, "x86_64")) - def->features = (1 << VIR_DOMAIN_FEATURE_ACPI) - /*| (1 << VIR_DOMAIN_FEATURE_APIC)*/; #define WANT_VALUE() \ const char *val = progargv[++i]; \ if (!val) { \ @@ -7246,7 +7248,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, def->disks[def->ndisks++] = disk; disk = NULL; } else if (STREQ(arg, "-no-acpi")) { - def->features &= ~(1 << VIR_DOMAIN_FEATURE_ACPI); + disabled_features |= (1 << VIR_DOMAIN_FEATURE_ACPI); } else if (STREQ(arg, "-no-reboot")) { def->onReboot = VIR_DOMAIN_LIFECYCLE_DESTROY; } else if (STREQ(arg, "-no-kvm")) { @@ -7542,6 +7544,26 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, } } + if (!def->os.arch) { + if (STRPREFIX(def->emulator, "qemu")) + path = def->emulator; + else + path = strstr(def->emulator, "qemu"); + if (path && + STRPREFIX(path, "qemu-system-")) + def->os.arch = strdup(path + strlen("qemu-system-")); + else + def->os.arch = strdup("i686"); + if (!def->os.arch) + goto no_memory; + } + + if (STREQ(def->os.arch, "i686")||STREQ(def->os.arch, "x86_64")) + def->features = (1 << VIR_DOMAIN_FEATURE_ACPI) + /*| (1 << VIR_DOMAIN_FEATURE_APIC)*/; + + def->features &= ~disabled_features; + #undef WANT_VALUE if (def->ndisks > 0 && ceph_args) { char *hosts, *port, *saveptr = NULL, *token; -- 1.7.7.1
On Mon, Jan 23, 2012 at 14:11:08 +0100, Paolo Bonzini wrote:
The qemu32 CPU model is chosen based on the <os arch=...> name when creating the QEMU command line. Reflect the kvm32/kvm64/qemu32/qemu64 CPU models in the <os> element when doing the opposite transformation. To do this, we need to not look at def->os.arch until after the command-line has been parsed.
At the same time, avoid creating an empty <cpu> element when the QEMU command-line specifies the default CPU model for the guest arch.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- src/qemu/qemu_command.c | 70 +++++++++++++++++++++++++++++++---------------- 1 files changed, 46 insertions(+), 24 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index aaccf62..7c4460e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c ... @@ -6831,6 +6832,21 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, } } while ((p = next));
+ if (model) { + if (STREQ(model, "qemu32") || STREQ(model, "kvm32")) { + dom->os.arch = strdup("i686"); + VIR_FREE(model); + } else if (STREQ(model, "qemu64") || STREQ(model, "kvm64")) { + dom->os.arch = strdup("x86_64"); + VIR_FREE(model); + } else { + if (!cpu && !(cpu = qemuInitGuestCPU(dom))) + goto error; + + cpu->model = model; + } + } +
I think we could just set cpu->model even if the model used in qemu command line was {qemu,kvm}{32,64}. That would probably mean we need to add some of the models in cpu_map.xml, though.
return 0;
syntax:
...
@@ -7542,6 +7544,26 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, } }
+ if (!def->os.arch) { + if (STRPREFIX(def->emulator, "qemu")) + path = def->emulator; + else + path = strstr(def->emulator, "qemu"); + if (path && + STRPREFIX(path, "qemu-system-")) + def->os.arch = strdup(path + strlen("qemu-system-")); + else + def->os.arch = strdup("i686"); + if (!def->os.arch) + goto no_memory; + } + + if (STREQ(def->os.arch, "i686")||STREQ(def->os.arch, "x86_64")) + def->features = (1 << VIR_DOMAIN_FEATURE_ACPI) + /*| (1 << VIR_DOMAIN_FEATURE_APIC)*/;
I know you were just copy&pasting, but while doing that, you could also make this last statement a bit more readable and add spaces around "||" and either use {} or remove the last line (the comment).
+ + def->features &= ~disabled_features; + #undef WANT_VALUE if (def->ndisks > 0 && ceph_args) { char *hosts, *port, *saveptr = NULL, *token;
Otherwise the patch looks good. Jirka
On 01/23/2012 04:52 PM, Jiri Denemark wrote:
I think we could just set cpu->model even if the model used in qemu command line was {qemu,kvm}{32,64}. That would probably mean we need to add some of the models in cpu_map.xml, though.
Actually I was doing that on purpose, not just for laziness. :) The idea is that, if you were using -cpu just to set "-cpu {qemu,kvm}{32,64},-kvmclock", you will not get an irrelevant <cpu><model>qemu32</model></cpu> in domxml-from-native output. Paolo
On Mon, Jan 23, 2012 at 17:21:01 +0100, Paolo Bonzini wrote:
On 01/23/2012 04:52 PM, Jiri Denemark wrote:
I think we could just set cpu->model even if the model used in qemu command line was {qemu,kvm}{32,64}. That would probably mean we need to add some of the models in cpu_map.xml, though.
Actually I was doing that on purpose, not just for laziness. :)
The idea is that, if you were using -cpu just to set "-cpu {qemu,kvm}{32,64},-kvmclock", you will not get an irrelevant <cpu><model>qemu32</model></cpu> in domxml-from-native output.
Yeah, I understand why you did so, however, you just discard {qemu,kvm}{32,64} models even if there were more flags than just +-kvmclock specified, don't you? Anyway, <cpu><model>qemu32</model></cpu> doesn't seem irrelevant to me, it just describes the guest visible hardware. Jirka
On 01/24/2012 02:28 PM, Jiri Denemark wrote:
The idea is that, if you were using -cpu just to set "-cpu {qemu,kvm}{32,64},-kvmclock", you will not get an irrelevant <cpu><model>qemu32</model></cpu> in domxml-from-native output. Yeah, I understand why you did so, however, you just discard {qemu,kvm}{32,64} models even if there were more flags than just +-kvmclock specified, don't you?
Right, that gives "Non-empty feature list specified without CPU model" when you later start the domain. I'll fix this.
Anyway,<cpu><model>qemu32</model></cpu> doesn't seem irrelevant to me, it just describes the guest visible hardware.
Yeah, but arch="i686" says the same (kvm32/kvm64 shouldn't be considered by this code, only qemu32/qemu64). Paolo
Add kvmclock timer to documentation, schema and parsers. Keep the platform timer first since it is kind of special, and alphabetize the others when possible (i.e. when it does not change the ABI). Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- docs/formatdomain.html.in | 4 ++-- docs/schemas/domaincommon.rng | 3 ++- src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 1 + 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index de9b480..963811a 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -926,8 +926,8 @@ <dt><code>name</code></dt> <dd> The <code>name</code> attribute selects which timer is - being modified, and can be one of "platform", "pit", - "rtc", "hpet", or "tsc". + being modified, and can be one of "platform", "hpet", + "kvmclock", "pit", "rtc", or "tsc". </dd> <dt><code>track</code></dt> <dd> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 2041dfb..f59bf60 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -568,9 +568,10 @@ <attribute name="name"> <choice> <value>platform</value> + <value>hpet</value> + <value>kvmclock</value> <value>pit</value> <value>rtc</value> - <value>hpet</value> <value>tsc</value> </choice> </attribute> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8eed85b..7b059fa 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -576,7 +576,8 @@ VIR_ENUM_IMPL(virDomainTimerName, VIR_DOMAIN_TIMER_NAME_LAST, "pit", "rtc", "hpet", - "tsc"); + "tsc", + "kvmclock"); VIR_ENUM_IMPL(virDomainTimerTrack, VIR_DOMAIN_TIMER_TRACK_LAST, "boot", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a49795c..39c34f5 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1285,6 +1285,7 @@ enum virDomainTimerNameType { VIR_DOMAIN_TIMER_NAME_RTC, VIR_DOMAIN_TIMER_NAME_HPET, VIR_DOMAIN_TIMER_NAME_TSC, + VIR_DOMAIN_TIMER_NAME_KVMCLOCK, VIR_DOMAIN_TIMER_NAME_LAST, }; -- 1.7.7.1
On Mon, Jan 23, 2012 at 14:11:09 +0100, Paolo Bonzini wrote:
Add kvmclock timer to documentation, schema and parsers. Keep the platform timer first since it is kind of special, and alphabetize the others when possible (i.e. when it does not change the ABI).
Any chance the "clock" part in "kvmclock" is redundant since we're setting a timer? I'm not suggesting either way, I'm just asking whether it would make sense to name the new timer "kvm" instead of "kvmclock".
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- docs/formatdomain.html.in | 4 ++-- docs/schemas/domaincommon.rng | 3 ++- src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 1 + 4 files changed, 7 insertions(+), 4 deletions(-)
ACK regardless on the answer to the question above. Jirka
On 01/23/2012 05:05 PM, Jiri Denemark wrote:
Add kvmclock timer to documentation, schema and parsers. Keep the platform timer first since it is kind of special, and alphabetize the others when possible (i.e. when it does not change the ABI).
Any chance the "clock" part in "kvmclock" is redundant since we're setting a timer? I'm not suggesting either way, I'm just asking whether it would make sense to name the new timer "kvm" instead of "kvmclock".
I think it's worthwhile to be consistent with the no-kvmclock and clock=kvm-clock kernel command line options. Which are inconsistent between themselves. :) Paolo
Creating part of the -cpu command-line from something other than the <cpu> XML element introduces some ugliness. --- src/qemu/qemu_command.c | 72 ++++++++++++++++++-- tests/qemuargv2xmltest.c | 4 + .../qemuxml2argv-cpu-host-kvmclock.args | 4 + .../qemuxml2argv-cpu-host-kvmclock.xml | 23 ++++++ .../qemuxml2argv-cpu-kvmclock.args | 4 + .../qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.xml | 24 +++++++ tests/qemuxml2argvdata/qemuxml2argv-kvmclock.args | 4 + tests/qemuxml2argvdata/qemuxml2argv-kvmclock.xml | 21 ++++++ tests/qemuxml2argvtest.c | 3 + tests/qemuxml2xmltest.c | 3 + 10 files changed, 157 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-host-kvmclock.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-host-kvmclock.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvmclock.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvmclock.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7c4460e..24e3adf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3500,7 +3500,9 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, virCPUDefPtr cpu = NULL; unsigned int ncpus = 0; const char **cpus = NULL; + const char *default_model; union cpuData *data = NULL; + bool have_cpu = false; int ret = -1; virBuffer buf = VIR_BUFFER_INITIALIZER; int i; @@ -3517,6 +3519,11 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, goto cleanup; } + if (STREQ(def->os.arch, "i686")) + default_model = "qemu32"; + else + default_model = "qemu64"; + if (cpu) { virCPUCompareResult cmp; const char *preferred; @@ -3594,6 +3601,7 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, virBufferAsprintf(&buf, ",%c%s", sign, guest->features[i].name); } } + have_cpu = true; } else { /* * Need to force a 32-bit guest CPU type if @@ -3610,8 +3618,26 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, if (STREQ(def->os.arch, "i686") && ((STREQ(ut->machine, "x86_64") && strstr(emulator, "kvm")) || - strstr(emulator, "x86_64"))) - virBufferAddLit(&buf, "qemu32"); + strstr(emulator, "x86_64"))) { + virBufferAdd(&buf, default_model, -1); + have_cpu = true; + } + } + + /* Now force kvmclock on/off based on the corresponding <timer> element. */ + for (i = 0; i < def->clock.ntimers; i++) { + if (def->clock.timers[i]->name == VIR_DOMAIN_TIMER_NAME_KVMCLOCK && + def->clock.timers[i]->present != -1) { + char sign; + if (def->clock.timers[i]->present) + sign = '+'; + else + sign = '-'; + virBufferAsprintf(&buf, "%s,%ckvmclock", + have_cpu ? "" : default_model, + sign); + break; + } } if (virBufferError(&buf)) @@ -4091,6 +4117,10 @@ qemuBuildCommandLine(virConnectPtr conn, virDomainTimerNameTypeToString(def->clock.timers[i]->name)); goto error; + case VIR_DOMAIN_TIMER_NAME_KVMCLOCK: + /* This is handled when building -cpu. */ + break; + case VIR_DOMAIN_TIMER_NAME_RTC: /* This has already been taken care of (in qemuBuildClockArgStr) if QEMU_CAPS_RTC is set (mutually exclusive with @@ -6822,10 +6852,42 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, if (!feature) goto no_memory; - if (!cpu && !(cpu = qemuInitGuestCPU(dom))) - goto error; + if (STREQ(feature, "kvmclock")) { + bool present = (policy == VIR_CPU_FEATURE_REQUIRE); + int i; + + for (i = 0; i < dom->clock.ntimers; i++) { + if (dom->clock.timers[i]->name == VIR_DOMAIN_TIMER_NAME_KVMCLOCK) { + break; + } + } + + if (i == dom->clock.ntimers) { + if (VIR_REALLOC_N(dom->clock.timers, i+1) < 0 || + VIR_ALLOC(dom->clock.timers[i]) < 0) + goto no_memory; + dom->clock.timers[i]->name = VIR_DOMAIN_TIMER_NAME_KVMCLOCK; + dom->clock.timers[i]->present = -1; + dom->clock.timers[i]->tickpolicy = -1; + dom->clock.timers[i]->track = -1; + dom->clock.ntimers++; + } + + if (dom->clock.timers[i]->present != -1 && + dom->clock.timers[i]->present != present) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("multiple occurrences of kvmclock feature")); + goto error; + } + dom->clock.timers[i]->present = present; + ret = 0; + } else { + if (!cpu && !(cpu = qemuInitGuestCPU(dom))) + goto error; + + ret = virCPUDefAddFeature(cpu, feature, policy); + } - ret = virCPUDefAddFeature(cpu, feature, policy); VIR_FREE(feature); if (ret < 0) goto error; diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index cb3cafd..28434ff 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -144,6 +144,10 @@ mymain(void) DO_TEST("boot-cdrom"); DO_TEST("boot-network"); DO_TEST("boot-floppy"); + DO_TEST("kvmclock"); + /* This needs <emulator>./qemu.sh</emulator> which doesn't work here. */ + /*DO_TEST("cpu-kvmclock");*/ + /* Can't roundtrip xenner arch */ /*DO_TEST("bootloader");*/ DO_TEST("clock-utc"); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-kvmclock.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-kvmclock.args new file mode 100644 index 0000000..6a5df28 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-kvmclock.args @@ -0,0 +1,4 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test ./qemu.sh -S -M pc \ +-cpu host,-kvmclock -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-host-kvmclock.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-kvmclock.xml new file mode 100644 index 0000000..15a9e44 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-kvmclock.xml @@ -0,0 +1,23 @@ +<domain type='kvm'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219100</memory> + <currentMemory>219100</currentMemory> + <vcpu>6</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='network'/> + </os> + <cpu mode='host-passthrough'> + </cpu> + <clock offset='utc'> + <timer name='kvmclock' present='no'/> + </clock> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/./qemu.sh</emulator> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.args new file mode 100644 index 0000000..f7c33fb --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.args @@ -0,0 +1,4 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test ./qemu.sh -S -M pc \ +-cpu core2duo,-kvmclock -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-kvmclock.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.xml new file mode 100644 index 0000000..e8b0bb1 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.xml @@ -0,0 +1,24 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219100</memory> + <currentMemory>219100</currentMemory> + <vcpu>6</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='network'/> + </os> + <cpu mode='custom' match='exact'> + <model fallback='allow'>core2duo</model> + </cpu> + <clock offset='utc'> + <timer name='kvmclock' present='no'/> + </clock> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/./qemu.sh</emulator> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-kvmclock.args b/tests/qemuxml2argvdata/qemuxml2argv-kvmclock.args new file mode 100644 index 0000000..4c7aa3b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-kvmclock.args @@ -0,0 +1,4 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu-kvm -S -M pc \ +-cpu qemu32,-kvmclock -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-kvmclock.xml b/tests/qemuxml2argvdata/qemuxml2argv-kvmclock.xml new file mode 100644 index 0000000..2e090ae --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-kvmclock.xml @@ -0,0 +1,21 @@ +<domain type='kvm'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219100</memory> + <currentMemory>219100</currentMemory> + <vcpu>6</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='network'/> + </os> + <clock offset='utc'> + <timer name='kvmclock' present='no'/> + </clock> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-kvm</emulator> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index d40b37e..ddb4369 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -385,6 +385,9 @@ mymain(void) DO_TEST("clock-variable", false, QEMU_CAPS_RTC); */ DO_TEST("clock-france", false, QEMU_CAPS_RTC); + DO_TEST("cpu-kvmclock", false, NONE); + DO_TEST("cpu-host-kvmclock", false, QEMU_CAPS_KVM, QEMU_CAPS_CPU_HOST); + DO_TEST("kvmclock", false, NONE); DO_TEST("hugepages", false, QEMU_CAPS_MEM_PATH); DO_TEST("disk-cdrom", false, NONE); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 293c2a7..caff5c6 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -124,6 +124,9 @@ mymain(void) DO_TEST("bootloader"); DO_TEST("clock-utc"); DO_TEST("clock-localtime"); + DO_TEST("cpu-kvmclock"); + DO_TEST("cpu-host-kvmclock"); + DO_TEST("kvmclock"); DO_TEST("hugepages"); DO_TEST("disk-aio"); DO_TEST("disk-cdrom"); -- 1.7.7.1
On Mon, Jan 23, 2012 at 14:11:10 +0100, Paolo Bonzini wrote:
Creating part of the -cpu command-line from something other than the <cpu> XML element introduces some ugliness.
Well, that's what we get for creating virtual CPUID features :-)
--- src/qemu/qemu_command.c | 72 ++++++++++++++++++-- tests/qemuargv2xmltest.c | 4 + .../qemuxml2argv-cpu-host-kvmclock.args | 4 + .../qemuxml2argv-cpu-host-kvmclock.xml | 23 ++++++ .../qemuxml2argv-cpu-kvmclock.args | 4 + .../qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.xml | 24 +++++++ tests/qemuxml2argvdata/qemuxml2argv-kvmclock.args | 4 + tests/qemuxml2argvdata/qemuxml2argv-kvmclock.xml | 21 ++++++ tests/qemuxml2argvtest.c | 3 + tests/qemuxml2xmltest.c | 3 + 10 files changed, 157 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-host-kvmclock.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-host-kvmclock.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvmclock.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvmclock.xml
Thanks for creating tests right away :-) ACK Jirka
The qemu32/qemu64 models are weird in that the exact combination of CPUID flags does not match any actual processor. kvm32 and kvm64 are a better match when not using TCG. Use them when -cpu is only needed to hardcode a 32-bit guest arch or for kvmclock. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- src/qemu/qemu_command.c | 4 ++-- tests/qemuxml2argvdata/qemu-lib.sh | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 24e3adf..236d779 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3520,9 +3520,9 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, } if (STREQ(def->os.arch, "i686")) - default_model = "qemu32"; + default_model = (def->virtType == VIR_DOMAIN_VIRT_QEMU ? "qemu32" : "kvm32"); else - default_model = "qemu64"; + default_model = (def->virtType == VIR_DOMAIN_VIRT_QEMU ? "qemu64" : "kvm64"); if (cpu) { virCPUCompareResult cmp; diff --git a/tests/qemuxml2argvdata/qemu-lib.sh b/tests/qemuxml2argvdata/qemu-lib.sh index ba19119..098a110 100644 --- a/tests/qemuxml2argvdata/qemu-lib.sh +++ b/tests/qemuxml2argvdata/qemu-lib.sh @@ -41,6 +41,7 @@ x86 [pentium] x86 [486] x86 [coreduo] x86 [qemu32] +x86 [kvm32] x86 [kvm64] x86 [core2duo] x86 [phenom] -- 1.7.7.1
On Mon, Jan 23, 2012 at 14:11:11 +0100, Paolo Bonzini wrote:
The qemu32/qemu64 models are weird in that the exact combination of CPUID flags does not match any actual processor. kvm32 and kvm64 are a better match when not using TCG. Use them when -cpu is only needed to hardcode a 32-bit guest arch or for kvmclock.
I don't think we can do this as it means the guest CPU may change unexpectedly for existing domains. A 32b domain started on current libvirt would see qemu32, while the same domain started after this patch would see kvm32. Jirka
On Mon, Jan 23, 2012 at 04:57:21PM +0100, Jiri Denemark wrote:
On Mon, Jan 23, 2012 at 14:11:11 +0100, Paolo Bonzini wrote:
The qemu32/qemu64 models are weird in that the exact combination of CPUID flags does not match any actual processor. kvm32 and kvm64 are a better match when not using TCG. Use them when -cpu is only needed to hardcode a 32-bit guest arch or for kvmclock.
I don't think we can do this as it means the guest CPU may change unexpectedly for existing domains. A 32b domain started on current libvirt would see qemu32, while the same domain started after this patch would see kvm32.
Also, IIUC, kvm32 is a fairly newly introduced CPU type for KVM - ie most deployments of KVM won't support it. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 01/23/2012 05:03 PM, Daniel P. Berrange wrote:
The qemu32/qemu64 models are weird in that the exact combination of CPUID flags does not match any actual processor. kvm32 and kvm64 are a better match when not using TCG. Use them when -cpu is only needed to hardcode a 32-bit guest arch or for kvmclock.
I don't think we can do this as it means the guest CPU may change unexpectedly for existing domains. A 32b domain started on current libvirt would see qemu32, while the same domain started after this patch would see kvm32. Also, IIUC, kvm32 is a fairly newly introduced CPU type for KVM - ie most deployments of KVM won't support it.
Ok, I'll redo this patch with just kvm64. As the wrong subject in 0/3 show, it was an afterthought. As soon as I get the review of 3/4, I'll see whether I need to resubmit the whole series or just this patch. Paolo
On Mon, Jan 23, 2012 at 05:23:32PM +0100, Paolo Bonzini wrote:
On 01/23/2012 05:03 PM, Daniel P. Berrange wrote:
The qemu32/qemu64 models are weird in that the exact combination of CPUID flags does not match any actual processor. kvm32 and kvm64 are a better match when not using TCG. Use them when -cpu is only needed to hardcode a 32-bit guest arch or for kvmclock.
I don't think we can do this as it means the guest CPU may change unexpectedly for existing domains. A 32b domain started on current libvirt would see qemu32, while the same domain started after this patch would see kvm32. Also, IIUC, kvm32 is a fairly newly introduced CPU type for KVM - ie most deployments of KVM won't support it.
Ok, I'll redo this patch with just kvm64. As the wrong subject in 0/3 show, it was an afterthought.
Is kvm64 actually any more widely supported than kvm32 ? I though they were all fairly new. If I'm wrong, then we could trivially provide a kvm32, by using the kvm64 model, and subtracting the the "long mode" CPU flag. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 01/23/2012 05:30 PM, Daniel P. Berrange wrote:
On Mon, Jan 23, 2012 at 05:23:32PM +0100, Paolo Bonzini wrote:
On 01/23/2012 05:03 PM, Daniel P. Berrange wrote:
> The qemu32/qemu64 models are weird in that the exact combination of > CPUID flags does not match any actual processor. kvm32 and kvm64 are > a better match when not using TCG. Use them when -cpu is only needed > to hardcode a 32-bit guest arch or for kvmclock.
I don't think we can do this as it means the guest CPU may change unexpectedly for existing domains. A 32b domain started on current libvirt would see qemu32, while the same domain started after this patch would see kvm32. Also, IIUC, kvm32 is a fairly newly introduced CPU type for KVM - ie most deployments of KVM won't support it.
Ok, I'll redo this patch with just kvm64. As the wrong subject in 0/3 show, it was an afterthought.
Is kvm64 actually any more widely supported than kvm32 ? I though they were all fairly new.
If I'm wrong, then we could trivially provide a kvm32, by using the kvm64 model, and subtracting the the "long mode" CPU flag.
commit d1cd4bf41906980434bee1c6656e8aeb9a132df3 Author: Andre Przywara <andre.przywara@amd.com> Date: Thu Aug 20 23:34:17 2009 +0200 introduce kvm64 CPU In addition to the TCG based qemu64 type let's introduce a kvm64 CPU type, which is the least common denominator of all KVM-capable x86-CPUs (based on Intel Pentium 4 Prescott). It can be used as a base type for migration. Signed-off-by: Andre Przywara <andre.przywara@amd.com> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> It is in RHEL6, but let's just scrap this patch until (at least) after 0.9.10. Paolo
participants (3)
-
Daniel P. Berrange -
Jiri Denemark -
Paolo Bonzini