[libvirt] [PATCH] treat host models as case-insensitive strings

Scott Garfinkle posted 1 patch 6 years, 4 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1513708766-34246-1-git-send-email-scottgar@linux.vnet.ibm.com
There is a newer version of this series
src/conf/domain_capabilities.c |  2 +-
src/cpu/cpu_ppc64.c            | 16 ++++++++--------
src/internal.h                 |  1 +
3 files changed, 10 insertions(+), 9 deletions(-)
[libvirt] [PATCH] treat host models as case-insensitive strings
Posted by Scott Garfinkle 6 years, 4 months ago
Qemu now allows case-insensitive specification of CPU models. This fixes the
resulting problems on POWER arch machines. I believe a similar change is needed
in src/cpu/cpu_x86.c but I don't have a way to test this.

Signed-off-by: Scott Garfinkle <scottgar@linux.vnet.ibm.com>
---
 src/conf/domain_capabilities.c |  2 +-
 src/cpu/cpu_ppc64.c            | 16 ++++++++--------
 src/internal.h                 |  1 +
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index e7323a8..f7d9be5 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -271,7 +271,7 @@ virDomainCapsCPUModelsGet(virDomainCapsCPUModelsPtr cpuModels,
         return NULL;
 
     for (i = 0; i < cpuModels->nmodels; i++) {
-        if (STREQ(cpuModels->models[i].name, name))
+        if (STRCASEEQ(cpuModels->models[i].name, name))
             return cpuModels->models + i;
     }
 
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index 76582d4..9f7e2bb 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -67,10 +67,10 @@ static int
 virCPUppc64ConvertLegacy(virCPUDefPtr cpu)
 {
     if (cpu->model &&
-        (STREQ(cpu->model, "POWER7_v2.1") ||
-         STREQ(cpu->model, "POWER7_v2.3") ||
-         STREQ(cpu->model, "POWER7+_v2.1") ||
-         STREQ(cpu->model, "POWER8_v1.0"))) {
+        (STRCASEEQ(cpu->model, "POWER7_v2.1") ||
+         STRCASEEQ(cpu->model, "POWER7_v2.3") ||
+         STRCASEEQ(cpu->model, "POWER7+_v2.1") ||
+         STRCASEEQ(cpu->model, "POWER8_v1.0"))) {
         cpu->model[strlen("POWERx")] = 0;
     }
 
@@ -93,7 +93,7 @@ ppc64CheckCompatibilityMode(const char *host_model,
         return VIR_CPU_COMPARE_IDENTICAL;
 
     /* Valid host CPUs: POWER6, POWER7, POWER8, POWER9 */
-    if (!STRPREFIX(host_model, "POWER") ||
+    if (!STRCASEPREFIX(host_model, "POWER") ||
         !(tmp = (char *) host_model + strlen("POWER")) ||
         virStrToLong_i(tmp, NULL, 10, &host) < 0 ||
         host < 6 || host > 9) {
@@ -104,7 +104,7 @@ ppc64CheckCompatibilityMode(const char *host_model,
     }
 
     /* Valid compatibility modes: power6, power7, power8, power9 */
-    if (!STRPREFIX(compat_mode, "power") ||
+    if (!STRCASEPREFIX(compat_mode, "power") ||
         !(tmp = (char *) compat_mode + strlen("power")) ||
         virStrToLong_i(tmp, NULL, 10, &compat) < 0 ||
         compat < 6 || compat > 9) {
@@ -168,7 +168,7 @@ ppc64VendorFind(const struct ppc64_map *map,
     size_t i;
 
     for (i = 0; i < map->nvendors; i++) {
-        if (STREQ(map->vendors[i]->name, name))
+        if (STRCASEEQ(map->vendors[i]->name, name))
             return map->vendors[i];
     }
 
@@ -216,7 +216,7 @@ ppc64ModelFind(const struct ppc64_map *map,
     size_t i;
 
     for (i = 0; i < map->nmodels; i++) {
-        if (STREQ(map->models[i]->name, name))
+        if (STRCASEEQ(map->models[i]->name, name))
             return map->models[i];
     }
 
diff --git a/src/internal.h b/src/internal.h
index 5895030..1760e3b 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -75,6 +75,7 @@
 # define STRNEQLEN(a, b, n) (strncmp(a, b, n) != 0)
 # define STRCASENEQLEN(a, b, n) (c_strncasecmp(a, b, n) != 0)
 # define STRPREFIX(a, b) (strncmp(a, b, strlen(b)) == 0)
+# define STRCASEPREFIX(a, b) (c_strncasecmp(a, b, strlen(b)) == 0)
 # define STRSKIP(a, b) (STRPREFIX(a, b) ? (a) + strlen(b) : NULL)
 
 # define STREQ_NULLABLE(a, b) \
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] treat host models as case-insensitive strings
Posted by Jiri Denemark 6 years, 4 months ago
On Tue, Dec 19, 2017 at 12:39:26 -0600, Scott Garfinkle wrote:
> Qemu now allows case-insensitive specification of CPU models. This fixes the
> resulting problems on POWER arch machines. I believe a similar change is needed
> in src/cpu/cpu_x86.c but I don't have a way to test this.

I believe this change is not needed or even acceptable at all. What kind
of issue are you trying to solve? CPU models accepted by libvirt can be
listed with

    virsh cpu-models $ARCH

Is any of them not accepted by QEMU?

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] treat host models as case-insensitive strings
Posted by Scott Garfinkle 6 years, 4 months ago
On Tue, 2017-12-19 at 20:05 +0100, Jiri Denemark wrote:
> On Tue, Dec 19, 2017 at 12:39:26 -0600, Scott Garfinkle wrote:
> > Qemu now allows case-insensitive specification of CPU models. This fixes the
> > resulting problems on POWER arch machines. I believe a similar change is needed
> > in src/cpu/cpu_x86.c but I don't have a way to test this.
> 
> I believe this change is not needed or even acceptable at all. What kind
> of issue are you trying to solve?
So, the problem is that as of qemu 2.11, Power arch guests don't start,
since libvirt ppc code is looking for the string POWER to determine
whether the system supports the model at hand. 
 
Comment from our qemu maintainer: "There was a rework in QEMU side to
allow case-insensitive specification of CPU models. As a side-effect of
that some models no longer show up as all caps in -cpu ? /
query-cpu-definitions. This requires libvirt changes to avoid relying on
specific capitalized models."
 
Are you concerned that we SHOULD be looking for case-sensitive models in
libvirt? Is this a problem absent qemu?

-Scott

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] treat host models as case-insensitive strings
Posted by Daniel P. Berrange 6 years, 4 months ago
On Tue, Dec 19, 2017 at 02:24:47PM -0600, Scott Garfinkle wrote:
> On Tue, 2017-12-19 at 20:05 +0100, Jiri Denemark wrote:
> > On Tue, Dec 19, 2017 at 12:39:26 -0600, Scott Garfinkle wrote:
> > > Qemu now allows case-insensitive specification of CPU models. This fixes the
> > > resulting problems on POWER arch machines. I believe a similar change is needed
> > > in src/cpu/cpu_x86.c but I don't have a way to test this.
> > 
> > I believe this change is not needed or even acceptable at all. What kind
> > of issue are you trying to solve?
> So, the problem is that as of qemu 2.11, Power arch guests don't start,
> since libvirt ppc code is looking for the string POWER to determine
> whether the system supports the model at hand. 
>  
> Comment from our qemu maintainer: "There was a rework in QEMU side to
> allow case-insensitive specification of CPU models. As a side-effect of
> that some models no longer show up as all caps in -cpu ? /
> query-cpu-definitions. This requires libvirt changes to avoid relying on
> specific capitalized models."
>  
> Are you concerned that we SHOULD be looking for case-sensitive models in
> libvirt? Is this a problem absent qemu?

You need to dinstinguish what libvirt does internally from what it does
in the public XML/APIs.   Your change affected both, so it is wrong. Any
fix should only apply to libvirt's internal communication with QEMU, and
not affect what's done in the public XML/APIs.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list