[PATCH] Modify virCPUarmCompare to perform compare actions
Daniel Henrique Barboza
danielhb413 at gmail.com
Mon Aug 31 13:13:41 UTC 2020
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 at 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(©->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
>
More information about the libvir-list
mailing list