[PATCH v2] qemu: Do not silently allow non-available timers on non-x86 systems
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. Note: The error is only generated if the timer is marked with present="yes" - otherwise we would suddenly refuse XML definitions that worked without problems before. Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1754887 Signed-off-by: Thomas Huth <thuth@redhat.com> --- v2: Check also for timer->present == 1 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..561e7b12c7 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) && timer->present == 1) { + 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; -- 2.18.1
On 8/5/20 7:22 AM, 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. Note: The error is only generated if the timer is marked with present="yes" - otherwise we would suddenly refuse XML definitions that worked without problems before.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1754887 Signed-off-by: Thomas Huth <thuth@redhat.com> ---
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
v2: Check also for timer->present == 1
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..561e7b12c7 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) && timer->present == 1) { + 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;
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> On 8/5/20 12:22 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. Note: The error is only generated if the timer is marked with present="yes" - otherwise we would suddenly refuse XML definitions that worked without problems before.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1754887 Signed-off-by: Thomas Huth <thuth@redhat.com> --- v2: Check also for timer->present == 1
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..561e7b12c7 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) && timer->present == 1) { + 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;
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On 05/08/2020 12.22, 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. Note: The error is only generated if the timer is marked with present="yes" - otherwise we would suddenly refuse XML definitions that worked without problems before.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1754887 Signed-off-by: Thomas Huth <thuth@redhat.com> --- v2: Check also for timer->present == 1
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..561e7b12c7 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) && timer->present == 1) { + 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;
Ping! The patch got two Reviewed-bys, so I guess it should be fine now? Could somebody please pick it up? Thanks, Thomas
On Wed, Sep 02, 2020 at 06:23:32PM +0200, Thomas Huth wrote:
On 05/08/2020 12.22, 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. Note: The error is only generated if the timer is marked with present="yes" - otherwise we would suddenly refuse XML definitions that worked without problems before.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1754887 Signed-off-by: Thomas Huth <thuth@redhat.com> --- v2: Check also for timer->present == 1
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..561e7b12c7 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) && timer->present == 1) { + 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;
Ping! The patch got two Reviewed-bys, so I guess it should be fine now? Could somebody please pick it up?
I've pushed Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 8/5/20 12:22 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. Note: The error is only generated if the timer is marked with present="yes" - otherwise we would suddenly refuse XML definitions that worked without problems before.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1754887 Signed-off-by: Thomas Huth <thuth@redhat.com> --- v2: Check also for timer->present == 1
src/qemu/qemu_validate.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
Terribly sorry for letting this slip. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> and pushed now. Michal
participants (5)
-
Boris Fiuczynski -
Daniel Henrique Barboza -
Daniel P. Berrangé -
Michal Privoznik -
Thomas Huth