libvirt currently silently allows <timer name="kvmclock"/> and some other timer tags in the guest XML definition for timers that do not exist on non-x86 systems. We should not silently ignore these tags since the users might not get what they expected otherwise. Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1754887 Signed-off-by: Thomas Huth <thuth(a)redhat.com&gt; --- src/qemu/qemu_validate.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 488f258d00..667ac5cc23 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -371,6 +371,18 @@ qemuValidateDomainDefClockTimers(const virDomainDef *def, 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)) { + 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: break;

On 28/07/2020 17.13, Boris Fiuczynski wrote: > On 7/22/20 1:21 PM, Thomas Huth wrote: >> libvirt currently silently allows <timer name="kvmclock"/> and some >> other timer tags in the guest XML definition for timers that do not >> exist on non-x86 systems. We should not silently ignore these tags >> since the users might not get what they expected otherwise. >> >> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1754887 >> Signed-off-by: Thomas Huth <thuth(a)redhat.com&gt; >> --- >>   src/qemu/qemu_validate.c | 12 ++++++++++++ >>   1 file changed, 12 insertions(+) >> >> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c >> index 488f258d00..667ac5cc23 100644 >> --- a/src/qemu/qemu_validate.c >> +++ b/src/qemu/qemu_validate.c >> @@ -371,6 +371,18 @@ qemuValidateDomainDefClockTimers(const >> virDomainDef *def, >>           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)) { >> +                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: >>               break; >>   > > This would render previously as valid accepted domains invalid, e.g. on > s390x using kvmclock: As long as the user does not specify the "present" > attribute the domain starts without error since qemus cpu parameter is > not extended. Shall I turn it into a VIR_WARN() instead?