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

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 >

> > 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.