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

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



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 redhat com>


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

I will deal with usedQMP separately.

Jano

Attachment: signature.asc
Description: Digital signature


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