[PATCH 0/2] Revert two patches I've pushed mistakenly
Firstly, QEMU part is not merged which alone should have prevented me from pushing these patches. Secondly, in the end QEMU is fixing the bug in a different manner that does not require any libvirt work. https://lists.gnu.org/archive/html/qemu-devel/2022-03/msg06077.html https://lists.gnu.org/archive/html/qemu-devel/2022-03/msg06188.html Michal Prívozník (2): Revert "domain: add tsc.on_reboot element" Revert "qemu: add support for tsc.on_reboot element" docs/formatdomain.rst | 4 -- src/conf/domain_conf.c | 17 ------- src/conf/domain_conf.h | 10 ----- src/conf/schemas/domaincommon.rng | 8 ---- src/qemu/qemu_capabilities.c | 2 - src/qemu/qemu_capabilities.h | 1 - src/qemu/qemu_command.c | 11 ----- src/qemu/qemu_validate.c | 45 ++++++++----------- .../caps_7.0.0.x86_64.replies | 4 -- .../caps_7.0.0.x86_64.xml | 1 - .../cpu-tsc-clear-on-reset.args | 32 ------------- .../cpu-tsc-clear-on-reset.x86_64-7.0.0.args | 34 -------------- .../cpu-tsc-clear-on-reset.xml | 35 --------------- tests/qemuxml2argvtest.c | 2 - 14 files changed, 19 insertions(+), 187 deletions(-) delete mode 100644 tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.args delete mode 100644 tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.x86_64-7.0.0.args delete mode 100644 tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.xml -- 2.34.1
This reverts commit 150540394ddaa515f6857616a2bcf792748f162c. Turns out, this feature is not needed and QEMU will fix TSC without any intervention from outside. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/formatdomain.rst | 4 ---- src/conf/domain_conf.c | 17 ----------------- src/conf/domain_conf.h | 10 ---------- src/conf/schemas/domaincommon.rng | 8 -------- 4 files changed, 39 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 514c165bb5..e492532004 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -2243,10 +2243,6 @@ Windows, however, expects it to be in so called 'localtime'. ``frequency`` The ``frequency`` attribute is an unsigned integer specifying the frequency at which ``name="tsc"`` runs. - ``on_reboot`` - The ``on_reboot`` attribute controls how the ``name="tsc"`` timer behaves - when the VM is reset, and can be "default", "clear" or "keep". The reset - behavior of other timers is hardcoded, and depends on the type of timer. ``mode`` The ``mode`` attribute controls how the ``name="tsc"`` timer is managed, and can be "auto", "native", "emulate", "paravirt", or "smpsafe". Other diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ed927ab669..731139f80f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1217,13 +1217,6 @@ VIR_ENUM_IMPL(virDomainTimerMode, "smpsafe", ); -VIR_ENUM_IMPL(virDomainTimerRebootMode, - VIR_DOMAIN_TIMER_REBOOT_MODE_LAST, - "default", - "keep", - "clear", -); - VIR_ENUM_IMPL(virDomainStartupPolicy, VIR_DOMAIN_STARTUP_POLICY_LAST, "default", @@ -12087,11 +12080,6 @@ virDomainTimerDefParseXML(xmlNodePtr node, } } - if (virXMLPropEnum(node, "on_reboot", - virDomainTimerRebootModeTypeFromString, - VIR_XML_PROP_NONZERO, &def->reboot) < 0) - goto error; - catchup = virXPathNode("./catchup", ctxt); if (catchup != NULL) { ret = virXPathULong("string(./catchup/@threshold)", ctxt, @@ -26170,11 +26158,6 @@ virDomainTimerDefFormat(virBuffer *buf, virBufferAsprintf(&timerAttr, " mode='%s'", virDomainTimerModeTypeToString(def->mode)); } - - if (def->reboot) { - virBufferAsprintf(&timerAttr, " on_reboot='%s'", - virDomainTimerRebootModeTypeToString(def->mode)); - } } if (def->catchup.threshold > 0) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 42db4b69c3..49c964e6e1 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2408,14 +2408,6 @@ typedef enum { VIR_DOMAIN_TIMER_MODE_LAST } virDomainTimerModeType; -typedef enum { - VIR_DOMAIN_TIMER_REBOOT_MODE_DEFAULT = 0, - VIR_DOMAIN_TIMER_REBOOT_MODE_KEEP, - VIR_DOMAIN_TIMER_REBOOT_MODE_CLEAR, - - VIR_DOMAIN_TIMER_REBOOT_MODE_LAST -} virDomainTimerRebootModeType; - typedef enum { VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC = 0, VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO, @@ -2447,7 +2439,6 @@ struct _virDomainTimerDef { /* frequency & mode are only valid for name='tsc' */ unsigned long long frequency; /* in Hz, unspecified = 0 */ int mode; /* enum virDomainTimerModeType */ - virDomainTimerRebootModeType reboot; }; typedef enum { @@ -4042,7 +4033,6 @@ VIR_ENUM_DECL(virDomainClockBasis); VIR_ENUM_DECL(virDomainTimerName); VIR_ENUM_DECL(virDomainTimerTrack); VIR_ENUM_DECL(virDomainTimerTickpolicy); -VIR_ENUM_DECL(virDomainTimerRebootMode); VIR_ENUM_DECL(virDomainTimerMode); VIR_ENUM_DECL(virDomainCpuPlacementMode); diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 8890034f52..34bccee2f5 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -1292,14 +1292,6 @@ <ref name="unsignedLong"/> </attribute> </optional> - <optional> - <attribute name="on_reboot"> - <choice> - <value>clear</value> - <value>keep</value> - </choice> - </attribute> - </optional> <optional> <attribute name="mode"> <choice> -- 2.34.1
This reverts commit 06c960e477de4561c7ba956f82994fa120226397. Turns out, this feature is not needed and QEMU will fix TSC without any intervention from outside. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_capabilities.c | 2 - src/qemu/qemu_capabilities.h | 1 - src/qemu/qemu_command.c | 11 ----- src/qemu/qemu_validate.c | 45 ++++++++----------- .../caps_7.0.0.x86_64.replies | 4 -- .../caps_7.0.0.x86_64.xml | 1 - .../cpu-tsc-clear-on-reset.args | 32 ------------- .../cpu-tsc-clear-on-reset.x86_64-7.0.0.args | 34 -------------- .../cpu-tsc-clear-on-reset.xml | 35 --------------- tests/qemuxml2argvtest.c | 2 - 10 files changed, 19 insertions(+), 148 deletions(-) delete mode 100644 tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.args delete mode 100644 tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.x86_64-7.0.0.args delete mode 100644 tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.xml diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 7c427cefc6..6b4ed08499 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -669,7 +669,6 @@ VIR_ENUM_IMPL(virQEMUCaps, /* 425 */ "blockdev.nbd.tls-hostname", /* QEMU_CAPS_BLOCKDEV_NBD_TLS_HOSTNAME */ "memory-backend-file.prealloc-threads", /* QEMU_CAPS_MEMORY_BACKEND_PREALLOC_THREADS */ - "x86-cpu.tsc-clear-on-reset", /* QEMU_CAPS_X86_CPU_TSC_CLEAR_ON_RESET */ ); @@ -1778,7 +1777,6 @@ 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 995a3e8edf..948029d60d 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -644,7 +644,6 @@ 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_MEMORY_BACKEND_PREALLOC_THREADS, /* -object memory-backend-*.prealloc-threads */ - 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 3565a5967e..8246ab515a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6616,17 +6616,6 @@ qemuBuildCpuCommandLine(virCommand *cmd, case VIR_DOMAIN_TIMER_NAME_TSC: if (timer->frequency > 0) virBufferAsprintf(&buf, ",tsc-frequency=%llu", timer->frequency); - switch (timer->reboot) { - case VIR_DOMAIN_TIMER_REBOOT_MODE_CLEAR: - virBufferAddLit(&buf, ",tsc-clear-on-reset=on"); - break; - case VIR_DOMAIN_TIMER_REBOOT_MODE_KEEP: - virBufferAddLit(&buf, ",tsc-clear-on-reset=off"); - break; - case VIR_DOMAIN_TIMER_REBOOT_MODE_DEFAULT: - case VIR_DOMAIN_TIMER_REBOOT_MODE_LAST: - break; - } 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 8f569554c6..e0708b8a76 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -396,11 +396,10 @@ static int qemuValidateDomainDefClockTimers(const virDomainDef *def, virQEMUCaps *qemuCaps) { - virDomainTimerDef *timer; size_t i; for (i = 0; i < def->clock.ntimers; i++) { - timer = def->clock.timers[i]; + virDomainTimerDef *timer = def->clock.timers[i]; switch ((virDomainTimerNameType)timer->name) { case VIR_DOMAIN_TIMER_NAME_PLATFORM: @@ -410,22 +409,18 @@ qemuValidateDomainDefClockTimers(const virDomainDef *def, return -1; case VIR_DOMAIN_TIMER_NAME_TSC: - 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 to reset on reboot is not supported with this QEMU binary"), - 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; + if (!ARCH_IS_X86(def->os.arch) && timer->present == VIR_TRISTATE_BOOL_YES) { + 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; + } break; case VIR_DOMAIN_TIMER_NAME_LAST: @@ -504,7 +499,14 @@ qemuValidateDomainDefClockTimers(const virDomainDef *def, case VIR_DOMAIN_TIMER_NAME_ARMVTIMER: if (def->virtType != VIR_DOMAIN_VIRT_KVM || !qemuDomainIsARMVirt(def)) { - goto 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; } if (timer->present == VIR_TRISTATE_BOOL_NO) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -538,15 +540,6 @@ 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; } diff --git a/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.replies index d919caf7f3..82ccbab6eb 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.replies @@ -28849,10 +28849,6 @@ { "name": "sse4_1", "type": "bool" - }, - { - "name": "tsc-clear-on-reset", - "type": "bool" } ], "id": "libvirt-41" diff --git a/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml index 02868b7695..5227e3ee0b 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml @@ -245,7 +245,6 @@ <flag name='dirtyrate-param.mode'/> <flag name='blockdev.nbd.tls-hostname'/> <flag name='memory-backend-file.prealloc-threads'/> - <flag name='x86-cpu.tsc-clear-on-reset'/> <version>6002050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100243</microcodeVersion> diff --git a/tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.args b/tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.args deleted file mode 100644 index f2b8a98968..0000000000 --- a/tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.args +++ /dev/null @@ -1,32 +0,0 @@ -LC_ALL=C \ -PATH=/bin \ -HOME=/tmp/lib/domain--1-QEMUGuest1 \ -USER=test \ -LOGNAME=test \ -XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ -XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ -XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ -QEMU_AUDIO_DRV=none \ -/usr/bin/qemu-system-x86_64 \ --name guest=QEMUGuest1,debug-threads=on \ --S \ --object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ --machine pc,usb=off,dump-guest-core=off \ --accel kvm \ --cpu Haswell,vme=on,ds=on,acpi=on,ss=on,ht=on,tm=on,pbe=on,dtes64=on,monitor=on,ds-cpl=on,vmx=on,smx=on,est=on,tm2=on,xtpr=on,pdcm=on,f16c=on,rdrand=on,pdpe1gb=on,abm=on,lahf-lm=on,invtsc=on,tsc-clear-on-reset=on \ --m 214 \ --realtime mlock=off \ --smp 1,sockets=1,cores=1,threads=1 \ --uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ --display none \ --no-user-config \ --nodefaults \ --chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server=on,wait=off \ --mon chardev=charmonitor,id=monitor,mode=control \ --rtc base=utc \ --no-shutdown \ --no-acpi \ --boot strict=on \ --usb \ --device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \ --msg timestamp=on diff --git a/tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.x86_64-7.0.0.args b/tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.x86_64-7.0.0.args deleted file mode 100644 index 59f1d66ed4..0000000000 --- a/tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.x86_64-7.0.0.args +++ /dev/null @@ -1,34 +0,0 @@ -LC_ALL=C \ -PATH=/bin \ -HOME=/tmp/lib/domain--1-QEMUGuest1 \ -USER=test \ -LOGNAME=test \ -XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ -XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ -XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ -/usr/bin/qemu-system-x86_64 \ --name guest=QEMUGuest1,debug-threads=on \ --S \ --object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ --machine pc-i440fx-7.0,usb=off,dump-guest-core=off,memory-backend=pc.ram \ --accel kvm \ --cpu EPYC-Rome,x2apic=on,tsc-deadline=on,hypervisor=on,tsc-adjust=on,stibp=on,arch-capabilities=on,ssbd=on,xsaves=on,cmp-legacy=on,amd-ssbd=on,virt-ssbd=on,svme-addr-chk=on,rdctl-no=on,skip-l1dfl-vmentry=on,mds-no=on,pschange-mc-no=on,invtsc=on,tsc-clear-on-reset=on \ --m 214 \ --object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ --overcommit mem-lock=off \ --smp 1,sockets=1,cores=1,threads=1 \ --uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ --display none \ --no-user-config \ --nodefaults \ --chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ --mon chardev=charmonitor,id=monitor,mode=control \ --rtc base=utc \ --no-shutdown \ --no-acpi \ --boot strict=on \ --device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ --audiodev '{"id":"audio1","driver":"none"}' \ --device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x2"}' \ --sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ --msg timestamp=on diff --git a/tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.xml b/tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.xml deleted file mode 100644 index 2521349ce7..0000000000 --- a/tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.xml +++ /dev/null @@ -1,35 +0,0 @@ -<domain type='kvm'> - <name>QEMUGuest1</name> - <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> - <title>A description of the test machine.</title> - <description> - A test of qemu's minimal configuration. - This test also tests the description and title elements. - </description> - <memory unit='KiB'>219100</memory> - <currentMemory unit='KiB'>219100</currentMemory> - <vcpu placement='static'>1</vcpu> - <os> - <type arch='x86_64' machine='pc'>hvm</type> - <boot dev='hd'/> - </os> - <cpu mode='host-model'> - <model fallback='allow'/> - <feature policy='require' name='invtsc'/> - </cpu> - <clock offset='utc'> - <timer name='tsc' on_reboot='clear'/> - </clock> - <on_poweroff>destroy</on_poweroff> - <on_reboot>restart</on_reboot> - <on_crash>destroy</on_crash> - <devices> - <emulator>/usr/bin/qemu-system-x86_64</emulator> - <controller type='usb' index='0'/> - <controller type='ide' index='0'/> - <controller type='pci' index='0' model='pci-root'/> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> - <memballoon model='virtio'/> - </devices> -</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 9d5aba85a1..e7fecb24d3 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2094,8 +2094,6 @@ mymain(void) DO_TEST("cpu-Haswell-noTSX", QEMU_CAPS_KVM); DO_TEST_NOCAPS("cpu-host-model-cmt"); DO_TEST_CAPS_VER("cpu-host-model-cmt", "4.0.0"); - DO_TEST("cpu-tsc-clear-on-reset", QEMU_CAPS_X86_CPU_TSC_CLEAR_ON_RESET); - DO_TEST_CAPS_VER("cpu-tsc-clear-on-reset", "7.0.0"); DO_TEST("cpu-tsc-frequency", QEMU_CAPS_KVM); DO_TEST_CAPS_VER("cpu-tsc-frequency", "4.0.0"); DO_TEST_CAPS_VER("cpu-translation", "4.0.0"); -- 2.34.1
On Mon, Mar 28, 2022 at 09:31:46 +0200, Michal Privoznik wrote:
Firstly, QEMU part is not merged which alone should have prevented me from pushing these patches.
Secondly, in the end QEMU is fixing the bug in a different manner that does not require any libvirt work.
https://lists.gnu.org/archive/html/qemu-devel/2022-03/msg06077.html
https://lists.gnu.org/archive/html/qemu-devel/2022-03/msg06188.html
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
participants (2)
-
Jiri Denemark -
Michal Privoznik