[libvirt] [PATCH 0/6] qemu: caps: Clean up virQEMUCapsInitQMPMonitor

Some cleanups done while attempting to implement a new capability. Peter Krempa (6): qemu: caps: Separate capabilities based on qemu version qemu: caps: Aggregate all caps post-processing into a function qemu: capabilities: Move logic deciding whether to probe into probing functions qemu: caps: Don't leak package name string in virQEMUCapsInitQMPMonitor qemu: caps: Remove cleanup section in virQEMUCapsInitQMPMonitor qemu: caps: Remove pointless debug message in virQEMUCapsInitQMPMonitor src/qemu/qemu_capabilities.c | 278 +++++++++++++++++++---------------- 1 file changed, 150 insertions(+), 128 deletions(-) -- 2.20.1

virQEMUCapsInitQMPMonitor is massive now since it collects calls to the various probing functions and also version based capabilities. Split out the version based caps into a separate function. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.c | 124 +++++++++++++++++++---------------- 1 file changed, 69 insertions(+), 55 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 56228e7a36..04199b1a76 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4145,6 +4145,73 @@ virQEMUCapsInitQMPBasicArch(virQEMUCapsPtr qemuCaps) } +/** + * virQEMUCapsInitQMPVersionCaps: + * @qemuCaps: QEMU capabilities + * + * Add all QEMU capabilities based on version of QEMU. + */ +static void +virQEMUCapsInitQMPVersionCaps(virQEMUCapsPtr qemuCaps) +{ + if (qemuCaps->version >= 1006000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); + + /* vmport option is supported v2.2.0 onwards */ + if (qemuCaps->version >= 2002000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_VMPORT_OPT); + + /* -cpu ...,aarch64=off supported in v2.3.0 and onwards. But it + isn't detectable via qmp at this point */ + if (qemuCaps->arch == VIR_ARCH_AARCH64 && + qemuCaps->version >= 2003000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_CPU_AARCH64_OFF); + + /* vhost-user supports multi-queue from v2.4.0 onwards, + * but there is no way to query for that capability */ + if (qemuCaps->version >= 2004000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQUEUE); + + /* smm option is supported from v2.4.0 */ + if (qemuCaps->version >= 2004000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_SMM_OPT); + + /* sdl -gl option is supported from v2.4.0 (qemu commit id 0b71a5d5) */ + if (qemuCaps->version >= 2004000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_SDL_GL); + + /* Since 2.4.50 ARM virt machine supports gic-version option */ + if (qemuCaps->version >= 2004050) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACH_VIRT_GIC_VERSION); + + /* no way to query if -machine kernel_irqchip supports split */ + if (qemuCaps->version >= 2006000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_KERNEL_IRQCHIP_SPLIT); + + /* HPT resizing is supported since QEMU 2.10 on ppc64; unfortunately + * there's no sane way to probe for it */ + if (qemuCaps->version >= 2010000 && + ARCH_IS_PPC64(qemuCaps->arch)) { + virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT); + } + + /* '-display egl-headless' cmdline option is supported since QEMU 2.10, but + * there's no way to probe it */ + if (qemuCaps->version >= 2010000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_EGL_HEADLESS); + + /* no way to query for -numa dist */ + if (qemuCaps->version >= 2010000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_NUMA_DIST); + + /* no way to query max-cpu-compat */ + if (qemuCaps->version >= 2010000 && + ARCH_IS_PPC64(qemuCaps->arch)) { + virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT); + } +} + + static int virQEMUCapsProbeQMPSchemaCapabilities(virQEMUCapsPtr qemuCaps, qemuMonitorPtr mon) @@ -4223,61 +4290,8 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, virQEMUCapsInitQMPBasicArch(qemuCaps); - if (qemuCaps->version >= 1006000) - virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); - - /* vmport option is supported v2.2.0 onwards */ - if (qemuCaps->version >= 2002000) - virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_VMPORT_OPT); - - /* -cpu ...,aarch64=off supported in v2.3.0 and onwards. But it - isn't detectable via qmp at this point */ - if (qemuCaps->arch == VIR_ARCH_AARCH64 && - qemuCaps->version >= 2003000) - virQEMUCapsSet(qemuCaps, QEMU_CAPS_CPU_AARCH64_OFF); - - /* vhost-user supports multi-queue from v2.4.0 onwards, - * but there is no way to query for that capability */ - if (qemuCaps->version >= 2004000) - virQEMUCapsSet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQUEUE); - - /* smm option is supported from v2.4.0 */ - if (qemuCaps->version >= 2004000) - virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_SMM_OPT); - - /* sdl -gl option is supported from v2.4.0 (qemu commit id 0b71a5d5) */ - if (qemuCaps->version >= 2004000) - virQEMUCapsSet(qemuCaps, QEMU_CAPS_SDL_GL); - - /* Since 2.4.50 ARM virt machine supports gic-version option */ - if (qemuCaps->version >= 2004050) - virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACH_VIRT_GIC_VERSION); - - /* no way to query if -machine kernel_irqchip supports split */ - if (qemuCaps->version >= 2006000) - virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_KERNEL_IRQCHIP_SPLIT); - - /* HPT resizing is supported since QEMU 2.10 on ppc64; unfortunately - * there's no sane way to probe for it */ - if (qemuCaps->version >= 2010000 && - ARCH_IS_PPC64(qemuCaps->arch)) { - virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT); - } - - /* '-display egl-headless' cmdline option is supported since QEMU 2.10, but - * there's no way to probe it */ - if (qemuCaps->version >= 2010000) - virQEMUCapsSet(qemuCaps, QEMU_CAPS_EGL_HEADLESS); - - /* no way to query for -numa dist */ - if (qemuCaps->version >= 2010000) - virQEMUCapsSet(qemuCaps, QEMU_CAPS_NUMA_DIST); - - /* no way to query max-cpu-compat */ - if (qemuCaps->version >= 2010000 && - ARCH_IS_PPC64(qemuCaps->arch)) { - virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT); - } + /* initiate all capapbilities based on qemu version */ + virQEMUCapsInitQMPVersionCaps(qemuCaps); if (virQEMUCapsProbeQMPCommands(qemuCaps, mon) < 0) goto cleanup; -- 2.20.1

On Fri, Mar 29, 2019 at 01:26:29PM +0100, Peter Krempa wrote:
virQEMUCapsInitQMPMonitor is massive now since it collects calls to the various probing functions and also version based capabilities. Split out the version based caps into a separate function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>

Some caps are cleared according to some more advanced logic after detection. Split all that logic out into virQEMUCapsInitProcessCaps. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.c | 85 +++++++++++++++++++++--------------- 1 file changed, 50 insertions(+), 35 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 04199b1a76..0e48022fdb 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4212,6 +4212,55 @@ virQEMUCapsInitQMPVersionCaps(virQEMUCapsPtr qemuCaps) } +/** + * virQEMUCapsInitProcessCaps: + * @qemuCaps: QEMU capabilities + * + * Some capability bits are enabled or disabled according to specific logic. + * This function collects all capability processing after the capabilities + * are detected. + */ +static void +virQEMUCapsInitProcessCaps(virQEMUCapsPtr qemuCaps) +{ + /* 'intel-iommu' shows up as a device since 2.2.0, but can + * not be used with -device until 2.7.0. Before that it + * requires -machine iommu=on. So we must clear the device + * capability we detected on older QEMUs + */ + if (qemuCaps->version < 2007000 && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_INTEL_IOMMU)) { + virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_INTEL_IOMMU); + virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_IOMMU); + } + + /* Prealloc on NVDIMMs is broken on older QEMUs leading to + * user data corruption. If we are dealing with such version + * of QEMU pretend we don't know how to NVDIMM. */ + if (qemuCaps->version < 2009000 && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM)) + virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM); + + if (ARCH_IS_X86(qemuCaps->arch) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION)) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_CPU_CACHE); + + if (ARCH_IS_S390(qemuCaps->arch)) { + /* Legacy assurance for QEMU_CAPS_CCW */ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCW) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_CCW)) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_CCW); + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCW_CSSID_UNRESTRICTED)) + virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_VFIO_CCW); + } + + /* To avoid guest ABI regression, blockdev shall be enabled only when + * we are able to pass the custom 'device_id' for SCSI disks and cdroms. */ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_DEVICE_ID)) + virQEMUCapsClear(qemuCaps, QEMU_CAPS_BLOCKDEV); +} + + static int virQEMUCapsProbeQMPSchemaCapabilities(virQEMUCapsPtr qemuCaps, qemuMonitorPtr mon) @@ -4320,17 +4369,6 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (virQEMUCapsProbeQMPHostCPU(qemuCaps, mon, false) < 0) goto cleanup; - /* 'intel-iommu' shows up as a device since 2.2.0, but can - * not be used with -device until 2.7.0. Before that it - * requires -machine iommu=on. So we must clear the device - * capability we detected on older QEMUs - */ - if (qemuCaps->version < 2007000 && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_INTEL_IOMMU)) { - virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_INTEL_IOMMU); - virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_IOMMU); - } - /* GIC capabilities, eg. available GIC versions */ if ((qemuCaps->arch == VIR_ARCH_AARCH64 || qemuCaps->arch == VIR_ARCH_ARMV6L || @@ -4338,26 +4376,6 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, virQEMUCapsProbeQMPGICCapabilities(qemuCaps, mon) < 0) goto cleanup; - /* Prealloc on NVDIMMs is broken on older QEMUs leading to - * user data corruption. If we are dealing with such version - * of QEMU pretend we don't know how to NVDIMM. */ - if (qemuCaps->version < 2009000 && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM)) - virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM); - - if (ARCH_IS_X86(qemuCaps->arch) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION)) - virQEMUCapsSet(qemuCaps, QEMU_CAPS_CPU_CACHE); - - if (ARCH_IS_S390(qemuCaps->arch)) { - /* Legacy assurance for QEMU_CAPS_CCW */ - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCW) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_CCW)) - virQEMUCapsSet(qemuCaps, QEMU_CAPS_CCW); - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCW_CSSID_UNRESTRICTED)) - virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_VFIO_CCW); - } - /* Probe for SEV capabilities */ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV_GUEST)) { int rc = virQEMUCapsProbeQMPSEVCapabilities(qemuCaps, mon); @@ -4369,10 +4387,7 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, virQEMUCapsClear(qemuCaps, QEMU_CAPS_SEV_GUEST); } - /* To avoid guest ABI regression, blockdev shall be enabled only when - * we are able to pass the custom 'device_id' for SCSI disks and cdroms. */ - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_DEVICE_ID)) - virQEMUCapsClear(qemuCaps, QEMU_CAPS_BLOCKDEV); + virQEMUCapsInitProcessCaps(qemuCaps); ret = 0; cleanup: -- 2.20.1

On Fri, Mar 29, 2019 at 01:26:30PM +0100, Peter Krempa wrote:
Some caps are cleared according to some more advanced logic after detection. Split all that logic out into virQEMUCapsInitProcessCaps.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>

Most probing functions in virQEMUCapsInitQMPMonitor decide internally if there's anything to do and return success if there isn't. Move the decision logic for virQEMUCapsProbeQMPGICCapabilities, virQEMUCapsProbeQMPSEVCapabilities, and virQEMUCapsProbeQMPSchemaCapabilities into the function itself so that virQEMUCapsInitQMPMonitor looks tidy. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.c | 47 ++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 0e48022fdb..4c52dfc714 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2738,6 +2738,11 @@ virQEMUCapsProbeQMPGICCapabilities(virQEMUCapsPtr qemuCaps, virGICCapability *caps = NULL; int ncaps; + if (!(qemuCaps->arch == VIR_ARCH_AARCH64 || + qemuCaps->arch == VIR_ARCH_ARMV6L || + qemuCaps->arch == VIR_ARCH_ARMV7L)) + return 0; + if ((ncaps = qemuMonitorGetGICCapabilities(mon, &caps)) < 0) return -1; @@ -2747,7 +2752,6 @@ virQEMUCapsProbeQMPGICCapabilities(virQEMUCapsPtr qemuCaps, } -/* Returns -1 on error, 0 if SEV is not supported, 1 if SEV is supported */ static int virQEMUCapsProbeQMPSEVCapabilities(virQEMUCapsPtr qemuCaps, qemuMonitorPtr mon) @@ -2755,12 +2759,21 @@ virQEMUCapsProbeQMPSEVCapabilities(virQEMUCapsPtr qemuCaps, int rc = -1; virSEVCapability *caps = NULL; - if ((rc = qemuMonitorGetSEVCapabilities(mon, &caps)) <= 0) - return rc; + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV_GUEST)) + return 0; + + if ((rc = qemuMonitorGetSEVCapabilities(mon, &caps)) < 0) + return -1; + + /* SEV isn't actually supported */ + if (rc == 0) { + virQEMUCapsClear(qemuCaps, QEMU_CAPS_SEV_GUEST); + return 0; + } virSEVCapabilitiesFree(qemuCaps->sevCapabilities); qemuCaps->sevCapabilities = caps; - return rc; + return 0; } @@ -4270,6 +4283,9 @@ virQEMUCapsProbeQMPSchemaCapabilities(virQEMUCapsPtr qemuCaps, virHashTablePtr schema = NULL; size_t i; + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_QMP_SCHEMA)) + return 0; + if (!(schemareply = qemuMonitorQueryQMPSchema(mon))) return -1; @@ -4363,29 +4379,14 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, goto cleanup; if (virQEMUCapsProbeQMPMigrationCapabilities(qemuCaps, mon) < 0) goto cleanup; - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_QMP_SCHEMA) && - virQEMUCapsProbeQMPSchemaCapabilities(qemuCaps, mon) < 0) + if (virQEMUCapsProbeQMPSchemaCapabilities(qemuCaps, mon) < 0) goto cleanup; if (virQEMUCapsProbeQMPHostCPU(qemuCaps, mon, false) < 0) goto cleanup; - - /* GIC capabilities, eg. available GIC versions */ - if ((qemuCaps->arch == VIR_ARCH_AARCH64 || - qemuCaps->arch == VIR_ARCH_ARMV6L || - qemuCaps->arch == VIR_ARCH_ARMV7L) && - virQEMUCapsProbeQMPGICCapabilities(qemuCaps, mon) < 0) + if (virQEMUCapsProbeQMPGICCapabilities(qemuCaps, mon) < 0) + goto cleanup; + if (virQEMUCapsProbeQMPSEVCapabilities(qemuCaps, mon) < 0) goto cleanup; - - /* Probe for SEV capabilities */ - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV_GUEST)) { - int rc = virQEMUCapsProbeQMPSEVCapabilities(qemuCaps, mon); - - if (rc < 0) - goto cleanup; - - if (rc == 0) - virQEMUCapsClear(qemuCaps, QEMU_CAPS_SEV_GUEST); - } virQEMUCapsInitProcessCaps(qemuCaps); -- 2.20.1

On Fri, Mar 29, 2019 at 01:26:31PM +0100, Peter Krempa wrote:
Most probing functions in virQEMUCapsInitQMPMonitor decide internally if there's anything to do and return success if there isn't. Move the decision logic for virQEMUCapsProbeQMPGICCapabilities, virQEMUCapsProbeQMPSEVCapabilities, and virQEMUCapsProbeQMPSchemaCapabilities into the function itself so that virQEMUCapsInitQMPMonitor looks tidy.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> ---
You might want to split the adjustments into 3 patches, your call. Reviewed-by: Erik Skultety <eskultet@redhat.com>

If the detected qemu version is below our required version 'package' would be leaked. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 4c52dfc714..e3db7ce71c 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4322,7 +4322,7 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, { int ret = -1; int major, minor, micro; - char *package = NULL; + VIR_AUTOFREE(char *) package = NULL; /* @mon is supposed to be locked by callee */ @@ -4347,7 +4347,7 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, } qemuCaps->version = major * 1000000 + minor * 1000 + micro; - qemuCaps->package = package; + VIR_STEAL_PTR(qemuCaps->package, package); qemuCaps->usedQMP = true; if (virQEMUCapsInitQMPArch(qemuCaps, mon) < 0) -- 2.20.1

On Fri, Mar 29, 2019 at 01:26:32PM +0100, Peter Krempa wrote:
If the detected qemu version is below our required version 'package' would be leaked.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.c | 37 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index e3db7ce71c..2127cfb85c 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4320,7 +4320,6 @@ int virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, qemuMonitorPtr mon) { - int ret = -1; int major, minor, micro; VIR_AUTOFREE(char *) package = NULL; @@ -4331,7 +4330,7 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, &package) < 0) { VIR_DEBUG("Failed to query monitor version %s", virGetLastErrorMessage()); - goto cleanup; + return -1; } VIR_DEBUG("Got version %d.%d.%d (%s)", @@ -4343,7 +4342,7 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, _("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; + return -1; } qemuCaps->version = major * 1000000 + minor * 1000 + micro; @@ -4351,7 +4350,7 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, qemuCaps->usedQMP = true; if (virQEMUCapsInitQMPArch(qemuCaps, mon) < 0) - goto cleanup; + return -1; virQEMUCapsInitQMPBasicArch(qemuCaps); @@ -4359,40 +4358,38 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, virQEMUCapsInitQMPVersionCaps(qemuCaps); if (virQEMUCapsProbeQMPCommands(qemuCaps, mon) < 0) - goto cleanup; + return -1; /* Some capabilities may differ depending on KVM state */ if (virQEMUCapsProbeQMPKVMState(qemuCaps, mon) < 0) - goto cleanup; + return -1; if (virQEMUCapsProbeQMPEvents(qemuCaps, mon) < 0) - goto cleanup; + return -1; if (virQEMUCapsProbeQMPDevices(qemuCaps, mon) < 0) - goto cleanup; + return -1; if (virQEMUCapsProbeQMPMachineTypes(qemuCaps, mon) < 0) - goto cleanup; + return -1; if (virQEMUCapsProbeQMPCPUDefinitions(qemuCaps, mon, false) < 0) - goto cleanup; + return -1; if (virQEMUCapsProbeQMPTPM(qemuCaps, mon) < 0) - goto cleanup; + return -1; if (virQEMUCapsProbeQMPCommandLine(qemuCaps, mon) < 0) - goto cleanup; + return -1; if (virQEMUCapsProbeQMPMigrationCapabilities(qemuCaps, mon) < 0) - goto cleanup; + return -1; if (virQEMUCapsProbeQMPSchemaCapabilities(qemuCaps, mon) < 0) - goto cleanup; + return -1; if (virQEMUCapsProbeQMPHostCPU(qemuCaps, mon, false) < 0) - goto cleanup; + return -1; if (virQEMUCapsProbeQMPGICCapabilities(qemuCaps, mon) < 0) - goto cleanup; + return -1; if (virQEMUCapsProbeQMPSEVCapabilities(qemuCaps, mon) < 0) - goto cleanup; + return -1; virQEMUCapsInitProcessCaps(qemuCaps); - ret = 0; - cleanup: - return ret; + return 0; } -- 2.20.1

On Fri, Mar 29, 2019 at 01:26:33PM +0100, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> ---
Let's include the TCG equivalent of the same function. Reviewed-by: Erik Skultety <eskultet@redhat.com>

Failure of qemuMonitorGetVersion is fatal now that we only support QMP based qemus. Remove the debug message since we report an error already. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 2127cfb85c..eb91295f93 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4325,13 +4325,8 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, /* @mon is supposed to be locked by callee */ - if (qemuMonitorGetVersion(mon, - &major, &minor, µ, - &package) < 0) { - VIR_DEBUG("Failed to query monitor version %s", - virGetLastErrorMessage()); + if (qemuMonitorGetVersion(mon, &major, &minor, µ, &package) < 0) return -1; - } VIR_DEBUG("Got version %d.%d.%d (%s)", major, minor, micro, NULLSTR(package)); -- 2.20.1

On Fri, Mar 29, 2019 at 01:26:34PM +0100, Peter Krempa wrote:
Failure of qemuMonitorGetVersion is fatal now that we only support QMP based qemus. Remove the debug message since we report an error already.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>
participants (2)
-
Erik Skultety
-
Peter Krempa