[libvirt] [PATCH 0/2] Add support for reporting failure on incompatible CPUs

When CPU virConnectCompareCPU returns VIR_CPU_COMPARE_INCOMPATIBLE, the caller has no clue why the CPU is considered incompatible with host CPU. And in some cases, it would be nice to be able to get such info in a client rather than having to look in logs. Jiri Denemark (2): cpuCompare*: Add support for reporting failure on incompatible CPUs virConnectCompareCPU: Introduce FAIL_INCOMPATIBLE flag include/libvirt/libvirt.h.in | 5 +++++ include/libvirt/virterror.h | 2 ++ src/bhyve/bhyve_driver.c | 17 +++++++++++++---- src/cpu/cpu.c | 10 ++++++---- src/cpu/cpu.h | 9 ++++++--- src/cpu/cpu_aarch64.c | 3 ++- src/cpu/cpu_arm.c | 3 ++- src/cpu/cpu_generic.c | 15 +++++++++++---- src/cpu/cpu_powerpc.c | 10 ++++++++-- src/cpu/cpu_x86.c | 20 ++++++++++++++++++-- src/libvirt.c | 9 +++++++-- src/qemu/qemu_driver.c | 17 +++++++++++++---- src/util/virerror.c | 6 ++++++ tests/cputest.c | 4 ++-- tools/virsh-domain.c | 11 +++++++++-- 15 files changed, 110 insertions(+), 31 deletions(-) -- 2.0.0

When CPU comparison APIs return VIR_CPU_COMPARE_INCOMPATIBLE, the caller has no clue why the CPU is considered incompatible with host CPU. And in some cases, it would be nice to be able to get such info in a client rather than having to look in logs. To achieve this, the APIs can be told to return VIR_ERR_CPU_INCOMPATIBLE error for incompatible CPUs and the reason will be described in the associated error message. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- include/libvirt/virterror.h | 2 ++ src/bhyve/bhyve_driver.c | 2 +- src/cpu/cpu.c | 10 ++++++---- src/cpu/cpu.h | 9 ++++++--- src/cpu/cpu_aarch64.c | 3 ++- src/cpu/cpu_arm.c | 3 ++- src/cpu/cpu_generic.c | 15 +++++++++++---- src/cpu/cpu_powerpc.c | 10 ++++++++-- src/cpu/cpu_x86.c | 20 ++++++++++++++++++-- src/qemu/qemu_driver.c | 2 +- src/util/virerror.c | 6 ++++++ tests/cputest.c | 4 ++-- 12 files changed, 65 insertions(+), 21 deletions(-) diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index be90797..15ba4f1 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -300,6 +300,8 @@ typedef enum { was denied */ VIR_ERR_DBUS_SERVICE = 89, /* error from a dbus service */ VIR_ERR_STORAGE_VOL_EXIST = 90, /* the storage vol already exists */ + VIR_ERR_CPU_INCOMPATIBLE = 91, /* given CPU is incompatible with host + CPU*/ } virErrorNumber; /** diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index bb9bcb7..9bece84 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -1332,7 +1332,7 @@ bhyveConnectCompareCPU(virConnectPtr conn, VIR_WARN("cannot get host CPU capabilities"); ret = VIR_CPU_COMPARE_INCOMPATIBLE; } else { - ret = cpuCompareXML(caps->host.cpu, xmlDesc); + ret = cpuCompareXML(caps->host.cpu, xmlDesc, false); } cleanup: diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index c3d66dd..08bec5e 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -92,7 +92,8 @@ cpuGetSubDriver(virArch arch) */ virCPUCompareResult cpuCompareXML(virCPUDefPtr host, - const char *xml) + const char *xml, + bool failIncompatible) { xmlDocPtr doc = NULL; xmlXPathContextPtr ctxt = NULL; @@ -108,7 +109,7 @@ cpuCompareXML(virCPUDefPtr host, if (cpu == NULL) goto cleanup; - ret = cpuCompare(host, cpu); + ret = cpuCompare(host, cpu, failIncompatible); cleanup: virCPUDefFree(cpu); @@ -134,7 +135,8 @@ cpuCompareXML(virCPUDefPtr host, */ virCPUCompareResult cpuCompare(virCPUDefPtr host, - virCPUDefPtr cpu) + virCPUDefPtr cpu, + bool failIncompatible) { struct cpuArchDriver *driver; @@ -156,7 +158,7 @@ cpuCompare(virCPUDefPtr host, return VIR_CPU_COMPARE_ERROR; } - return driver->compare(host, cpu); + return driver->compare(host, cpu, failIncompatible); } diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index e9f2713..339964c 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -46,7 +46,8 @@ struct _virCPUData { typedef virCPUCompareResult (*cpuArchCompare) (virCPUDefPtr host, - virCPUDefPtr cpu); + virCPUDefPtr cpu, + bool failIncompatible); typedef int (*cpuArchDecode) (virCPUDefPtr cpu, @@ -119,12 +120,14 @@ struct cpuArchDriver { extern virCPUCompareResult cpuCompareXML(virCPUDefPtr host, - const char *xml) + const char *xml, + bool failIncompatible) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); extern virCPUCompareResult cpuCompare (virCPUDefPtr host, - virCPUDefPtr cpu) + virCPUDefPtr cpu, + bool failIncompatible) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); extern int diff --git a/src/cpu/cpu_aarch64.c b/src/cpu/cpu_aarch64.c index 7255d9f..6346f9b 100644 --- a/src/cpu/cpu_aarch64.c +++ b/src/cpu/cpu_aarch64.c @@ -110,7 +110,8 @@ AArch64Baseline(virCPUDefPtr *cpus, static virCPUCompareResult AArch64Compare(virCPUDefPtr host ATTRIBUTE_UNUSED, - virCPUDefPtr cpu ATTRIBUTE_UNUSED) + virCPUDefPtr cpu ATTRIBUTE_UNUSED, + bool failIncompatible ATTRIBUTE_UNUSED) { return VIR_CPU_COMPARE_IDENTICAL; } diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index 39e8f12..ec755bd 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -113,7 +113,8 @@ ArmBaseline(virCPUDefPtr *cpus, static virCPUCompareResult ArmCompare(virCPUDefPtr host ATTRIBUTE_UNUSED, - virCPUDefPtr cpu ATTRIBUTE_UNUSED) + virCPUDefPtr cpu ATTRIBUTE_UNUSED, + bool failMessages ATTRIBUTE_UNUSED) { return VIR_CPU_COMPARE_IDENTICAL; } diff --git a/src/cpu/cpu_generic.c b/src/cpu/cpu_generic.c index f115c40..d6890c0 100644 --- a/src/cpu/cpu_generic.c +++ b/src/cpu/cpu_generic.c @@ -57,17 +57,20 @@ genericHashFeatures(virCPUDefPtr cpu) static virCPUCompareResult genericCompare(virCPUDefPtr host, - virCPUDefPtr cpu) + virCPUDefPtr cpu, + bool failIncompatible) { - virHashTablePtr hash; + virHashTablePtr hash = NULL; virCPUCompareResult ret = VIR_CPU_COMPARE_ERROR; size_t i; unsigned int reqfeatures; if ((cpu->arch != VIR_ARCH_NONE && host->arch != cpu->arch) || - STRNEQ(host->model, cpu->model)) - return VIR_CPU_COMPARE_INCOMPATIBLE; + STRNEQ(host->model, cpu->model)) { + ret = VIR_CPU_COMPARE_INCOMPATIBLE; + goto cleanup; + } if ((hash = genericHashFeatures(host)) == NULL) goto cleanup; @@ -102,6 +105,10 @@ genericCompare(virCPUDefPtr host, cleanup: virHashFree(hash); + if (failIncompatible && ret == VIR_CPU_COMPARE_INCOMPATIBLE) { + ret = VIR_CPU_COMPARE_ERROR; + virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL); + } return ret; } diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c index 05fa55b..67cb9ff 100644 --- a/src/cpu/cpu_powerpc.c +++ b/src/cpu/cpu_powerpc.c @@ -440,13 +440,19 @@ ppcCompute(virCPUDefPtr host, static virCPUCompareResult ppcCompare(virCPUDefPtr host, - virCPUDefPtr cpu) + virCPUDefPtr cpu, + bool failIncompatible) { if ((cpu->arch == VIR_ARCH_NONE || host->arch == cpu->arch) && STREQ(host->model, cpu->model)) return VIR_CPU_COMPARE_IDENTICAL; - return VIR_CPU_COMPARE_INCOMPATIBLE; + if (failIncompatible) { + virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL); + return VIR_CPU_COMPARE_ERROR; + } else { + return VIR_CPU_COMPARE_INCOMPATIBLE; + } } static int diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 72c5216..235fa49 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1463,9 +1463,25 @@ x86Compute(virCPUDefPtr host, static virCPUCompareResult x86Compare(virCPUDefPtr host, - virCPUDefPtr cpu) + virCPUDefPtr cpu, + bool failIncomaptible) { - return x86Compute(host, cpu, NULL, NULL); + virCPUCompareResult ret; + char *message = NULL; + + ret = x86Compute(host, cpu, NULL, &message); + + if (failIncomaptible && ret == VIR_CPU_COMPARE_INCOMPATIBLE) { + ret = VIR_CPU_COMPARE_ERROR; + if (message) { + virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s", message); + } else { + virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL); + } + } + VIR_FREE(message); + + return ret; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 22699c1..3c23fc7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11528,7 +11528,7 @@ qemuConnectCompareCPU(virConnectPtr conn, VIR_WARN("cannot get host CPU capabilities"); ret = VIR_CPU_COMPARE_INCOMPATIBLE; } else { - ret = cpuCompareXML(caps->host.cpu, xmlDesc); + ret = cpuCompareXML(caps->host.cpu, xmlDesc, false); } cleanup: diff --git a/src/util/virerror.c b/src/util/virerror.c index e0bc970..6bd3d09 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -1277,6 +1277,12 @@ virErrorMsg(virErrorNumber error, const char *info) else errmsg = _("error from service: %s"); break; + case VIR_ERR_CPU_INCOMPATIBLE: + if (info == NULL) + errmsg = _("the CPU is incompatible with host CPU"); + else + errmsg = _("the CPU is incompatible with host CPU: %s"); + break; } return errmsg; } diff --git a/tests/cputest.c b/tests/cputest.c index 3766c2f..38cd71e 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -228,7 +228,7 @@ cpuTestCompare(const void *arg) !(cpu = cpuTestLoadXML(data->arch, data->name))) goto cleanup; - result = cpuCompare(host, cpu); + result = cpuCompare(host, cpu, false); if (data->result == VIR_CPU_COMPARE_ERROR) virResetLastError(); @@ -357,7 +357,7 @@ cpuTestBaseline(const void *arg) for (i = 0; i < ncpus; i++) { virCPUCompareResult cmp; - cmp = cpuCompare(cpus[i], baseline); + cmp = cpuCompare(cpus[i], baseline, false); if (cmp != VIR_CPU_COMPARE_SUPERSET && cmp != VIR_CPU_COMPARE_IDENTICAL) { if (virTestGetVerbose()) { -- 2.0.0

On 06/25/14 14:34, Jiri Denemark wrote:
When CPU comparison APIs return VIR_CPU_COMPARE_INCOMPATIBLE, the caller has no clue why the CPU is considered incompatible with host CPU. And in some cases, it would be nice to be able to get such info in a client rather than having to look in logs.
To achieve this, the APIs can be told to return VIR_ERR_CPU_INCOMPATIBLE error for incompatible CPUs and the reason will be described in the associated error message.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- include/libvirt/virterror.h | 2 ++ src/bhyve/bhyve_driver.c | 2 +- src/cpu/cpu.c | 10 ++++++---- src/cpu/cpu.h | 9 ++++++--- src/cpu/cpu_aarch64.c | 3 ++- src/cpu/cpu_arm.c | 3 ++- src/cpu/cpu_generic.c | 15 +++++++++++---- src/cpu/cpu_powerpc.c | 10 ++++++++-- src/cpu/cpu_x86.c | 20 ++++++++++++++++++-- src/qemu/qemu_driver.c | 2 +- src/util/virerror.c | 6 ++++++ tests/cputest.c | 4 ++-- 12 files changed, 65 insertions(+), 21 deletions(-)
ACK, Peter

The new VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE flag for virConnectCompareCPU can be used to get an error (VIR_ERR_CPU_INCOMPATIBLE) describing the incompatibility instead of the usual VIR_CPU_COMPARE_INCOMPATIBLE return code. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- include/libvirt/libvirt.h.in | 5 +++++ src/bhyve/bhyve_driver.c | 17 +++++++++++++---- src/libvirt.c | 9 +++++++-- src/qemu/qemu_driver.c | 17 +++++++++++++---- tools/virsh-domain.c | 11 +++++++++-- 5 files changed, 47 insertions(+), 12 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 3f7a201..594521e 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4122,6 +4122,11 @@ typedef enum { #endif } virCPUCompareResult; +typedef enum { + VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE = (1 << 0), /* treat incompatible + CPUs as failure */ +} virConnectCompareCPUFlags; + int virConnectCompareCPU(virConnectPtr conn, const char *xmlDesc, unsigned int flags); diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 9bece84..d784ed1 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -1318,21 +1318,30 @@ bhyveConnectCompareCPU(virConnectPtr conn, bhyveConnPtr driver = conn->privateData; int ret = VIR_CPU_COMPARE_ERROR; virCapsPtr caps = NULL; + bool failIncompatible; - virCheckFlags(0, VIR_CPU_COMPARE_ERROR); + virCheckFlags(VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE, + VIR_CPU_COMPARE_ERROR); if (virConnectCompareCPUEnsureACL(conn) < 0) goto cleanup; + failIncomaptible = !!(flags & VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE); + if (!(caps = bhyveDriverGetCapabilities(driver))) goto cleanup; if (!caps->host.cpu || !caps->host.cpu->model) { - VIR_WARN("cannot get host CPU capabilities"); - ret = VIR_CPU_COMPARE_INCOMPATIBLE; + if (failIncomaptible) { + virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s", + _("cannot get host CPU capabilities")); + } else { + VIR_WARN("cannot get host CPU capabilities"); + ret = VIR_CPU_COMPARE_INCOMPATIBLE; + } } else { - ret = cpuCompareXML(caps->host.cpu, xmlDesc, false); + ret = cpuCompareXML(caps->host.cpu, xmlDesc, failIncomaptible); } cleanup: diff --git a/src/libvirt.c b/src/libvirt.c index a0cdfa2..48ce225 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -17245,11 +17245,16 @@ virConnectIsSecure(virConnectPtr conn) * virConnectCompareCPU: * @conn: virConnect connection * @xmlDesc: XML describing the CPU to compare with host CPU - * @flags: extra flags; not used yet, so callers should always pass 0 + * @flags: bitwise-OR of virConnectCompareCPUFlags * * Compares the given CPU description with the host CPU * - * Returns comparison result according to enum virCPUCompareResult + * Returns comparison result according to enum virCPUCompareResult. If + * VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE is used and @xmlDesc CPU is + * incompatible with host CPU, this function will return VIR_CPU_COMPARE_ERROR + * (instead of VIR_CPU_COMPARE_INCOMPATIBLE) and the error will use + * VIR_ERR_CPU_INCOMPATIBLE code the error message will provide more details + * about the incompatibility. */ int virConnectCompareCPU(virConnectPtr conn, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3c23fc7..d8cecff 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11514,21 +11514,30 @@ qemuConnectCompareCPU(virConnectPtr conn, virQEMUDriverPtr driver = conn->privateData; int ret = VIR_CPU_COMPARE_ERROR; virCapsPtr caps = NULL; + bool failIncomaptible; - virCheckFlags(0, VIR_CPU_COMPARE_ERROR); + virCheckFlags(VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE, + VIR_CPU_COMPARE_ERROR); if (virConnectCompareCPUEnsureACL(conn) < 0) goto cleanup; + failIncomaptible = !!(flags & VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE); + if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; if (!caps->host.cpu || !caps->host.cpu->model) { - VIR_WARN("cannot get host CPU capabilities"); - ret = VIR_CPU_COMPARE_INCOMPATIBLE; + if (failIncomaptible) { + virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s", + _("cannot get host CPU capabilities")); + } else { + VIR_WARN("cannot get host CPU capabilities"); + ret = VIR_CPU_COMPARE_INCOMPATIBLE; + } } else { - ret = cpuCompareXML(caps->host.cpu, xmlDesc, false); + ret = cpuCompareXML(caps->host.cpu, xmlDesc, failIncomaptible); } cleanup: diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 0ae1538..f55dae4 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6214,6 +6214,10 @@ static const vshCmdOptDef opts_cpu_compare[] = { .flags = VSH_OFLAG_REQ, .help = N_("file containing an XML CPU description") }, + {.name = "error", + .type = VSH_OT_BOOL, + .help = N_("report error if CPUs are incompatible") + }, {.name = NULL} }; @@ -6225,11 +6229,14 @@ cmdCPUCompare(vshControl *ctl, const vshCmd *cmd) char *buffer; int result; char *snippet = NULL; - + unsigned int flags = 0; xmlDocPtr xml = NULL; xmlXPathContextPtr ctxt = NULL; xmlNodePtr node; + if (vshCommandOptBool(cmd, "error")) + flags |= VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE; + if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) return false; @@ -6253,7 +6260,7 @@ cmdCPUCompare(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - result = virConnectCompareCPU(ctl->conn, snippet, 0); + result = virConnectCompareCPU(ctl->conn, snippet, flags); switch (result) { case VIR_CPU_COMPARE_INCOMPATIBLE: -- 2.0.0

On 06/25/14 14:34, Jiri Denemark wrote:
The new VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE flag for virConnectCompareCPU can be used to get an error (VIR_ERR_CPU_INCOMPATIBLE) describing the incompatibility instead of the usual VIR_CPU_COMPARE_INCOMPATIBLE return code.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- include/libvirt/libvirt.h.in | 5 +++++ src/bhyve/bhyve_driver.c | 17 +++++++++++++---- src/libvirt.c | 9 +++++++-- src/qemu/qemu_driver.c | 17 +++++++++++++---- tools/virsh-domain.c | 11 +++++++++-- 5 files changed, 47 insertions(+), 12 deletions(-)
virsh man page change is missing. Otherwise looks good to me. ACK if you add the man page section Peter

On 06/25/2014 06:34 AM, Jiri Denemark wrote:
The new VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE flag for virConnectCompareCPU can be used to get an error (VIR_ERR_CPU_INCOMPATIBLE) describing the incompatibility instead of the usual VIR_CPU_COMPARE_INCOMPATIBLE return code.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> ---
* - * Returns comparison result according to enum virCPUCompareResult + * Returns comparison result according to enum virCPUCompareResult. If + * VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE is used and @xmlDesc CPU is + * incompatible with host CPU, this function will return VIR_CPU_COMPARE_ERROR + * (instead of VIR_CPU_COMPARE_INCOMPATIBLE) and the error will use + * VIR_ERR_CPU_INCOMPATIBLE code the error message will provide more details + * about the incompatibility.
Reads better if you change the tail to: the error will use the VIR_ERR_CPU_INCOMPATIBLE code with a message providing more details about the incompatibility -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Wed, Jun 25, 2014 at 14:34:42 +0200, Jiri Denemark wrote:
When CPU virConnectCompareCPU returns VIR_CPU_COMPARE_INCOMPATIBLE, the caller has no clue why the CPU is considered incompatible with host CPU. And in some cases, it would be nice to be able to get such info in a client rather than having to look in logs.
Jiri Denemark (2): cpuCompare*: Add support for reporting failure on incompatible CPUs virConnectCompareCPU: Introduce FAIL_INCOMPATIBLE flag
I updated the virsh man page as requested by Peter, amended virConnectCompareCPU docs as suggested by Eric and pushed this series. Jirka
participants (3)
-
Eric Blake
-
Jiri Denemark
-
Peter Krempa