[libvirt] [PATCH] qemu: Default hwclock source for sPAPR to RTC

[Cole Robinson]

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 at linux.vnet.ibm.com>
> ---
>  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",
>