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
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
