<div dir="ltr">oh, yes, thanks for the correction, I've messed this up with another version of code.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Sep 16, 2020 at 3:20 PM Peter Krempa <<a href="mailto:pkrempa@redhat.com">pkrempa@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Wed, Sep 16, 2020 at 11:04:58 +0800, Zhenyu Zheng wrote:<br>
> Modify virCPUarmCompare in cpu_arm.c to perform compare action.<br>
> This patch only adds host to host CPU compare, the rest cases<br>
> remains the same. This is useful for source and destination host<br>
> compare during migrations to avoid migration between different<br>
> CPU models that have different CPU freatures.<br>
> <br>
> Signed-off-by: Zhenyu Zheng <<a href="mailto:zheng.zhenyu@outlook.com" target="_blank">zheng.zhenyu@outlook.com</a>><br>
> ---<br>
>  src/cpu/cpu_arm.c | 51 +++++++++++++++++++++++++++++++++++++++++++----<br>
>  1 file changed, 47 insertions(+), 4 deletions(-)<br>
> <br>
> diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c<br>
> index 939a3b8390..e8581ec31f 100644<br>
> --- a/src/cpu/cpu_arm.c<br>
> +++ b/src/cpu/cpu_arm.c<br>
> @@ -463,11 +463,54 @@ virCPUarmBaseline(virCPUDefPtr *cpus,<br>
>  }<br>
>  <br>
>  static virCPUCompareResult<br>
> -virCPUarmCompare(virCPUDefPtr host G_GNUC_UNUSED,<br>
> -                 virCPUDefPtr cpu G_GNUC_UNUSED,<br>
> -                 bool failMessages G_GNUC_UNUSED)<br>
> +virCPUarmCompare(virCPUDefPtr host,<br>
> +                 virCPUDefPtr cpu,<br>
> +                 bool failIncompatible<br>
> +)<br>
>  {<br>
> -    return VIR_CPU_COMPARE_IDENTICAL;<br>
> +    g_autofree char *message = NULL;<br>
> +    virCPUCompareResult ret = VIR_CPU_COMPARE_IDENTICAL;<br>
> +<br>
> +    /* Only support host to host CPU compare for ARM*/<br>
> +    if (cpu->type != VIR_CPU_TYPE_HOST)<br>
> +        return ret;<br>
> +<br>
> +    if (!host || !host->model) {<br>
> +        if (failIncompatible) {<br>
> +            virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s",<br>
> +                           _("unknown host CPU"));<br>
> +            ret = VIR_CPU_COMPARE_ERROR;<br>
> +        } else {<br>
> +            VIR_WARN("unknown host CPU");<br>
> +            ret = VIR_CPU_COMPARE_INCOMPATIBLE;<br>
> +        }<br>
> +        return ret;<br>
> +    }<br>
> +<br>
> +    /* Compare vendor and model to check if CPUs are identical */<br>
> +    if (STRNEQ(host->vendor, cpu->vendor) ||<br>
> +        STRNEQ(host->model, cpu->model)) {<br>
> +        VIR_DEBUG("host CPU model does not match required CPU model %s",<br>
> +                  cpu->model);<br>
> +        if (message) {<br>
<br>
'message' was initialized to NULL here so this code won't ever be<br>
executed.<br>
<br>
> +            message = g_strdup_printf(_("host CPU model does not match required "<br>
> +                                        "CPU model %s"),<br>
> +                                      cpu->model);<br>
> +        }<br>
> +<br>
> +        ret = VIR_CPU_COMPARE_INCOMPATIBLE;<br>
> +    }<br>
> +<br>
> +    if (failIncompatible && ret == VIR_CPU_COMPARE_INCOMPATIBLE) {<br>
> +        ret = VIR_CPU_COMPARE_ERROR;<br>
> +        if (message) {<br>
> +            virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s", message);<br>
<br>
Neither this code since it will still be NULL.<br>
<br>
Also please don't store the error message in a random variable, but<br>
rather use a specific virReportError call. If you want to omit it in<br>
certain cases, just skip the whole virReportError call.<br>
<br>
> +        } else {<br>
> +            virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL);<br>
> +        }<br>
> +    }<br>
> +<br>
> +    return ret;<br>
>  }<br>
>  <br>
>  static int<br>
> -- <br>
> 2.20.1<br>
> <br>
> <br>
<br>
</blockquote></div>