On 8/20/20 11:20 PM, Zhenyu Zheng wrote:
Modify virCPUarmCompare in cpu_arm.c to perform
actual compare actions. Compare host cpu vendor
and model info with guest cpu as initial implementation,
as most ARM clouds uses host-passthrogh mode.
Typo: host-passthrogh -> host-passthrough
Signed-off-by: Zhenyu Zheng <zheng.zhenyu@xxxxxxxxxxx>
---
src/cpu/cpu_arm.c | 188 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 184 insertions(+), 4 deletions(-)
diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
index 374a4d6f6c..98c5e551f5 100644
--- a/src/cpu/cpu_arm.c
+++ b/src/cpu/cpu_arm.c
@@ -118,6 +118,36 @@ virCPUarmMapNew(void)
return map;
}
+static int
+virCPUarmDataCopy(virCPUarmData *dst, const virCPUarmData *src)
+{
+ if (VIR_ALLOC(dst->features) < 0)
+ return -1;
+
+ dst->pvr = src->pvr;
+ dst->vendor_id = src->vendor_id;
+ dst->features = src->features;
+
+ return 0;
+}
+
+static virCPUDataPtr
+virCPUarmMakeCPUData(virArch arch,
+ virCPUarmData *data)
+{
+ virCPUDataPtr cpuData;
+
+ if (VIR_ALLOC(cpuData) < 0)
+ return NULL;
+
+ cpuData->arch = arch;
+
+ if (virCPUarmDataCopy(&cpuData->data.arm, data) < 0)
+ VIR_FREE(cpuData);
+
+ return cpuData;
+}
+
static void
virCPUarmDataClear(virCPUarmData *data)
{
@@ -376,6 +406,42 @@ virCPUarmModelParse(xmlXPathContextPtr ctxt,
return 0;
}
+static virCPUarmModelPtr
+virCPUarmModelCopy(virCPUarmModelPtr model)
+{
+ virCPUarmModelPtr copy;
+
+ copy = g_new0(virCPUarmModel, 1);
+ copy->name = g_strdup(model->name);
+ virCPUarmDataCopy(©->data, &model->data);
+ copy->vendor = model->vendor;
+
+ return g_steal_pointer(©);
You don't need g_steal_pointer() in this situation - you're not
attempting to VIR_FREE ou using g_autoptr() the 'copy' pointer.
'return copy' works fine in this situation.
+}
+
+static virCPUarmModelPtr
+virCPUarmModelFromCPU(const virCPUDef *cpu,
+ virCPUarmMapPtr map)
+{
+ g_autoptr(virCPUarmModel) model = NULL;
+
+ if (!cpu->model) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("no CPU model specified"));
+ return NULL;
+ }
+
+ if (!(model = virCPUarmModelFind(map, cpu->model))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unknown CPU model %s"), cpu->model);
+ return NULL;
+ }
+
+ model = virCPUarmModelCopy(model);
+
+ return g_steal_pointer(&model);
The reason g_steal_pointer() is needed here is because you're using
g_autoptr() in the initialization, but you don't need g_autoptr() in
this case. virCPUarmModelFind() will return a pointer to an existing
virCPUarmModel in 'map', then you're copying it to be able to return it
to the caller.
You can get rid of both g_autoptr() and g_steal_pointer().
+}
+
static virCPUarmMapPtr
virCPUarmLoadMap(void)
{
@@ -463,11 +529,125 @@ virCPUarmBaseline(virCPUDefPtr *cpus,
}
static virCPUCompareResult
-virCPUarmCompare(virCPUDefPtr host G_GNUC_UNUSED,
- virCPUDefPtr cpu G_GNUC_UNUSED,
- bool failMessages G_GNUC_UNUSED)
+armCompute(virCPUDefPtr host,
+ virCPUDefPtr cpu,
+ virCPUDataPtr *guestData,
+ char **message)
{
- return VIR_CPU_COMPARE_IDENTICAL;
+ virCPUarmMapPtr map = NULL;
+ virCPUarmModelPtr host_model = NULL;
+ virCPUarmModelPtr guest_model = NULL;
+ virCPUCompareResult ret = VIR_CPU_COMPARE_ERROR;
+ virArch arch;
+ size_t i;
+
+ if (cpu->arch != VIR_ARCH_NONE) {
+ bool found = false;
+
+ for (i = 0; i < G_N_ELEMENTS(archs); i++) {
+ if (archs[i] == cpu->arch) {
+ found = true;
+ break;
+ }
+ }
+
+ if (!found) {
+ VIR_DEBUG("CPU arch %s does not match host arch",
+ virArchToString(cpu->arch));
+ if (message)
+ *message = g_strdup_printf(_("CPU arch %s does not match host arch"),
+ virArchToString(cpu->arch));
+
+ ret = VIR_CPU_COMPARE_INCOMPATIBLE;
+ goto cleanup;
+ }
+ arch = cpu->arch;
+ } else {
+ arch = host->arch;
+ }
+
+ if (cpu->vendor &&
+ (!host->vendor || STRNEQ(cpu->vendor, host->vendor))) {
+ VIR_DEBUG("host CPU vendor does not match required CPU vendor %s",
+ cpu->vendor);
+ if (message) {
+ *message = g_strdup_printf(_("host CPU vendor does not match required "
+ "CPU vendor %s"),
+ cpu->vendor);
+ }
+
+ ret = VIR_CPU_COMPARE_INCOMPATIBLE;
+ goto cleanup;
+ }
+
+ if (!(map = virCPUarmLoadMap()))
+ goto cleanup;
+
+ /* Host CPU information */
+ if (!(host_model = virCPUarmModelFromCPU(host, map)))
+ goto cleanup;
+
+ /* Guest CPU information */
+ if (!(guest_model = virCPUarmModelFromCPU(cpu, map)))
+ goto cleanup;
+
+ if (STRNEQ(guest_model->name, host_model->name)) {
+ VIR_DEBUG("host CPU model does not match required CPU model %s",
+ guest_model->name);
+ if (message) {
+ *message = g_strdup_printf(_("host CPU model does not match required "
+ "CPU model %s"),
+ guest_model->name);
+ }
+
+ ret = VIR_CPU_COMPARE_INCOMPATIBLE;
+ goto cleanup;
+ }
+
+ if (guestData)
+ if (!(*guestData = virCPUarmMakeCPUData(arch, &guest_model->data)))
+ goto cleanup;
+
+ ret = VIR_CPU_COMPARE_IDENTICAL;
+
+ cleanup:
+ virCPUarmModelFree(host_model);
+ virCPUarmModelFree(guest_model);
+ return ret;
+}
+
+static virCPUCompareResult
+virCPUarmCompare(virCPUDefPtr host,
+ virCPUDefPtr cpu,
+ bool failIncompatible)
+{
+ virCPUCompareResult ret;
Extra spaces on indentation
+ char *message = NULL;
You can use g_autofree char *message and avoid the last VIR_FREE call.
Thanks,
DHB
+
+ if (!host || !host->model) {
+ if (failIncompatible) {
+ virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s",
+ _("unknown host CPU"));
+ } else {
+ VIR_WARN("unknown host CPU");
+ ret = VIR_CPU_COMPARE_INCOMPATIBLE;
+ }
+ return -1;
+ }
+
+ ret = armCompute(host, cpu, NULL, &message);
+
+ if (failIncompatible && 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;
}
static int
