[libvirt] [PATCH v3 0/2] Restrict usage of unsupported clock timers
hpet and kvm.pit clock timers are specific to x86 architecture and are not to be used by unsuported architectures. Similarly sPAPR guests only allow RTC timer. This patchset will restrict the usage of unsupported clock timers. Kothapally Madhu Pavan (2): qemu: Restrict usage of hpet and kvm.pit timers by unsupported architectures qemu: Default hwclock source for sPAPR to RTC src/qemu/qemu_domain.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) -- 1.8.3.1
hpet and kvm.pit clock timers are specific to x86 architecture and are not suppose to be used in other architectures. This patch restricts the usage of hpet and kvm.pit timers in unsupported architectures. Signed-off-by: Kothapally Madhu Pavan <kmp@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2c8c9a7..e5e4208 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3083,12 +3083,28 @@ qemuDomainDefValidate(const virDomainDef *def, virQEMUCapsPtr qemuCaps = NULL; unsigned int topologycpus; int ret = -1; + size_t i; if (!(qemuCaps = virQEMUCapsCacheLookup(caps, driver->qemuCapsCache, def->emulator))) goto cleanup; + /* Restrict usage of unsupported clock sources */ + for (i = 0; i < def->clock.ntimers; i++) { + virDomainTimerDefPtr timer = def->clock.timers[i]; + if ((!(virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_HPET)) && + (timer->name == VIR_DOMAIN_TIMER_NAME_HPET)) || + (!(virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_KVM_PIT)) && + (timer->name == VIR_DOMAIN_TIMER_NAME_PIT))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported clock timer '%s' for %s architecture"), + virDomainTimerNameTypeToString(def->clock.timers[i]->name), + virArchToString(def->os.arch)); + goto cleanup; + } + } + if (def->mem.min_guarantee) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Parameter 'min_guarantee' not supported by QEMU.")); -- 1.8.3.1
On 07/25/2017 10:36 AM, Kothapally Madhu Pavan wrote:
hpet and kvm.pit clock timers are specific to x86 architecture and are not suppose to be used in other architectures. This patch restricts the usage of hpet and kvm.pit timers in unsupported architectures.
Signed-off-by: Kothapally Madhu Pavan <kmp@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2c8c9a7..e5e4208 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3083,12 +3083,28 @@ qemuDomainDefValidate(const virDomainDef *def, virQEMUCapsPtr qemuCaps = NULL; unsigned int topologycpus; int ret = -1; + size_t i;
if (!(qemuCaps = virQEMUCapsCacheLookup(caps, driver->qemuCapsCache, def->emulator))) goto cleanup;
+ /* Restrict usage of unsupported clock sources */ + for (i = 0; i < def->clock.ntimers; i++) { + virDomainTimerDefPtr timer = def->clock.timers[i]; + if ((!(virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_HPET)) && + (timer->name == VIR_DOMAIN_TIMER_NAME_HPET)) || + (!(virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_KVM_PIT)) && + (timer->name == VIR_DOMAIN_TIMER_NAME_PIT))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported clock timer '%s' for %s architecture"), + virDomainTimerNameTypeToString(def->clock.timers[i]->name), + virArchToString(def->os.arch)); + goto cleanup; + } + } + if (def->mem.min_guarantee) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Parameter 'min_guarantee' not supported by QEMU."));
The HPET bit isn't strictly correct. See the HPET handling in qemu_command.c, the only case that's explicitly rejected is when hpet present=yes is specified, but qemu doesn't have NO_HPET. For example requesting hpet present=no on PPC64 is arguably a valid request, since there won't ever be hpet available. So I think in this case it's better to just drop the HPET checking, OR move the qemu_command.c error message here. But the benefit of this is very small (validating hpet present=yes at XML define time vs startup time) However patch #2 will also reject that HPET case. I think patch #2 should be dropped entirely. Figure out what timer XML settings produce ambiguous qemu results (the kvm-pit bit you mentioned previously that qemu won't error about) and only reject those, and do it with QEMU_CAPS settings. If qemu/libvirt already throws an error, like it seems to do for hypervclock, tsc, kvmclock, hpet, platform, then adding an extra validation only adds the benefit of an improved error message and catching the issue at XML parse time, but the downside is it runs the risk of being overly restrictive, and it adds more code to maintain. So IMO for those cases that are going to error anyway it's typically better to just leave it alone unless the qemu error message is really ambiguous For the kvmclock/hypervclock/tsc issue, I posted patches to try and improve our handling a bit and give a more explicit error, still need to address reviews/push them though https://www.redhat.com/archives/libvir-list/2017-July/msg00537.html - Cole
QEMU fails to launch a sPAPR guest with clock sources other that RTC. Internally qemu only uses RTC timer for hwclock. This patch reports the right error message instead of qemu erroring out when any other timer other than RTC is used. Signed-off-by: Kothapally Madhu Pavan <kmp@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index e5e4208..b3cfb6c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3103,6 +3103,14 @@ qemuDomainDefValidate(const virDomainDef *def, virArchToString(def->os.arch)); goto cleanup; } + /* /* Only RTC timer is supported as hwclock for sPAPR machines */ + if (ARCH_IS_PPC64(def->os.arch) && timer->name != VIR_DOMAIN_TIMER_NAME_RTC) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported clock timer '%s' for %s architecture"), + virDomainTimerNameTypeToString(def->clock.timers[i]->name), + virArchToString(def->os.arch)); + goto cleanup; + } } if (def->mem.min_guarantee) { -- 1.8.3.1
participants (2)
-
Cole Robinson -
Kothapally Madhu Pavan