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.zhe...@outlook.com>
---
  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(&copy->data, &model->data);
+    copy->vendor = model->vendor;
+
+    return g_steal_pointer(&copy);


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


Reply via email to