On 07/13/2017 04:36 AM, Kothapally Madhu Pavan wrote: > 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. > How does it fail exactly? Is it a qemu error message or a guest OS failure? If it's from qemu, and the error message is reasonably clear what hardware/xml config option is at fauly, then these checks don't add much functional benefit, just more code to maintain. A general point, these types of checks should be considered for qemuDomainDefValidate which adds the benefit of rejecting the config at XML define time. Thanks, Cole > Signed-off-by: Kothapally Madhu Pavan <kmp@xxxxxxxxxxxxxxxxxx> > --- > src/qemu/qemu_command.c | 28 +++++++++++++++++++++++++++- > 1 file changed, 27 insertions(+), 1 deletion(-) > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index c53ab97..31561ce 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -6440,6 +6440,15 @@ qemuBuildClockCommandLine(virCommandPtr cmd, > break; > > case VIR_DOMAIN_TIMER_NAME_PIT: > + /* Only RTC timer is supported as hwclock for sPAPR machines */ > + if (ARCH_IS_PPC64(def->os.arch)) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("unsupported clock timer '%s' for '%s' architecture"), > + virDomainTimerNameTypeToString(def->clock.timers[i]->name), > + virArchToString(def->os.arch)); > + return -1; > + } > + > switch (def->clock.timers[i]->tickpolicy) { > case -1: > case VIR_DOMAIN_TIMER_TICKPOLICY_DELAY: > @@ -6483,13 +6492,21 @@ qemuBuildClockCommandLine(virCommandPtr cmd, > break; > > case VIR_DOMAIN_TIMER_NAME_HPET: > + /* Only RTC timer is supported as hwclock for sPAPR machines */ > + if (ARCH_IS_PPC64(def->os.arch)) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("unsupported clock timer '%s' for '%s' architecture"), > + virDomainTimerNameTypeToString(def->clock.timers[i]->name), > + virArchToString(def->os.arch)); > + return -1; > + } > + > /* the only meaningful attribute for hpet is "present". If > * present is -1, that means it wasn't specified, and > * should be left at the default for the > * hypervisor. "default" when -no-hpet exists is "yes", > * and when -no-hpet doesn't exist is "no". "confusing"? > * "yes"! */ > - > if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_HPET)) { > if (def->clock.timers[i]->present == 0) > virCommandAddArg(cmd, "-no-hpet"); > @@ -7047,6 +7064,15 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, > for (i = 0; i < def->clock.ntimers; i++) { > virDomainTimerDefPtr timer = def->clock.timers[i]; > > + /* 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)); > + return -1; > + } > + > if (timer->name == VIR_DOMAIN_TIMER_NAME_KVMCLOCK && > timer->present != -1) { > virBufferAsprintf(&buf, "%s,%ckvmclock", > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list