Re: [PATCH V3] Modify virCPUarmCompare to perform compare actions

On Wed, Sep 16, 2020 at 16:58:03 +0800, Zhenyu Zheng wrote: > Modify virCPUarmCompare in cpu_arm.c to perform compare action. > This patch only adds host to host CPU compare, the rest cases > remains the same. This is useful for source and destination host > compare during migrations to avoid migration between different > CPU models that have different CPU freatures. > > Signed-off-by: Zhenyu Zheng <zheng.zhenyu(a)outlook.com&gt; > --- > src/cpu/cpu_arm.c | 43 +++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 39 insertions(+), 4 deletions(-) > > diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c > index 939a3b8390..b420b14e86 100644 > --- a/src/cpu/cpu_arm.c > +++ b/src/cpu/cpu_arm.c > @@ -463,11 +463,46 @@ virCPUarmBaseline(virCPUDefPtr *cpus, > } > > static virCPUCompareResult > -virCPUarmCompare(virCPUDefPtr host G_GNUC_UNUSED, > - virCPUDefPtr cpu G_GNUC_UNUSED, > - bool failMessages G_GNUC_UNUSED) > +virCPUarmCompare(virCPUDefPtr host, > + virCPUDefPtr cpu, > + bool failIncompatible > +) > { > - return VIR_CPU_COMPARE_IDENTICAL; > + virCPUCompareResult ret = VIR_CPU_COMPARE_IDENTICAL; This looks a bit too complicated as there's no common code to be executed for several ret values. Just drop this variable and return directly wherever you set it. > + > + /* Only support host to host CPU compare for ARM*/ > + if (cpu->type != VIR_CPU_TYPE_HOST) > + return ret; > + > + if (!host || !host->model) { > + if (failIncompatible) { > + virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s", > + _("unknown host CPU")); > + ret = VIR_CPU_COMPARE_ERROR; > + } else { > + VIR_WARN("unknown host CPU"); > + ret = VIR_CPU_COMPARE_INCOMPATIBLE; > + } > + return ret; > + } > + > + /* Compare vendor and model to check if CPUs are identical */ > + if (STRNEQ(host->vendor, cpu->vendor) || > + STRNEQ(host->model, cpu->model)) { Use STRNEQ_NULLABLE instead. > + VIR_DEBUG("Host CPU model does not match required CPU model %s", > + cpu->model); NULLSTR(cpu->model) and I would also add NULLSTR(cpu->vendor) in the message just in case the CPUs differ only in vendor. > + > + if (failIncompatible) { > + ret = VIR_CPU_COMPARE_ERROR; > + virReportError(VIR_ERR_CPU_INCOMPATIBLE, > + _("Host CPU model does not match required CPU model %s"), > + cpu->model); > + } else { > + ret = VIR_CPU_COMPARE_INCOMPATIBLE; > + } > + } > + > + return ret; > } > > static int Jirka