[libvirt] [libvirt PATCH v2 04/44] Require QEMU 1.5.0

[Ján Tomko]

On Mon, Apr 09, 2018 at 05:54:17PM +0200, Andrea Bolognani wrote:
>On Thu, 2018-04-05 at 14:22 +0200, Ján Tomko wrote:
>> According to the policy described on https://libvirt.org/platforms.html
>> the QEMU versions in the oldest relevant releses are:
>
>Empty line here. Possibly indent the distros with two spaces.
>
>> SLES 12: 2.0.0
>> RHEL 7: 1.5.3
>> Ubuntu 14.04: 2.0.0
>>
>> Set the minimum to 1.5.0 and drop support for RHEL 6.
>>
>> This lets us drop the -help parsing code and assume lots of
>> capabilities.
>
>Except we already dropped the -help parsing code in the previous
>commit, and we haven't started assuming capabilities yet :)
>
>So I would use
>
>  This will let us assume lots of capabilities.
>
>here.
>
>> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
>> index 0be39b76dd..f427cfdeaa 100644
>> --- a/src/qemu/qemu_capabilities.c
>> +++ b/src/qemu/qemu_capabilities.c
>> @@ -3755,6 +3755,9 @@ virQEMUCapsProbeQMPSchemaCapabilities(virQEMUCapsPtr qemuCaps,
>>      return 0;
>>  }
>>
>> +#define QEMU_MIN_MAJOR 1
>> +#define QEMU_MIN_MINOR 5
>> +#define QEMU_MIN_MICRO 0
>>
>>  int
>>  virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
>> @@ -3785,9 +3788,12 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
>>      VIR_DEBUG("Got version %d.%d.%d (%s)",
>>                major, minor, micro, NULLSTR(package));
>>
>> -    if (major < 1 || (major == 1 && minor < 2)) {
>> -        VIR_DEBUG("Not new enough for QMP capabilities detection");
>> -        ret = 0;
>> +    if (major < QEMU_MIN_MAJOR ||
>> +        (major == QEMU_MIN_MAJOR && minor < QEMU_MIN_MINOR)) {
>> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> +                       _("QEMU version >= %d.%d.%d is required, but %d.%d.%d found"),
>> +                       QEMU_MIN_MAJOR, QEMU_MIN_MINOR, QEMU_MIN_MICRO,
>> +                       major, minor, micro);
>>          goto cleanup;
>>      }
>
>I think it would make more sense for the check and the error message
>to be converted in the previous commit, where you raise the minimum
>QEMU version to 1.2.0, so that this commit will end up only changing
>QEMU_MIN_MINOR to 5 and dropping "ret = 0" (along with the expected
>test suite churn, of course).
>
>With the comments addressed,
>
>  Reviewed-by: Andrea Bolognani <abologna at redhat.com>
>

Thanks, I have pushed the first four patches
(this time including your review feedback)

I will deal with usedQMP separately.

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180410/0c6e8e2a/attachment-0001.sig>