[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH 1/3] virSysinfoDef: Exempt BIOS variables



Move all the bios_* fields into a separate struct. Not only this
simplifies the code a bit it also helps us to identify whether BIOS
info is present. We don't have to check all the four variables for
being not-NULL, but we can just check the pointer to the struct.

Signed-off-by: Michal Privoznik <mprivozn redhat com>
---
 src/conf/domain_conf.c   | 109 +++++++++++++++++++++++++++++---------------
 src/libvirt_private.syms |   1 +
 src/qemu/qemu_command.c  |  23 +++++-----
 src/util/virsysinfo.c    | 114 +++++++++++++++++++++++++++++++++++------------
 src/util/virsysinfo.h    |  15 +++++--
 5 files changed, 181 insertions(+), 81 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index add857c..e37e453 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10863,46 +10863,28 @@ virDomainShmemDefParseXML(xmlNodePtr node,
     return ret;
 }
 
-static virSysinfoDefPtr
-virSysinfoParseXML(xmlNodePtr node,
-                  xmlXPathContextPtr ctxt,
-                  unsigned char *domUUID,
-                  bool uuid_generated)
+static int
+virSysinfoBIOSParseXML(xmlNodePtr node,
+                       xmlXPathContextPtr ctxt,
+                       virSysinfoBIOSDefPtr *bios)
 {
-    virSysinfoDefPtr def;
-    char *type;
-    char *tmpUUID = NULL;
+    int ret = -1;
+    virSysinfoBIOSDefPtr def;
 
-    if (!xmlStrEqual(node->name, BAD_CAST "sysinfo")) {
+    if (!xmlStrEqual(node->name, BAD_CAST "bios")) {
         virReportError(VIR_ERR_XML_ERROR, "%s",
-                       _("XML does not contain expected 'sysinfo' element"));
-        return NULL;
+                       _("XML does not contain expected 'bios' element"));
+        return ret;
     }
 
     if (VIR_ALLOC(def) < 0)
-        return NULL;
+        goto cleanup;
 
-    type = virXMLPropString(node, "type");
-    if (type == NULL) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("sysinfo must contain a type attribute"));
-        goto error;
-    }
-    if ((def->type = virSysinfoTypeFromString(type)) < 0) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                       _("unknown sysinfo type '%s'"), type);
-        goto error;
-    }
-
-
-    /* Extract BIOS related metadata */
-    def->bios_vendor =
-        virXPathString("string(bios/entry[ name='vendor'])", ctxt);
-    def->bios_version =
-        virXPathString("string(bios/entry[ name='version'])", ctxt);
-    def->bios_date =
-        virXPathString("string(bios/entry[ name='date'])", ctxt);
-    if (def->bios_date != NULL) {
+    def->vendor = virXPathString("string(entry[ name='vendor'])", ctxt);
+    def->version = virXPathString("string(entry[ name='version'])", ctxt);
+    def->date = virXPathString("string(entry[ name='date'])", ctxt);
+    def->release = virXPathString("string(entry[ name='release'])", ctxt);
+    if (def->date != NULL) {
         char *ptr;
         int month, day, year;
 
@@ -10911,7 +10893,7 @@ virSysinfoParseXML(xmlNodePtr node,
          * where yy must be 00->99 and would be assumed to be 19xx
          * a yyyy date should be 1900 and beyond
          */
-        if (virStrToLong_i(def->bios_date, &ptr, 10, &month) < 0 ||
+        if (virStrToLong_i(def->date, &ptr, 10, &month) < 0 ||
             *ptr != '/' ||
             virStrToLong_i(ptr + 1, &ptr, 10, &day) < 0 ||
             *ptr != '/' ||
@@ -10922,11 +10904,66 @@ virSysinfoParseXML(xmlNodePtr node,
             (year < 0 || (year >= 100 && year < 1900))) {
             virReportError(VIR_ERR_XML_DETAIL, "%s",
                            _("Invalid BIOS 'date' format"));
+            goto cleanup;
+        }
+    }
+
+    if (!def->vendor && !def->version &&
+        !def->date && !def->release) {
+        virSysinfoBIOSDefFree(def);
+        def = NULL;
+    }
+
+    *bios = def;
+    def = NULL;
+    ret = 0;
+ cleanup:
+    virSysinfoBIOSDefFree(def);
+    return ret;
+}
+
+static virSysinfoDefPtr
+virSysinfoParseXML(xmlNodePtr node,
+                  xmlXPathContextPtr ctxt,
+                  unsigned char *domUUID,
+                  bool uuid_generated)
+{
+    virSysinfoDefPtr def;
+    xmlNodePtr oldnode, tmpnode;
+    char *type;
+    char *tmpUUID = NULL;
+
+    if (!xmlStrEqual(node->name, BAD_CAST "sysinfo")) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("XML does not contain expected 'sysinfo' element"));
+        return NULL;
+    }
+
+    if (VIR_ALLOC(def) < 0)
+        return NULL;
+
+    type = virXMLPropString(node, "type");
+    if (type == NULL) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("sysinfo must contain a type attribute"));
+        goto error;
+    }
+    if ((def->type = virSysinfoTypeFromString(type)) < 0) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("unknown sysinfo type '%s'"), type);
+        goto error;
+    }
+
+    /* Extract BIOS related metadata */
+    if ((tmpnode = virXPathNode("./bios[1]", ctxt)) != NULL) {
+        oldnode = ctxt->node;
+        ctxt->node = tmpnode;
+        if (virSysinfoBIOSParseXML(tmpnode, ctxt, &def->bios) < 0) {
+            ctxt->node = oldnode;
             goto error;
         }
+        ctxt->node = oldnode;
     }
-    def->bios_release =
-        virXPathString("string(bios/entry[ name='release'])", ctxt);
 
     /* Extract system related metadata */
     def->system_manufacturer =
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 67a7e21..06ef7ae 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2176,6 +2176,7 @@ virVasprintfInternal;
 
 
 # util/virsysinfo.h
+virSysinfoBIOSDefFree;
 virSysinfoDefFree;
 virSysinfoFormat;
 virSysinfoRead;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8ccbe82..7d58cb5 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6660,28 +6660,27 @@ static char *qemuBuildTPMDevStr(const virDomainDef *def,
 }
 
 
-static char *qemuBuildSmbiosBiosStr(virSysinfoDefPtr def)
+static char *qemuBuildSmbiosBiosStr(virSysinfoBIOSDefPtr def)
 {
     virBuffer buf = VIR_BUFFER_INITIALIZER;
 
-    if ((def->bios_vendor == NULL) && (def->bios_version == NULL) &&
-        (def->bios_date == NULL) && (def->bios_release == NULL))
+    if (!def)
         return NULL;
 
     virBufferAddLit(&buf, "type=0");
 
     /* 0:Vendor */
-    if (def->bios_vendor)
-        virBufferAsprintf(&buf, ",vendor=%s", def->bios_vendor);
+    if (def->vendor)
+        virBufferAsprintf(&buf, ",vendor=%s", def->vendor);
     /* 0:BIOS Version */
-    if (def->bios_version)
-        virBufferAsprintf(&buf, ",version=%s", def->bios_version);
+    if (def->version)
+        virBufferAsprintf(&buf, ",version=%s", def->version);
     /* 0:BIOS Release Date */
-    if (def->bios_date)
-        virBufferAsprintf(&buf, ",date=%s", def->bios_date);
+    if (def->date)
+        virBufferAsprintf(&buf, ",date=%s", def->date);
     /* 0:System BIOS Major Release and 0:System BIOS Minor Release */
-    if (def->bios_release)
-        virBufferAsprintf(&buf, ",release=%s", def->bios_release);
+    if (def->release)
+        virBufferAsprintf(&buf, ",release=%s", def->release);
 
     if (virBufferCheckError(&buf) < 0)
         goto error;
@@ -8915,7 +8914,7 @@ qemuBuildCommandLine(virConnectPtr conn,
         if (source != NULL) {
             char *smbioscmd;
 
-            smbioscmd = qemuBuildSmbiosBiosStr(source);
+            smbioscmd = qemuBuildSmbiosBiosStr(source->bios);
             if (smbioscmd != NULL) {
                 virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL);
                 VIR_FREE(smbioscmd);
diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c
index 4edce66..72b7bb7 100644
--- a/src/util/virsysinfo.c
+++ b/src/util/virsysinfo.c
@@ -64,6 +64,18 @@ void virSysinfoSetup(const char *dmidecode, const char *sysinfo,
     sysinfoCpuinfo = cpuinfo;
 }
 
+void virSysinfoBIOSDefFree(virSysinfoBIOSDefPtr def)
+{
+    if (def == NULL)
+        return;
+
+    VIR_FREE(def->vendor);
+    VIR_FREE(def->version);
+    VIR_FREE(def->date);
+    VIR_FREE(def->release);
+    VIR_FREE(def);
+}
+
 /**
  * virSysinfoDefFree:
  * @def: a sysinfo structure
@@ -78,10 +90,7 @@ void virSysinfoDefFree(virSysinfoDefPtr def)
     if (def == NULL)
         return;
 
-    VIR_FREE(def->bios_vendor);
-    VIR_FREE(def->bios_version);
-    VIR_FREE(def->bios_date);
-    VIR_FREE(def->bios_release);
+    virSysinfoBIOSDefFree(def->bios);
 
     VIR_FREE(def->system_manufacturer);
     VIR_FREE(def->system_product);
@@ -524,40 +533,56 @@ virSysinfoRead(void)
 #else /* !WIN32 && x86 */
 
 static int
-virSysinfoParseBIOS(const char *base, virSysinfoDefPtr ret)
+virSysinfoParseBIOS(const char *base, virSysinfoBIOSDefPtr *bios)
 {
+    int ret = -1;
     const char *cur, *eol = NULL;
+    virSysinfoBIOSDefPtr def;
 
     if ((cur = strstr(base, "BIOS Information")) == NULL)
         return 0;
 
+    if (VIR_ALLOC(def) < 0)
+        return ret;
+
     base = cur;
     if ((cur = strstr(base, "Vendor: ")) != NULL) {
         cur += 8;
         eol = strchr(cur, '\n');
-        if (eol && VIR_STRNDUP(ret->bios_vendor, cur, eol - cur) < 0)
-            return -1;
+        if (eol && VIR_STRNDUP(def->vendor, cur, eol - cur) < 0)
+            goto cleanup;
     }
     if ((cur = strstr(base, "Version: ")) != NULL) {
         cur += 9;
         eol = strchr(cur, '\n');
-        if (eol && VIR_STRNDUP(ret->bios_version, cur, eol - cur) < 0)
-            return -1;
+        if (eol && VIR_STRNDUP(def->version, cur, eol - cur) < 0)
+            goto cleanup;
     }
     if ((cur = strstr(base, "Release Date: ")) != NULL) {
         cur += 14;
         eol = strchr(cur, '\n');
-        if (eol && VIR_STRNDUP(ret->bios_date, cur, eol - cur) < 0)
-            return -1;
+        if (eol && VIR_STRNDUP(def->date, cur, eol - cur) < 0)
+            goto cleanup;
     }
     if ((cur = strstr(base, "BIOS Revision: ")) != NULL) {
         cur += 15;
         eol = strchr(cur, '\n');
-        if (eol && VIR_STRNDUP(ret->bios_release, cur, eol - cur) < 0)
-            return -1;
+        if (eol && VIR_STRNDUP(def->release, cur, eol - cur) < 0)
+            goto cleanup;
     }
 
-    return 0;
+    if (!def->vendor && !def->version &&
+        !def->date && !def->release) {
+        virSysinfoBIOSDefFree(def);
+        def = NULL;
+    }
+
+    *bios = def;
+    def = NULL;
+    ret = 0;
+ cleanup:
+    virSysinfoBIOSDefFree(def);
+    return ret;
 }
 
 static int
@@ -848,7 +873,7 @@ virSysinfoRead(void)
 
     ret->type = VIR_SYSINFO_SMBIOS;
 
-    if (virSysinfoParseBIOS(outbuf, ret) < 0)
+    if (virSysinfoParseBIOS(outbuf, &ret->bios) < 0)
         goto error;
 
     if (virSysinfoParseSystem(outbuf, ret) < 0)
@@ -878,22 +903,21 @@ virSysinfoRead(void)
 #endif /* !WIN32 && x86 */
 
 static void
-virSysinfoBIOSFormat(virBufferPtr buf, virSysinfoDefPtr def)
+virSysinfoBIOSFormat(virBufferPtr buf, virSysinfoBIOSDefPtr def)
 {
-    if (!def->bios_vendor && !def->bios_version &&
-        !def->bios_date && !def->bios_release)
+    if (!def)
         return;
 
     virBufferAddLit(buf, "<bios>\n");
     virBufferAdjustIndent(buf, 2);
     virBufferEscapeString(buf, "<entry name='vendor'>%s</entry>\n",
-                          def->bios_vendor);
+                          def->vendor);
     virBufferEscapeString(buf, "<entry name='version'>%s</entry>\n",
-                          def->bios_version);
+                          def->version);
     virBufferEscapeString(buf, "<entry name='date'>%s</entry>\n",
-                          def->bios_date);
+                          def->date);
     virBufferEscapeString(buf, "<entry name='release'>%s</entry>\n",
-                          def->bios_release);
+                          def->release);
     virBufferAdjustIndent(buf, -2);
     virBufferAddLit(buf, "</bios>\n");
 }
@@ -1055,7 +1079,7 @@ virSysinfoFormat(virBufferPtr buf, virSysinfoDefPtr def)
     virBufferAsprintf(buf, "<sysinfo type='%s'>\n", type);
     virBufferAdjustIndent(buf, 2);
 
-    virSysinfoBIOSFormat(buf, def);
+    virSysinfoBIOSFormat(buf, def->bios);
     virSysinfoSystemFormat(buf, def);
     virSysinfoProcessorFormat(buf, def);
     virSysinfoMemoryFormat(buf, def);
@@ -1069,6 +1093,41 @@ virSysinfoFormat(virBufferPtr buf, virSysinfoDefPtr def)
     return 0;
 }
 
+static bool
+virSysinfoBIOSIsEqual(virSysinfoBIOSDefPtr src,
+                      virSysinfoBIOSDefPtr dst)
+{
+    bool identical = false;
+
+    if (!src && !dst)
+        return true;
+
+    if ((src && !dst) || (!src && dst)) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("Target sysinfo does not match source"));
+        goto cleanup;
+    }
+
+#define CHECK_FIELD(name, desc)                                         \
+    do {                                                                \
+        if (STRNEQ_NULLABLE(src->name, dst->name)) {                    \
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,                  \
+                           _("Target sysinfo %s %s does not match source %s"), \
+                           desc, NULLSTR(src->name), NULLSTR(dst->name)); \
+        }                                                               \
+    } while (0)
+
+    CHECK_FIELD(vendor, "BIOS vendor");
+    CHECK_FIELD(version, "BIOS version");
+    CHECK_FIELD(date, "BIOS date");
+    CHECK_FIELD(release, "BIOS release");
+#undef CHECK_FIELD
+
+    identical = true;
+ cleanup:
+    return identical;
+}
+
 bool virSysinfoIsEqual(virSysinfoDefPtr src,
                        virSysinfoDefPtr dst)
 {
@@ -1091,6 +1150,9 @@ bool virSysinfoIsEqual(virSysinfoDefPtr src,
         goto cleanup;
     }
 
+    if (!virSysinfoBIOSIsEqual(src->bios, dst->bios))
+        goto cleanup;
+
 #define CHECK_FIELD(name, desc)                                         \
     do {                                                                \
         if (STRNEQ_NULLABLE(src->name, dst->name)) {                    \
@@ -1099,12 +1161,6 @@ bool virSysinfoIsEqual(virSysinfoDefPtr src,
                            desc, NULLSTR(src->name), NULLSTR(dst->name)); \
         }                                                               \
     } while (0)
-
-    CHECK_FIELD(bios_vendor, "BIOS vendor");
-    CHECK_FIELD(bios_version, "BIOS version");
-    CHECK_FIELD(bios_date, "BIOS date");
-    CHECK_FIELD(bios_release, "BIOS release");
-
     CHECK_FIELD(system_manufacturer, "system vendor");
     CHECK_FIELD(system_product, "system product");
     CHECK_FIELD(system_version, "system version");
diff --git a/src/util/virsysinfo.h b/src/util/virsysinfo.h
index 6236ca6..ec32f6c 100644
--- a/src/util/virsysinfo.h
+++ b/src/util/virsysinfo.h
@@ -65,15 +65,21 @@ struct _virSysinfoMemoryDef {
     char *memory_part_number;
 };
 
+typedef struct _virSysinfoBIOSDef virSysinfoBIOSDef;
+typedef virSysinfoBIOSDef *virSysinfoBIOSDefPtr;
+struct _virSysinfoBIOSDef {
+    char *vendor;
+    char *version;
+    char *date;
+    char *release;
+};
+
 typedef struct _virSysinfoDef virSysinfoDef;
 typedef virSysinfoDef *virSysinfoDefPtr;
 struct _virSysinfoDef {
     int type;
 
-    char *bios_vendor;
-    char *bios_version;
-    char *bios_date;
-    char *bios_release;
+    virSysinfoBIOSDefPtr bios;
 
     char *system_manufacturer;
     char *system_product;
@@ -92,6 +98,7 @@ struct _virSysinfoDef {
 
 virSysinfoDefPtr virSysinfoRead(void);
 
+void virSysinfoBIOSDefFree(virSysinfoBIOSDefPtr def);
 void virSysinfoDefFree(virSysinfoDefPtr def);
 
 int virSysinfoFormat(virBufferPtr buf, virSysinfoDefPtr def)
-- 
2.3.6


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]