| Message ID | OSYP286MB018131ABA7BA761FDD7BAA2799210@OSYP286MB0181.JPNP286.PROD.OUTLOOK.COM |
|---|---|
| State | New |
| Headers | show |
| Series | [V2] Modify virCPUarmCompare to perform compare actions | expand |
On Wed, Sep 16, 2020 at 11:04:58 +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 | 51 +++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 47 insertions(+), 4 deletions(-) > > diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c > index 939a3b8390..e8581ec31f 100644 > --- a/src/cpu/cpu_arm.c > +++ b/src/cpu/cpu_arm.c > @@ -463,11 +463,54 @@ 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; > + g_autofree char *message = NULL; > + 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 (message) { 'message' was initialized to NULL here so this code won't ever be executed. > + message = g_strdup_printf(_("host CPU model does not match required " > + "CPU model %s"), > + cpu->model); > + } > + > + ret = VIR_CPU_COMPARE_INCOMPATIBLE; > + } > + > + if (failIncompatible && ret == VIR_CPU_COMPARE_INCOMPATIBLE) { > + ret = VIR_CPU_COMPARE_ERROR; > + if (message) { > + virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s", message); Neither this code since it will still be NULL. Also please don't store the error message in a random variable, but rather use a specific virReportError call. If you want to omit it in certain cases, just skip the whole virReportError call. > + } else { > + virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL); > + } > + } > + > + return ret; > } > > static int > -- > 2.20.1 > >
oh, yes, thanks for the correction, I've messed this up with another version of code. On Wed, Sep 16, 2020 at 3:20 PM Peter Krempa <pkrempa@redhat.com> wrote: > On Wed, Sep 16, 2020 at 11:04:58 +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 | 51 +++++++++++++++++++++++++++++++++++++++++++---- > > 1 file changed, 47 insertions(+), 4 deletions(-) > > > > diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c > > index 939a3b8390..e8581ec31f 100644 > > --- a/src/cpu/cpu_arm.c > > +++ b/src/cpu/cpu_arm.c > > @@ -463,11 +463,54 @@ 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; > > + g_autofree char *message = NULL; > > + 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 (message) { > > 'message' was initialized to NULL here so this code won't ever be > executed. > > > + message = g_strdup_printf(_("host CPU model does not match > required " > > + "CPU model %s"), > > + cpu->model); > > + } > > + > > + ret = VIR_CPU_COMPARE_INCOMPATIBLE; > > + } > > + > > + if (failIncompatible && ret == VIR_CPU_COMPARE_INCOMPATIBLE) { > > + ret = VIR_CPU_COMPARE_ERROR; > > + if (message) { > > + virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s", message); > > Neither this code since it will still be NULL. > > Also please don't store the error message in a random variable, but > rather use a specific virReportError call. If you want to omit it in > certain cases, just skip the whole virReportError call. > > > + } else { > > + virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL); > > + } > > + } > > + > > + return ret; > > } > > > > static int > > -- > > 2.20.1 > > > > > > <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>
diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index 939a3b8390..e8581ec31f 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -463,11 +463,54 @@ 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; + g_autofree char *message = NULL; + 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 (message) { + message = g_strdup_printf(_("host CPU model does not match required " + "CPU model %s"), + cpu->model); + } + + ret = VIR_CPU_COMPARE_INCOMPATIBLE; + } + + if (failIncompatible && ret == VIR_CPU_COMPARE_INCOMPATIBLE) { + ret = VIR_CPU_COMPARE_ERROR; + if (message) { + virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s", message); + } else { + virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL); + } + } + + return ret; } static int
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 | 51 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 47 insertions(+), 4 deletions(-) -- 2.20.1