Thanks alot, I've addressed the comments and updated the patch to V4, please have a look.
BR On Thu, Sep 24, 2020 at 1:30 AM Jiri Denemark <jdene...@redhat.com> wrote: > 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.zhe...@outlook.com> > > --- > > 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 > >