Re: [PATCH 6/7] Introduce qemuDomainMachineIsVirt

[Andrea Bolognani <abologna@xxxxxxxxxx>]

On Tue, 2016-05-03 at 13:19 +0200, Andrea Bolognani wrote:
> On Tue, 2016-05-03 at 13:05 +0200, Peter Krempa wrote:
> > > 
> > > +bool
> > > +qemuDomainMachineIsVirt(const virDomainDef *def)
> > > +{
> > > +    return STREQ(def->os.machine, "virt") ||
> > > +           STRPREFIX(def->os.machine, "virt-");
> > 
> > This could be transcribed as:
> > strcmp(def->os.machine, "virt") ||
> > strncmp(def->os.machine, "virt-", strlen("virt-")) so
> > "STRPREFIX(def->os.machine, "virt")" should be equivalent.
> 
> I think we want to make sure we only accept "virt" and
> its versioned variants here - if QEMU introuced a new
> machine type called "virtpc" we wouldn't want it to
> pass the test.
> 
> (Then again, "virt-pc" would pass in any case. I think
>  the current check strikes a good balance, YMMV.)

How about something like the attached, untested patch? :)

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

From 2e10cbe328459de23b5ec216f967419e97b3f93a Mon Sep 17 00:00:00 2001
From: Andrea Bolognani <abologna@xxxxxxxxxx>
Date: Tue, 3 May 2016 14:12:54 +0200
Subject: [PATCH] qemu: Introduce qemuDomainMachineIsVariantOf()

Make machine type checking more robust by ensuring that the base
model can only be followed by version information, and that
similarly-named base models (eg. pc and pc-q35) are not mixed up.
---
 src/qemu/qemu_domain.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 3f1fbd7..b3a7cb2 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4615,10 +4615,29 @@ qemuFindAgentConfig(virDomainDefPtr def)
 }
 
 
+static bool
+qemuDomainMachineIsVariantOf(const virDomainDef *def,
+                             const char *base)
+{
+    /* No version information */
+    if (STREQ(def->os.machine, base))
+        return true;
+
+    /* The machine type start with the base name, followed by a dash which
+     * also happens to be the last in the whole string. This assumes version
+     * information will never contain a dash */
+    if (STRPREFIX(def->os.machine, base) &&
+        def->os.machine + strlen(base) == strrchr(def->os.machine, '-'))
+        return true;
+
+    return false;
+}
+
+
 bool
 qemuDomainMachineIsQ35(const virDomainDef *def)
 {
-    return (STRPREFIX(def->os.machine, "pc-q35") ||
+    return (qemuDomainMachineIsVariantOf(def, "pc-q35") ||
             STREQ(def->os.machine, "q35"));
 }
 
@@ -4626,10 +4645,8 @@ qemuDomainMachineIsQ35(const virDomainDef *def)
 bool
 qemuDomainMachineIsI440FX(const virDomainDef *def)
 {
-    return (STREQ(def->os.machine, "pc") ||
-            STRPREFIX(def->os.machine, "pc-0.") ||
-            STRPREFIX(def->os.machine, "pc-1.") ||
-            STRPREFIX(def->os.machine, "pc-i440") ||
+    return (qemuDomainMachineIsVariantOf(def, "pc-i440fx") ||
+            qemuDomainMachineIsVariantOf(def, "pc") ||
             STRPREFIX(def->os.machine, "rhel"));
 }
 
@@ -4655,7 +4672,7 @@ qemuDomainMachineNeedsFDC(const virDomainDef *def)
 bool
 qemuDomainMachineIsS390CCW(const virDomainDef *def)
 {
-    return STRPREFIX(def->os.machine, "s390-ccw");
+    return qemuDomainMachineIsVariantOf(def, "s390-ccw");
 }
 
 
-- 
2.5.5

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