On 11/04/2013 04:57 PM, Martin Kletzander wrote: > 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) } > Agreed. >> 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 wanted to remove it to get rid of that special case because: * in the cases of QEMU >=1.2 where this option is supported, .lost_tick_policy should be supported as well * in the case when it's not (non-KVM QEMU 1.2), .lost_tick_policy isn't supported If QEMU decides to remove .lost_tick_policy, but keep -no-kvm-pit-reinjection, it would be good to have NO_KVM_PIT capability hardcoded, other than that, it's just one extra line in the code. > > I'd keep the fallback until it is recognized, so it can be used even > if there is no other (non-deprecated) option. Okay, I'll drop the hunk dropping that capability. Jan
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list