On 10/16/19 3:21 PM, Collin Walling wrote:

Providing an erroneous CPU definition in the XML file provided to the hypervisor-cpu-compare/baseline command will result in a verbose internal error. Let's add some sanity checking before executing the QMP commands to provide a cleaner message if something is wrong with the CPU model or features.

An internal error will still appear for other messages originating from QEMU e.g. if a newer feature is not available for an older CPU model.

Signed-off-by: Collin Walling <walling@linux.ibm.com> ---

Pure code review since I don't have a proper env to test the hypervisor-cpu-compare command: Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

src/qemu/qemu_driver.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2e422b5..a4f916b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13645,6 +13645,52 @@ qemuConnectCompareCPU(virConnectPtr conn, }

+static int +qemuConnectCheckCPUModel(qemuMonitorPtr mon, + virCPUDefPtr cpu) +{ + qemuMonitorCPUModelInfoPtr modelInfo = NULL; + qemuMonitorCPUModelExpansionType type; + size_t i, j; + int ret = -1; + + /* Collect CPU model name and features known by QEMU */ + type = QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL; + if (qemuMonitorGetCPUModelExpansion(mon, type, cpu, true, + false, &modelInfo) < 0) + goto cleanup; + + /* Sanity check CPU model */ + if (!modelInfo) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unknown CPU model: %s"), cpu->model); + goto cleanup; + } + + /* Sanity check CPU features */ + for (i = 0; i < cpu->nfeatures; i++) { + const char *feat = cpu->features[i].name; + + for (j = 0; j < modelInfo->nprops; j++) { + const char *prop = modelInfo->props[j].name; + if (STREQ(feat, prop)) + break; + } + + if (j == modelInfo->nprops) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unknown CPU feature: %s"), feat); + goto cleanup; + } + } + ret = 0; + + cleanup: + qemuMonitorCPUModelInfoFree(modelInfo); + return ret; +} + + static virCPUCompareResult qemuConnectCPUModelComparison(virQEMUCapsPtr qemuCaps, const char *libDir, @@ -13665,6 +13711,11 @@ qemuConnectCPUModelComparison(virQEMUCapsPtr qemuCaps, if (qemuProcessQMPStart(proc) < 0) goto cleanup;

+ if (qemuConnectCheckCPUModel(proc->mon, cpu_a) < 0 || + qemuConnectCheckCPUModel(proc->mon, cpu_b) < 0) { + goto cleanup; + } + if (qemuMonitorGetCPUModelComparison(proc->mon, cpu_a, cpu_b, &result) < 0) goto cleanup;

@@ -13863,6 +13914,9 @@ qemuConnectCPUModelBaseline(virQEMUCapsPtr qemuCaps, if (qemuProcessQMPStart(proc) < 0) goto cleanup;

+ if (qemuConnectCheckCPUModel(proc->mon, cpus[0]) < 0) + goto cleanup; + if (VIR_ALLOC(baseline) < 0) goto cleanup;

@@ -13870,6 +13924,9 @@ qemuConnectCPUModelBaseline(virQEMUCapsPtr qemuCaps, goto cleanup;

for (i = 1; i < ncpus; i++) { + if (qemuConnectCheckCPUModel(proc->mon, cpus[i]) < 0) + goto cleanup; + if (qemuMonitorGetCPUModelBaseline(proc->mon, baseline, cpus[i], &result) < 0) goto cleanup;