[libvirt] [PATCH] qemu: don't use deprecated -no-kvm-pit-reinjection

[Jiri Denemark]

On Mon, Jul 01, 2013 at 18:36:11 +0200, Jano Tomko wrote:
> Since qemu-kvm 1.1 [1] '-no-kvm-pit-reinjection' has been deprecated
> in favor of '-global kvm-pit.lost_tick_policy=discard'
> 
> In upstream qemu since 1.3 [2].
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=978719
> 
> [1] http://git.kernel.org/cgit/virt/kvm/qemu-kvm.git/commit/?id=4e4fa39
> [2] http://git.qemu.org/?p=qemu.git;a=commitdiff;h=c21fb4f
> ---
>  src/qemu/qemu_capabilities.c | 12 ++++++++++--
>  src/qemu/qemu_capabilities.h |  1 +
>  src/qemu/qemu_command.c      |  5 ++++-
>  3 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 969b001..c6df463 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -233,6 +233,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
>                "mlock",
>  
>                "vnc-share-policy", /* 150 */
> +              "kvm-pit-property",
>      );
>  
>  struct _virQEMUCaps {
> @@ -2468,13 +2469,12 @@ virQEMUCapsInitArchQMPBasic(virQEMUCapsPtr qemuCaps,
>  
>      /*
>       * Currently only x86_64 and i686 support PCI-multibus,
> -     * -no-acpi and -no-kvm-pit-reinjection.
> +     * -no-acpi
>       */
>      if (qemuCaps->arch == VIR_ARCH_X86_64 ||
>          qemuCaps->arch == VIR_ARCH_I686) {
>          virQEMUCapsSet(qemuCaps, QEMU_CAPS_PCI_MULTIBUS);
>          virQEMUCapsSet(qemuCaps, QEMU_CAPS_NO_ACPI);
> -        virQEMUCapsSet(qemuCaps, QEMU_CAPS_NO_KVM_PIT);
>      }
>  
>      ret = 0;
> @@ -2640,6 +2640,14 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
>      if (virQEMUCapsProbeQMPCommandLine(qemuCaps, mon) < 0)
>          goto cleanup;
>  
> +    /* -global kvm-pit.lost_tick_policy=discard */
> +    if ((qemuCaps->arch == VIR_ARCH_X86_64 ||
> +         qemuCaps->arch == VIR_ARCH_I686) &&
> +        (qemuCaps->version >= 1003000 ||
> +         virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))) {
> +        virQEMUCapsSet(qemuCaps, QEMU_CAPS_KVM_PIT_PROPERTY);
> +    }
> +

I think we should keep setting QEMU_CAPS_NO_KVM_PIT if QEMU is older and
does not support QEMU_CAPS_KVM_PIT_PROPERTY. Moreover, isn't there a
better way of checking QEMU_CAPS_KVM_PIT_PROPERTY? We should not rely on
QEMU version unless absolutely necessary.

>      ret = 0;
>  
>  cleanup:
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 7088747..c64f648 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -189,6 +189,7 @@ enum virQEMUCapsFlags {
>      QEMU_CAPS_DRIVE_DISCARD      = 148, /* -drive discard=off(ignore)|on(unmap) */
>      QEMU_CAPS_MLOCK              = 149, /* -realtime mlock=on|off */
>      QEMU_CAPS_VNC_SHARE_POLICY   = 150, /* set display sharing policy */
> +    QEMU_CAPS_KVM_PIT_PROPERTY   = 151, /* -global kvm-pit.lost_tick_policy */
>  
>      QEMU_CAPS_LAST,                   /* this must always be the last item */
>  };
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index ba93233..a678666 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -7024,7 +7024,10 @@ qemuBuildCommandLine(virConnectPtr conn,
>              case VIR_DOMAIN_TIMER_TICKPOLICY_DELAY:
>                  /* delay is the default if we don't have kernel
>                     (-no-kvm-pit), otherwise, the default is catchup. */
> -                if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_KVM_PIT))
> +                if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM_PIT_PROPERTY))
> +                    virCommandAddArgList(cmd, "-global",
> +                                         "kvm-pit.lost_tick_policy=discard", NULL);
> +                else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_KVM_PIT))
>                      virCommandAddArg(cmd, "-no-kvm-pit-reinjection");
>                  break;
>              case VIR_DOMAIN_TIMER_TICKPOLICY_CATCHUP:

I'm surprised there are no changes to tests. That would make me think we
do not test QMP caps probing, which would be bad as we need to make sure
we don't break compatibility with older QEMU versions...

Jirka