8:19 a.m.
This patch modifies the CPU comparrison function to report the
incompatibilities in more detail to ease identification of problems.
* src/cpu/cpu.h:
cpuGuestData(): Add argument to return detailed error message.
* src/cpu/cpu.c:
cpuGuestData(): Add passthrough for error argument.
* src/cpu/cpu_x86.c
x86FeatureNames(): Add function to convert a CPU definition to flag
names.
x86Compute(): - Add error message parameter
- Add macro for reporting detailed error messages.
- Improve error reporting.
- Simplify calculation of forbidden flags.
x86DataIteratorInit():
x86cpuidMatchAny(): Remove functions that are no longer needed.
* src/qemu/qemu_command.c:
qemuBuildCpuArgStr(): - Modify for new function prototype
- Add detailed error reports
- Change error code on incompatible processors
to VIR_ERR_CONFIG_UNSUPPORTED instead of
internal error
* tests/cputest.c:
cpuTestGuestData(): Modify for new function prototype
---
Sample of error message:
$ virsh start Bare
error: Failed to start domain Bare
error: unsupported configuration: guest and host CPU are not compatible: Host CPU does not
provide required features: svm, avx
src/cpu/cpu.c | 7 ++-
src/cpu/cpu.h | 6 ++-
src/cpu/cpu_x86.c | 123 +++++++++++++++++++++++++++++++----------------
src/qemu/qemu_command.c | 12 ++++-
tests/cputest.c | 2 +-
5 files changed, 100 insertions(+), 50 deletions(-)
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index 01c31bb..b8ccd24 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -248,11 +248,12 @@ cpuNodeData(const char *arch)
virCPUCompareResult
cpuGuestData(virCPUDefPtr host,
virCPUDefPtr guest,
- union cpuData **data)
+ union cpuData **data,
+ char **msg)
{
struct cpuArchDriver *driver;
- VIR_DEBUG("host=%p, guest=%p, data=%p", host, guest, data);
+ VIR_DEBUG("host=%p, guest=%p, data=%p, msg=%p", host, guest, data, msg);
if ((driver = cpuGetSubDriver(host->arch)) == NULL)
return VIR_CPU_COMPARE_ERROR;
@@ -264,7 +265,7 @@ cpuGuestData(virCPUDefPtr host,
return VIR_CPU_COMPARE_ERROR;
}
- return driver->guestData(host, guest, data);
+ return driver->guestData(host, guest, data, msg);
}
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index 9f01f17..d7bc54e 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -70,7 +70,8 @@ typedef union cpuData *
typedef virCPUCompareResult
(*cpuArchGuestData) (virCPUDefPtr host,
virCPUDefPtr guest,
- union cpuData **data);
+ union cpuData **data,
+ char **message);
typedef virCPUDefPtr
(*cpuArchBaseline) (virCPUDefPtr *cpus,
@@ -138,7 +139,8 @@ cpuNodeData (const char *arch);
extern virCPUCompareResult
cpuGuestData(virCPUDefPtr host,
virCPUDefPtr guest,
- union cpuData **data);
+ union cpuData **data,
+ char **msg);
extern char *
cpuBaselineXML(const char **xmlCPUs,
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 8d92649..e1500bf 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -31,6 +31,7 @@
#include "cpu.h"
#include "cpu_map.h"
#include "cpu_x86.h"
+#include "buf.h"
#define VIR_FROM_THIS VIR_FROM_CPU
@@ -89,16 +90,6 @@ struct data_iterator {
{ data, -1, false }
-static void
-x86DataIteratorInit(struct data_iterator *iter,
- union cpuData *data)
-{
- struct data_iterator init = DATA_ITERATOR_INIT(data);
-
- *iter = init;
-}
-
-
static int
x86cpuidMatch(const struct cpuX86cpuid *cpuid1,
const struct cpuX86cpuid *cpuid2)
@@ -121,17 +112,6 @@ x86cpuidMatchMasked(const struct cpuX86cpuid *cpuid,
}
-static int
-x86cpuidMatchAny(const struct cpuX86cpuid *cpuid,
- const struct cpuX86cpuid *mask)
-{
- return ((cpuid->eax & mask->eax) ||
- (cpuid->ebx & mask->ebx) ||
- (cpuid->ecx & mask->ecx) ||
- (cpuid->edx & mask->edx));
-}
-
-
static void
x86cpuidSetBits(struct cpuX86cpuid *cpuid,
const struct cpuX86cpuid *mask)
@@ -649,6 +629,34 @@ x86FeatureFind(const struct x86_map *map,
}
+static char *
+x86FeatureNames(const struct x86_map *map,
+ const char *separator,
+ union cpuData *data)
+{
+ virBuffer ret = VIR_BUFFER_INITIALIZER;
+ bool first = true;
+
+ struct x86_feature *next_feature = map->features;
+
+ virBufferAdd(&ret, "", 0);
+
+ while (next_feature) {
+ if (x86DataIsSubset(data, next_feature->data)) {
+ if (!first)
+ virBufferAdd(&ret, separator, -1);
+ else
+ first = false;
+
+ virBufferAdd(&ret, next_feature->name, -1);
+ }
+ next_feature = next_feature->next;
+ }
+
+ return virBufferContentAndReset(&ret);
+}
+
+
static int
x86FeatureLoad(xmlXPathContextPtr ctxt,
struct x86_map *map)
@@ -1115,10 +1123,34 @@ error:
}
+/* A helper macro to exit the cpu computation function without writing
+ * redundant code:
+ * MSG: error message
+ * CPU_DEF: a union cpuData pointer with flags that are conflicting
+ * RET: return code to set
+ *
+ * This macro generates the error string outputs it into logs.
+ */
+#define virX86CpuIncompatible(MSG, CPU_DEF) \
+ do { \
+ char *flagsStr = NULL; \
+ if (!(flagsStr = x86FeatureNames(map, ", ", (CPU_DEF)))) \
+ goto no_memory; \
+ if (message && \
+ virAsprintf(message, "%s: %s", _(MSG), flagsStr) < 0) { \
+ VIR_FREE(flagsStr); \
+ goto no_memory; \
+ } \
+ VIR_DEBUG("%s: %s", MSG, flagsStr); \
+ VIR_FREE(flagsStr); \
+ ret = VIR_CPU_COMPARE_INCOMPATIBLE; \
+ } while (0);
+
static virCPUCompareResult
x86Compute(virCPUDefPtr host,
virCPUDefPtr cpu,
- union cpuData **guest)
+ union cpuData **guest,
+ char **message)
{
struct x86_map *map = NULL;
struct x86_model *host_model = NULL;
@@ -1129,8 +1161,6 @@ x86Compute(virCPUDefPtr host,
struct x86_model *cpu_forbid = NULL;
struct x86_model *diff = NULL;
struct x86_model *guest_model = NULL;
- struct data_iterator iter;
- const struct cpuX86cpuid *cpuid;
virCPUCompareResult ret;
enum compare_result result;
unsigned int i;
@@ -1147,6 +1177,11 @@ x86Compute(virCPUDefPtr host,
if (!found) {
VIR_DEBUG("CPU arch %s does not match host arch", cpu->arch);
+ if (message &&
+ virAsprintf(message,
+ _("CPU arch %s does not match host arch"),
+ cpu->arch) < 0)
+ goto no_memory;
return VIR_CPU_COMPARE_INCOMPATIBLE;
}
}
@@ -1155,6 +1190,12 @@ x86Compute(virCPUDefPtr host,
(!host->vendor || STRNEQ(cpu->vendor, host->vendor))) {
VIR_DEBUG("host CPU vendor does not match required CPU vendor %s",
cpu->vendor);
+ if (message &&
+ virAsprintf(message,
+ _("host CPU vendor does not match required "
+ "CPU vendor %s"),
+ cpu->vendor) < 0)
+ goto no_memory;
return VIR_CPU_COMPARE_INCOMPATIBLE;
}
@@ -1167,24 +1208,19 @@ x86Compute(virCPUDefPtr host,
!(cpu_forbid = x86ModelFromCPU(cpu, map, VIR_CPU_FEATURE_FORBID)))
goto error;
- x86DataIteratorInit(&iter, cpu_forbid->data);
- while ((cpuid = x86DataCpuidNext(&iter))) {
- const struct cpuX86cpuid *cpuid2;
-
- cpuid2 = x86DataCpuid(host_model->data, cpuid->function);
- if (cpuid2 != NULL && x86cpuidMatchAny(cpuid2, cpuid)) {
- VIR_DEBUG("Host CPU provides forbidden features in CPUID function
0x%x",
- cpuid->function);
- ret = VIR_CPU_COMPARE_INCOMPATIBLE;
- goto out;
- }
+ x86DataIntersect(cpu_forbid->data, host_model->data);
+ if (!x86DataIsEmpty(cpu_forbid->data)) {
+ virX86CpuIncompatible("Host CPU provides forbidden features",
+ cpu_forbid->data);
+ goto out;
}
x86DataSubtract(cpu_require->data, cpu_disable->data);
result = x86ModelCompare(host_model, cpu_require);
if (result == SUBSET || result == UNRELATED) {
- VIR_DEBUG("Host CPU does not provide all required features");
- ret = VIR_CPU_COMPARE_INCOMPATIBLE;
+ x86DataSubtract(cpu_require->data, host_model->data);
+ virX86CpuIncompatible("Host CPU does not provide required features",
+ cpu_require->data);
goto out;
}
@@ -1204,8 +1240,9 @@ x86Compute(virCPUDefPtr host,
if (ret == VIR_CPU_COMPARE_SUPERSET
&& cpu->type == VIR_CPU_TYPE_GUEST
&& cpu->match == VIR_CPU_MATCH_STRICT) {
- VIR_DEBUG("Host CPU does not strictly match guest CPU");
- ret = VIR_CPU_COMPARE_INCOMPATIBLE;
+ virX86CpuIncompatible("Host CPU does not strictly match guest CPU: "
+ "Extra features",
+ diff->data);
goto out;
}
@@ -1246,22 +1283,24 @@ error:
ret = VIR_CPU_COMPARE_ERROR;
goto out;
}
+#undef virX86CpuIncompatible
static virCPUCompareResult
x86Compare(virCPUDefPtr host,
virCPUDefPtr cpu)
{
- return x86Compute(host, cpu, NULL);
+ return x86Compute(host, cpu, NULL, NULL);
}
static virCPUCompareResult
x86GuestData(virCPUDefPtr host,
virCPUDefPtr guest,
- union cpuData **data)
+ union cpuData **data,
+ char **message)
{
- return x86Compute(host, guest, data);
+ return x86Compute(host, guest, data, message);
}
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8dedd80..45cd417 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3704,6 +3704,7 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver,
const char *default_model;
union cpuData *data = NULL;
bool have_cpu = false;
+ char *compare_msg = NULL;
int ret = -1;
virBuffer buf = VIR_BUFFER_INITIALIZER;
int i;
@@ -3740,11 +3741,17 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver,
cpuUpdate(cpu, host) < 0)
goto cleanup;
- cmp = cpuGuestData(host, cpu, &data);
+ cmp = cpuGuestData(host, cpu, &data, &compare_msg);
switch (cmp) {
case VIR_CPU_COMPARE_INCOMPATIBLE:
- qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ if (compare_msg) {
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("guest and host CPU are not compatible:
%s"),
+ compare_msg);
+ } else {
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("guest CPU is not compatible with host CPU"));
+ }
/* fall through */
case VIR_CPU_COMPARE_ERROR:
goto cleanup;
@@ -3848,6 +3855,7 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver,
ret = 0;
cleanup:
+ VIR_FREE(compare_msg);
if (host)
cpuDataFree(host->arch, data);
virCPUDefFree(guest);
diff --git a/tests/cputest.c b/tests/cputest.c
index 01db8f1..ccc17fd 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -269,7 +269,7 @@ cpuTestGuestData(const void *arg)
!(cpu = cpuTestLoadXML(data->arch, data->name)))
goto cleanup;
- cmpResult = cpuGuestData(host, cpu, &guestData);
+ cmpResult = cpuGuestData(host, cpu, &guestData, NULL);
if (cmpResult == VIR_CPU_COMPARE_ERROR ||
cmpResult == VIR_CPU_COMPARE_INCOMPATIBLE)
goto cleanup;
--
1.7.3.4