[libvirt] [PATCH v2 0/7] 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. Actually, while working on this I noticed that it is quite difficult to have round-trip testcases involving -cpu, that can be used with both qemuargv2xmltest and qemuxml2argvtest. This is what the first five four patches do. The first patch is unrelated. The second patch updates the CPU feature map with default CPUs, and is a prerequisite for patch 3. That one makes the -cpu parsing code detect the "lm" CPU feature, so that the guest arch is correctly set to either i686 or x86_64. The fifth patch is also mostly needed for testing. It notices the default models qemu32/qemu64 and does not generate <cpu> elements; this happens with "-cpu qemu32,-kvmclock" for example. Then, the last two patches actually add the new feature. Paolo Bonzini (7): qemu: parse -enable-kvm x86: add kvm32 and kvm64, update qemu64 qemu: detect arch correctly for KVM qemu: get arch name from <cpu> element qemu: do not create useless <cpu> element conf: add kvmclock timer qemu: parse and create -cpu ...,-kvmclock v1->v2: completely redone first patch (which became patches 1-5 :) docs/formatdomain.html.in | 4 +- docs/schemas/domaincommon.rng | 3 +- src/conf/domain_conf.c | 3 +- src/conf/domain_conf.h | 1 + src/cpu/cpu_map.xml | 31 +++++ src/qemu/qemu_command.c | 129 ++++++++++++++++++-- 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 + 15 files changed, 244 insertions(+), 17 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.6
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- src/qemu/qemu_command.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9b69ad0..1e3e8ef 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7250,6 +7250,8 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, def->onReboot = VIR_DOMAIN_LIFECYCLE_DESTROY; } else if (STREQ(arg, "-no-kvm")) { def->virtType = VIR_DOMAIN_VIRT_QEMU; + } else if (STREQ(arg, "-enable-kvm")) { + def->virtType = VIR_DOMAIN_VIRT_KVM; } else if (STREQ(arg, "-nographic")) { nographics = 1; } else if (STREQ(arg, "-full-screen")) { -- 1.7.7.6
On Fri, Jan 27, 2012 at 14:49:46 +0100, Paolo Bonzini wrote:
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- src/qemu/qemu_command.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9b69ad0..1e3e8ef 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7250,6 +7250,8 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, def->onReboot = VIR_DOMAIN_LIFECYCLE_DESTROY; } else if (STREQ(arg, "-no-kvm")) { def->virtType = VIR_DOMAIN_VIRT_QEMU; + } else if (STREQ(arg, "-enable-kvm")) { + def->virtType = VIR_DOMAIN_VIRT_KVM; } else if (STREQ(arg, "-nographic")) { nographics = 1; } else if (STREQ(arg, "-full-screen")) {
ACK Jirka
Recently (or not so recently) QEMU added the kvm32 and kvm64 architectures, representing a least common denominator of all hosts that can run KVM. Add them to the machine map. Also, some features that TCG supports were added to qemu64. Add them to the cpu_map.xml whenever KVM is guaranteed to support those. We still have to leave some out, because they would not be available to guests running on older hosts. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- src/cpu/cpu_map.xml | 31 +++++++++++++++++++++++++++++++ 1 files changed, 31 insertions(+), 0 deletions(-) diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml index 693caf1..3e6810f 100644 --- a/src/cpu/cpu_map.xml +++ b/src/cpu/cpu_map.xml @@ -303,6 +303,15 @@ <feature name='pni'/> </model> + <model name='kvm32'> + <model name='pentiumpro'/> + <feature name='mtrr'/> + <feature name='clflush'/> + <feature name='mca'/> + <feature name='pse36'/> + <feature name='pni'/> + </model> + <model name='coreduo'> <model name='pentiumpro'/> <feature name='vme'/> @@ -321,10 +330,32 @@ <feature name='mca'/> <feature name='pse36'/> <feature name='pni'/> + <feature name='cx16'/> <feature name='lm'/> <feature name='syscall'/> <feature name='nx'/> <feature name='svm'/> + <!-- These are supported only by TCG. KVM supports them only if the + host does. So we leave them out: + + <feature name='popcnt'/> + <feature name='lahf_lm'/> + <feature name='sse4a'/> + <feature name='abm'/> + --> + </model> + + <model name='kvm64'> + <model name='pentiumpro'/> + <feature name='mtrr'/> + <feature name='clflush'/> + <feature name='mca'/> + <feature name='pse36'/> + <feature name='pni'/> + <feature name='cx16'/> + <feature name='lm'/> + <feature name='syscall'/> + <feature name='nx'/> </model> <model name='core2duo'> -- 1.7.7.6
On Fri, Jan 27, 2012 at 14:49:47 +0100, Paolo Bonzini wrote:
Recently (or not so recently) QEMU added the kvm32 and kvm64 architectures, representing a least common denominator of all hosts that can run KVM. Add them to the machine map.
Also, some features that TCG supports were added to qemu64. Add them to the cpu_map.xml whenever KVM is guaranteed to support those. We still have to leave some out, because they would not be available to guests running on older hosts.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- src/cpu/cpu_map.xml | 31 +++++++++++++++++++++++++++++++ 1 files changed, 31 insertions(+), 0 deletions(-)
ACK Jirka
When running under KVM, the arch is usually set to i686 because the name of the emulator is not qemu-system-x86_64. Use the host arch instead. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- src/qemu/qemu_command.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1e3e8ef..fc337f7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7003,8 +7003,10 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, path = def->emulator; else path = strstr(def->emulator, "qemu"); - if (path && - STRPREFIX(path, "qemu-system-")) + if (def->virtType == VIR_DOMAIN_VIRT_KVM) + def->os.arch = strdup(caps->host.cpu->arch); + else if (path && + STRPREFIX(path, "qemu-system-")) def->os.arch = strdup(path + strlen("qemu-system-")); else def->os.arch = strdup("i686"); -- 1.7.7.6
On Fri, Jan 27, 2012 at 14:49:48 +0100, Paolo Bonzini wrote:
When running under KVM, the arch is usually set to i686 because the name of the emulator is not qemu-system-x86_64. Use the host arch instead.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- src/qemu/qemu_command.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1e3e8ef..fc337f7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7003,8 +7003,10 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, path = def->emulator; else path = strstr(def->emulator, "qemu"); - if (path && - STRPREFIX(path, "qemu-system-")) + if (def->virtType == VIR_DOMAIN_VIRT_KVM) + def->os.arch = strdup(caps->host.cpu->arch); + else if (path && + STRPREFIX(path, "qemu-system-")) def->os.arch = strdup(path + strlen("qemu-system-")); else def->os.arch = strdup("i686");
Makes sense. ACK Jirka
The qemu32 CPU model is chosen based on the <os arch=...> name when creating the QEMU command line for a 64-bit host. For the opposite transformation we can test the guest CPU model for the "lm" feature. If it is absent, def->os.arch needs to be corrected. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- src/qemu/qemu_command.c | 18 ++++++++++++++++++ 1 files changed, 18 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fc337f7..7fe8c48 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6830,6 +6830,24 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, } } while ((p = next)); + if (STREQ(dom->os.arch, "x86_64")) { + bool is_32bit = false; + union cpuData *cpuData = NULL; + int ret; + + ret = cpuEncode("x86_64", cpu, NULL, &cpuData, + NULL, NULL, NULL, NULL); + if (ret < 0) + goto error; + + is_32bit = (cpuHasFeature("x86_64", cpuData, "lm") != 1); + cpuDataFree("x86_64", cpuData); + + if (is_32bit) { + VIR_FREE(dom->os.arch); + dom->os.arch = strdup("i686"); + } + } return 0; syntax: -- 1.7.7.6
On Fri, Jan 27, 2012 at 14:49:49 +0100, Paolo Bonzini wrote:
The qemu32 CPU model is chosen based on the <os arch=...> name when creating the QEMU command line for a 64-bit host. For the opposite transformation we can test the guest CPU model for the "lm" feature. If it is absent, def->os.arch needs to be corrected.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- src/qemu/qemu_command.c | 18 ++++++++++++++++++ 1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fc337f7..7fe8c48 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6830,6 +6830,24 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, } } while ((p = next));
+ if (STREQ(dom->os.arch, "x86_64")) { + bool is_32bit = false; + union cpuData *cpuData = NULL; + int ret; + + ret = cpuEncode("x86_64", cpu, NULL, &cpuData, + NULL, NULL, NULL, NULL); + if (ret < 0) + goto error; + + is_32bit = (cpuHasFeature("x86_64", cpuData, "lm") != 1); + cpuDataFree("x86_64", cpuData);
OK, this proves I still didn't embed the arch into cpu so that we don't have to pass it to all cpu* APIs. Perhaps I'll find some time for this soon. Anyway, you're patch is good, ACK. Jirka
Avoid creating an empty <cpu> element when the QEMU command-line simply specifies the default "-cpu qemu32" or "-cpu qemu64". This requires the previous patch, which lets us represent "-cpu qemu32" as <os arch='i686'> in the generated XML. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- src/qemu/qemu_command.c | 50 +++++++++++++++++++++++++++++++--------------- 1 files changed, 34 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7fe8c48..d01696e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6778,12 +6778,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 == ',') @@ -6792,14 +6790,22 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, if ((next = strchr(p, ','))) next++; - if (!cpu->model) { + if (p == val) { 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; + + if (!STREQ(model, "qemu32") && !STREQ(model, "qemu64")) { + if (!(cpu = qemuInitGuestCPU(dom))) + goto error; + + cpu->model = model; + model = NULL; + } } else if (*p == '+' || *p == '-') { char *feature; @@ -6823,6 +6829,13 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, if (!feature) goto no_memory; + if (!cpu) { + if (!(cpu = qemuInitGuestCPU(dom))) + goto error; + + cpu->model = model; + model = NULL; + } ret = virCPUDefAddFeature(cpu, feature, policy); VIR_FREE(feature); if (ret < 0) @@ -6832,22 +6845,27 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, if (STREQ(dom->os.arch, "x86_64")) { bool is_32bit = false; - union cpuData *cpuData = NULL; - int ret; + if (cpu) { + union cpuData *cpuData = NULL; + int ret; - ret = cpuEncode("x86_64", cpu, NULL, &cpuData, - NULL, NULL, NULL, NULL); - if (ret < 0) - goto error; + ret = cpuEncode("x86_64", cpu, NULL, &cpuData, + NULL, NULL, NULL, NULL); + if (ret < 0) + goto error; - is_32bit = (cpuHasFeature("x86_64", cpuData, "lm") != 1); - cpuDataFree("x86_64", cpuData); + is_32bit = (cpuHasFeature("x86_64", cpuData, "lm") != 1); + cpuDataFree("x86_64", cpuData); + } else if (model) { + is_32bit = STREQ(model, "qemu32"); + } if (is_32bit) { VIR_FREE(dom->os.arch); dom->os.arch = strdup("i686"); } } + VIR_FREE(model); return 0; syntax: -- 1.7.7.6
On Fri, Jan 27, 2012 at 14:49:50 +0100, Paolo Bonzini wrote:
Avoid creating an empty <cpu> element when the QEMU command-line simply specifies the default "-cpu qemu32" or "-cpu qemu64".
This requires the previous patch, which lets us represent "-cpu qemu32" as <os arch='i686'> in the generated XML.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- src/qemu/qemu_command.c | 50 +++++++++++++++++++++++++++++++--------------- 1 files changed, 34 insertions(+), 16 deletions(-)
ACK Jirka
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). Reviewed-by: Jiri Denemark <jdenemar@redhat.com> 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 1d0211d..464c4a3 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -943,8 +943,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 4da6dca..2e53e14 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -571,9 +571,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 e872396..978e91c 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 7a8f12d..1d2fb81 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.6
On Fri, Jan 27, 2012 at 14:49:51 +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).
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
And my ACK is still valid. Jirka
Creating part of the -cpu command-line from something other than the <cpu> XML element introduces some ugliness. Reviewed-by: Jiri Denemark <jdenemar@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- src/qemu/qemu_command.c | 77 ++++++++++++++++++-- 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, 160 insertions(+), 7 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 d01696e..a39622c 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 @@ -6829,14 +6859,47 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, if (!feature) goto no_memory; - if (!cpu) { - if (!(cpu = qemuInitGuestCPU(dom))) + 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, + _("conflicting occurrences of kvmclock feature")); goto error; + } + dom->clock.timers[i]->present = present; + ret = 0; + } else { + if (!cpu) { + if (!(cpu = qemuInitGuestCPU(dom))) + goto error; - cpu->model = model; - model = NULL; + cpu->model = model; + model = NULL; + } + + 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..8472b8a --- /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 -enable-kvm -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..6816c00 --- /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 -enable-kvm -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..bae4c47 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.xml @@ -0,0 +1,24 @@ +<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> + <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..6fe9ff6 --- /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/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..6ff3b35 --- /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/kvm</emulator> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index d40b37e..63221ab 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, QEMU_CAPS_ENABLE_KVM); + DO_TEST("cpu-host-kvmclock", false, QEMU_CAPS_ENABLE_KVM, QEMU_CAPS_CPU_HOST); + DO_TEST("kvmclock", false, QEMU_CAPS_KVM); 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 df317fd..09628d8 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.6
On Fri, Jan 27, 2012 at 14:49:52 +0100, Paolo Bonzini wrote:
Creating part of the -cpu command-line from something other than the <cpu> XML element introduces some ugliness.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
ACK again, although now I noticed the following part of the cover letter would fit nicely into this commit message... 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. Jirka
On Fri, Jan 27, 2012 at 14:49:45 +0100, Paolo Bonzini wrote:
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.
Actually, while working on this I noticed that it is quite difficult to have round-trip testcases involving -cpu, that can be used with both qemuargv2xmltest and qemuxml2argvtest. This is what the first five four patches do. The first patch is unrelated. The second patch updates the CPU feature map with default CPUs, and is a prerequisite for patch 3. That one makes the -cpu parsing code detect the "lm" CPU feature, so that the guest arch is correctly set to either i686 or x86_64.
The fifth patch is also mostly needed for testing. It notices the default models qemu32/qemu64 and does not generate <cpu> elements; this happens with "-cpu qemu32,-kvmclock" for example. Then, the last two patches actually add the new feature.
Paolo Bonzini (7): qemu: parse -enable-kvm x86: add kvm32 and kvm64, update qemu64
Two cputests needed to be updated a bit after this 2/7 patch so I squashed them in, updated the 7/7 commit message as I suggested in the review and pushed the series. Jirka
participants (2)
-
Jiri Denemark -
Paolo Bonzini