[libvirt] [PATCH v3] qemu: don't use deprecated -no-kvm-pit-reinjection
Ján Tomko
jtomko at redhat.com
Tue Nov 5 10:13:09 UTC 2013
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20131105/9b8d982d/attachment-0001.sig>
More information about the libvir-list
mailing list