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@xxxxxxxxxx>
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
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
