[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On 07/02/2013 08:15 AM, Jiri Denemark wrote:
> On Mon, Jul 01, 2013 at 18:36:11 +0200, Jano Tomko wrote:
>> Since qemu-kvm 1.1 [1] '-no-kvm-pit-reinjection' has been deprecated
>> in favor of '-global kvm-pit.lost_tick_policy=discard'
>>
>> In upstream qemu since 1.3 [2].
>>
>> 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
>> ---
>>  src/qemu/qemu_capabilities.c | 12 ++++++++++--
>>  src/qemu/qemu_capabilities.h |  1 +
>>  src/qemu/qemu_command.c      |  5 ++++-
>>  3 files changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
>> index 969b001..c6df463 100644
>> --- a/src/qemu/qemu_capabilities.c
>> +++ b/src/qemu/qemu_capabilities.c
>> @@ -233,6 +233,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
>>                "mlock",
>>  
>>                "vnc-share-policy", /* 150 */
>> +              "kvm-pit-property",
>>      );
>>  
>>  struct _virQEMUCaps {
>> @@ -2468,13 +2469,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);
>>      }
>>  
>>      ret = 0;
>> @@ -2640,6 +2640,14 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
>>      if (virQEMUCapsProbeQMPCommandLine(qemuCaps, mon) < 0)
>>          goto cleanup;
>>  
>> +    /* -global kvm-pit.lost_tick_policy=discard */
>> +    if ((qemuCaps->arch == VIR_ARCH_X86_64 ||
>> +         qemuCaps->arch == VIR_ARCH_I686) &&
>> +        (qemuCaps->version >= 1003000 ||
>> +         virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))) {
>> +        virQEMUCapsSet(qemuCaps, QEMU_CAPS_KVM_PIT_PROPERTY);
>> +    }
>> +
> 
> I think we should keep setting QEMU_CAPS_NO_KVM_PIT if QEMU is older and
> does not support QEMU_CAPS_KVM_PIT_PROPERTY. Moreover, isn't there a
> better way of checking QEMU_CAPS_KVM_PIT_PROPERTY? We should not rely on
> QEMU version unless absolutely necessary.

Of all the versions we do QMP probing for, this omits QEMU 1.2, since it
doesn't support -no-kvm-pit-reinjection, but I'm not sure if it correctly
tells non-KVM and KVM QEMU apart.

It seems there is a better way of specifying it:
-device kvm-pit,lost_tick_policy=discard

Which means it can be detected by our device probing code.

I'll send another version.

> 
>>      ret = 0;
>>  
>>  cleanup:
>> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
>> index 7088747..c64f648 100644
>> --- a/src/qemu/qemu_capabilities.h
>> +++ b/src/qemu/qemu_capabilities.h
>> @@ -189,6 +189,7 @@ enum virQEMUCapsFlags {
>>      QEMU_CAPS_DRIVE_DISCARD      = 148, /* -drive discard=off(ignore)|on(unmap) */
>>      QEMU_CAPS_MLOCK              = 149, /* -realtime mlock=on|off */
>>      QEMU_CAPS_VNC_SHARE_POLICY   = 150, /* set display sharing policy */
>> +    QEMU_CAPS_KVM_PIT_PROPERTY   = 151, /* -global kvm-pit.lost_tick_policy */
>>  
>>      QEMU_CAPS_LAST,                   /* this must always be the last item */
>>  };
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index ba93233..a678666 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -7024,7 +7024,10 @@ qemuBuildCommandLine(virConnectPtr conn,
>>              case VIR_DOMAIN_TIMER_TICKPOLICY_DELAY:
>>                  /* delay is the default if we don't have kernel
>>                     (-no-kvm-pit), otherwise, the default is catchup. */
>> -                if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_KVM_PIT))
>> +                if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM_PIT_PROPERTY))
>> +                    virCommandAddArgList(cmd, "-global",
>> +                                         "kvm-pit.lost_tick_policy=discard", NULL);
>> +                else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_KVM_PIT))
>>                      virCommandAddArg(cmd, "-no-kvm-pit-reinjection");
>>                  break;
>>              case VIR_DOMAIN_TIMER_TICKPOLICY_CATCHUP:
> 
> I'm surprised there are no changes to tests. That would make me think we
> do not test QMP caps probing, which would be bad as we need to make sure
> we don't break compatibility with older QEMU versions...

We don't test QMP caps probing AFAICT.

Jan


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]