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

[Martin Kletzander]

On Thu, Sep 26, 2013 at 10:50:16AM +0200, Ján Tomko wrote:
> Since qemu-kvm 1.1 [1] (since 1.3. in upstream QEMU [2])
> '-no-kvm-pit-reinjection' has been deprecated.
> Use -global kvm-pit.lost_tick_policy=discard instead.
> 
> 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
> ---
> v3: use -global instead of trying to add another -device
> 
> re: https://www.redhat.com/archives/libvir-list/2013-July/msg00833.html
> Unsetting the QEMU_CAPS_NO_KVM_PIT capability for QEMU >1.2 seems to work
> okay with libvirtd upgrades.
> 
> v2: https://www.redhat.com/archives/libvir-list/2013-July/msg00821.html
> v1: https://www.redhat.com/archives/libvir-list/2013-July/msg00045.html
> 
>  src/qemu/qemu_capabilities.c                       |  5 ++--
>  src/qemu/qemu_capabilities.h                       |  1 +
>  src/qemu/qemu_command.c                            |  8 ++++--
>  .../qemuxml2argv-kvm-pit-delay.args                |  5 ++++
>  .../qemuxml2argv-kvm-pit-delay.xml                 | 29 ++++++++++++++++++++++
>  .../qemuxml2argv-kvm-pit-device.args               |  5 ++++
>  .../qemuxml2argv-kvm-pit-device.xml                | 29 ++++++++++++++++++++++
>  tests/qemuxml2argvtest.c                           |  4 +++
>  8 files changed, 82 insertions(+), 4 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-delay.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-delay.xml
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index dc8f0be..06b71b5 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -242,6 +242,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
>                "usb-storage.removable",
>                "virtio-mmio",
>                "ich9-intel-hda",
> +              "kvm-pit",
>      );
>  
>  struct _virQEMUCaps {
> @@ -1393,6 +1394,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
>      { "usb-storage", QEMU_CAPS_DEVICE_USB_STORAGE },
>      { "virtio-mmio", QEMU_CAPS_DEVICE_VIRTIO_MMIO },
>      { "ich9-intel-hda", QEMU_CAPS_DEVICE_ICH9_INTEL_HDA },
> +    { "kvm-pit", QEMU_CAPS_DEVICE_KVM_PIT },
>  };
>  

Cleaner way would be (IMHO):

static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsKvmPit[] = {
    { "lost_tick_policy", QEMU_CAPS_KVM_PIT_TICK_POLICY },
};

(or similar) and then adding into virQEMUCapsObjectProps[]:

    { "kvm-pit", virQEMUCapsObjectPropsKvmPit,
    ARRAY_CARDINALITY(virQEMUCapsObjectPropsKvmPit) }

>  static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
> @@ -2477,13 +2479,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);
>      }
>  

Sorry to dissapoint you in v3, but I must say, I still agree with
Eric.  Since that flag is just 'deprecated', it doesn't mean that it
is not recognized.  And thanks to the fact that you have a possibility
to check whether the newer 'kvm-pit.lost_tick_policy' is supported,
you can safely choose all three states even if there is a qemu with
QMP on x86_64 without 'kvm-pit.lost_tick_policy' and it will still
work correctly then.

I'd keep the fallback until it is recognized, so it can be used even
if there is no other (non-deprecated) option.

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20131104/db9061d5/attachment-0001.sig>