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
>
>

Reply via email to