On 3/24/22 10:48, Paolo Bonzini wrote:
QEMU 7.0.0 adds a new property tsc-clear-on-reset to x86 CPU, corresponding to Libvirt's <tsc on_reboot="clear"/> element. Plumb it in the validation, command line handling and tests.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 4 +++ src/qemu/qemu_validate.c | 36 +++++++++++++------ .../caps_7.0.0.x86_64.replies | 4 +++ .../caps_7.0.0.x86_64.xml | 1 + ...uency.args => cpu-tsc-clear-on-reset.args} | 2 +- ... cpu-tsc-clear-on-reset.x86_64-7.0.0.args} | 6 ++-- ...equency.xml => cpu-tsc-clear-on-reset.xml} | 2 +- tests/qemuxml2argvtest.c | 2 ++ 10 files changed, 44 insertions(+), 16 deletions(-) copy tests/qemuxml2argvdata/{cpu-tsc-frequency.args => cpu-tsc-clear-on-reset.args} (97%) copy tests/qemuxml2argvdata/{virtio-rng-builtin.x86_64-latest.args => cpu-tsc-clear-on-reset.x86_64-7.0.0.args} (77%) copy tests/qemuxml2argvdata/{cpu-tsc-frequency.xml => cpu-tsc-clear-on-reset.xml} (95%)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 32980e7330..d22bbee70e 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -668,6 +668,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
/* 425 */ "blockdev.nbd.tls-hostname", /* QEMU_CAPS_BLOCKDEV_NBD_TLS_HOSTNAME */ + "x86-cpu.tsc-clear-on-reset", /* QEMU_CAPS_X86_CPU_TSC_CLEAR_ON_RESET */ );
@@ -1775,6 +1776,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsMemoryBackendMemfd[] static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsMaxCPU[] = { { "unavailable-features", QEMU_CAPS_CPU_UNAVAILABLE_FEATURES }, { "kvm-no-adjvtime", QEMU_CAPS_CPU_KVM_NO_ADJVTIME }, + { "tsc-clear-on-reset", QEMU_CAPS_X86_CPU_TSC_CLEAR_ON_RESET }, { "migratable", QEMU_CAPS_CPU_MIGRATABLE }, };
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 0a215a11d5..7aee73a725 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -643,6 +643,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */
/* 425 */ QEMU_CAPS_BLOCKDEV_NBD_TLS_HOSTNAME, /* tls hostname can be overriden for NBD clients */ + QEMU_CAPS_X86_CPU_TSC_CLEAR_ON_RESET, /* x86-cpu.tsc-clear-on-reset */
QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c836799888..8ecede0ade 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6613,6 +6613,10 @@ qemuBuildCpuCommandLine(virCommand *cmd, case VIR_DOMAIN_TIMER_NAME_TSC: if (timer->frequency > 0) virBufferAsprintf(&buf, ",tsc-frequency=%llu", timer->frequency); + if (timer->reboot == VIR_DOMAIN_TIMER_REBOOT_MODE_CLEAR) + virBufferAddLit(&buf, ",tsc-clear-on-reset=on"); + else if (timer->reboot == VIR_DOMAIN_TIMER_REBOOT_MODE_KEEP) + virBufferAddLit(&buf, ",tsc-clear-on-reset=off");
Here, I'd prefer switch(), esp. after the reboot member is declared as corresponding enum instead of int so that compiler warns us about this place when a new enum value is added.
break; case VIR_DOMAIN_TIMER_NAME_ARMVTIMER: switch (timer->tickpolicy) { diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index f27e480696..c4ab2976dc 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -396,10 +396,11 @@ static int qemuValidateDomainDefClockTimers(const virDomainDef *def, virQEMUCaps *qemuCaps) { + virDomainTimerDef *timer; size_t i;
for (i = 0; i < def->clock.ntimers; i++) { - virDomainTimerDef *timer = def->clock.timers[i]; + timer = def->clock.timers[i];
switch ((virDomainTimerNameType)timer->name) { case VIR_DOMAIN_TIMER_NAME_PLATFORM: @@ -409,20 +410,25 @@ qemuValidateDomainDefClockTimers(const virDomainDef *def, return -1;
case VIR_DOMAIN_TIMER_NAME_TSC: - case VIR_DOMAIN_TIMER_NAME_KVMCLOCK: - case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK: - if (!ARCH_IS_X86(def->os.arch) && timer->present == VIR_TRISTATE_BOOL_YES) { + if (!ARCH_IS_X86(def->os.arch) && timer->present == VIR_TRISTATE_BOOL_YES) + goto no_support; + + if (timer->reboot != VIR_DOMAIN_TIMER_REBOOT_MODE_DEFAULT && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_X86_CPU_TSC_CLEAR_ON_RESET)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Configuring the '%s' timer is not supported " - "for virtType=%s arch=%s machine=%s guests"), - virDomainTimerNameTypeToString(timer->name), - virDomainVirtTypeToString(def->virtType), - virArchToString(def->os.arch), - def->os.machine); + _("Configuring the '%s' timer to reset on reboot is not supported " + "with this QEMU binary"),
Error messages are exempt from the 80 chars rule, so that they can be easily 'git grep'-ped.
+ virDomainTimerNameTypeToString(timer->name)); return -1; } break;
+ case VIR_DOMAIN_TIMER_NAME_KVMCLOCK: + case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK: + if (!ARCH_IS_X86(def->os.arch) && timer->present == VIR_TRISTATE_BOOL_YES) + goto no_support; + break; + case VIR_DOMAIN_TIMER_NAME_LAST: break;
@@ -540,6 +546,16 @@ qemuValidateDomainDefClockTimers(const virDomainDef *def, }
return 0; + + no_support: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Configuring the '%s' timer is not supported " + "for virtType=%s arch=%s machine=%s guests"), + virDomainTimerNameTypeToString(timer->name), + virDomainVirtTypeToString(def->virtType), + virArchToString(def->os.arch), + def->os.machine); + return -1; }
Michal