[V3] Modify virCPUarmCompare to perform compare actions

Message ID OSYP286MB0181D8A56ABD633B797B14C199210@OSYP286MB0181.JPNP286.PROD.OUTLOOK.COM
State New
Headers show
Series
  • [V3] Modify virCPUarmCompare to perform compare actions
Related show

Commit Message

Zhenyu Zheng Sept. 16, 2020, 8:58 a.m.
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@outlook.com>

---
 src/cpu/cpu_arm.c | 43 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 4 deletions(-)

-- 
2.20.1

Comments

Zhenyu Zheng Sept. 22, 2020, 1:11 a.m. | #1
Ping for reviews.

Thanks in advance

On Wed, Sep 16, 2020 at 4:59 PM Zhenyu Zheng <zheng.zhenyu@outlook.com>
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@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;

> +

> +    /* 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)) {

> +        VIR_DEBUG("Host CPU model does not match required CPU model %s",

> +                  cpu->model);

> +

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

> --

> 2.20.1

>

>

>
<div dir="ltr">Ping for reviews.<div><br></div><div>Thanks in advance</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Sep 16, 2020 at 4:59 PM Zhenyu Zheng &lt;<a href="mailto:zheng.zhenyu@outlook.com">zheng.zhenyu@outlook.com</a>&gt; 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">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 &lt;<a href="mailto:zheng.zhenyu@outlook.com" target="_blank">zheng.zhenyu@outlook.com</a>&gt;<br>

---<br>
 src/cpu/cpu_arm.c | 43 +++++++++++++++++++++++++++++++++++++++----<br>
 1 file changed, 39 insertions(+), 4 deletions(-)<br>
<br>
diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c<br>
index 939a3b8390..b420b14e86 100644<br>
--- a/src/cpu/cpu_arm.c<br>
+++ b/src/cpu/cpu_arm.c<br>
@@ -463,11 +463,46 @@ 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>
+    virCPUCompareResult ret = VIR_CPU_COMPARE_IDENTICAL;<br>
+<br>
+    /* Only support host to host CPU compare for ARM*/<br>
+    if (cpu-&gt;type != VIR_CPU_TYPE_HOST)<br>
+        return ret;<br>
+<br>
+    if (!host || !host-&gt;model) {<br>
+        if (failIncompatible) {<br>
+            virReportError(VIR_ERR_CPU_INCOMPATIBLE, &quot;%s&quot;,<br>
+                           _(&quot;unknown host CPU&quot;));<br>
+            ret = VIR_CPU_COMPARE_ERROR;<br>
+        } else {<br>
+            VIR_WARN(&quot;unknown host CPU&quot;);<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-&gt;vendor, cpu-&gt;vendor) ||<br>
+        STRNEQ(host-&gt;model, cpu-&gt;model)) {<br>
+        VIR_DEBUG(&quot;Host CPU model does not match required CPU model %s&quot;,<br>
+                  cpu-&gt;model);<br>
+<br>
+        if (failIncompatible) {<br>
+            ret = VIR_CPU_COMPARE_ERROR;<br>
+            virReportError(VIR_ERR_CPU_INCOMPATIBLE,<br>
+                           _(&quot;Host CPU model does not match required CPU model %s&quot;),<br>
+                           cpu-&gt;model);<br>
+        } else {<br>
+            ret = VIR_CPU_COMPARE_INCOMPATIBLE;<br>
+        }<br>
+    }<br>
+<br>
+    return ret;<br>
 }<br>
<br>
 static int<br>
-- <br>
2.20.1<br>
<br>
<br>
</blockquote></div>
Jiri Denemark Sept. 23, 2020, 5:29 p.m. | #2
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@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
Zhenyu Zheng Sept. 24, 2020, 2:15 p.m. | #3
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 <jdenemar@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.zhenyu@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

>

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

Patch

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;
+
+    /* 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)) {
+        VIR_DEBUG("Host CPU model does not match required CPU model %s",
+                  cpu->model);
+
+        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