[libvirt] [PATCH 0/3] Introduce yet another type to SMBIOS
The specification is here: http://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.0.0.pd... The first two patches rework the code a bit, and the last one adds something useful. Michal Privoznik (3): virSysinfoDef: Exempt BIOS variables virSysinfoDef: Exempt SYSTEM variables virSysinfo: Introduce SMBIOS type 2 support docs/formatdomain.html.in | 37 +- docs/schemas/domaincommon.rng | 24 ++ src/conf/domain_conf.c | 252 ++++++++--- src/libvirt_private.syms | 3 + src/qemu/qemu_command.c | 112 +++-- src/util/virsysinfo.c | 528 +++++++++++++++++++----- src/util/virsysinfo.h | 51 ++- tests/qemuxml2argvdata/qemuxml2argv-smbios.args | 2 + tests/qemuxml2argvdata/qemuxml2argv-smbios.xml | 9 + 9 files changed, 804 insertions(+), 214 deletions(-) -- 2.3.6
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
On 05/12/2015 10:56 AM, Michal Privoznik wrote:
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(-)
ACK John
Move all the system_* 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 | 152 ++++++++++++++++++---------- src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 41 ++++---- src/util/virsysinfo.c | 258 ++++++++++++++++++++++++++++++++--------------- src/util/virsysinfo.h | 22 ++-- 5 files changed, 312 insertions(+), 162 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e37e453..209416d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10922,66 +10922,42 @@ virSysinfoBIOSParseXML(xmlNodePtr node, return ret; } -static virSysinfoDefPtr -virSysinfoParseXML(xmlNodePtr node, - xmlXPathContextPtr ctxt, - unsigned char *domUUID, - bool uuid_generated) +static int +virSysinfoSystemParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, + virSysinfoSystemDefPtr *system, + unsigned char *domUUID, + bool uuid_generated) { - virSysinfoDefPtr def; - xmlNodePtr oldnode, tmpnode; - char *type; + int ret = -1; + virSysinfoSystemDefPtr def; char *tmpUUID = NULL; - if (!xmlStrEqual(node->name, BAD_CAST "sysinfo")) { + if (!xmlStrEqual(node->name, BAD_CAST "system")) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("XML does not contain expected 'sysinfo' element")); - return NULL; + _("XML does not contain expected 'system' 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 */ - 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; - } - - /* Extract system related metadata */ - def->system_manufacturer = - virXPathString("string(system/entry[@name='manufacturer'])", ctxt); - def->system_product = - virXPathString("string(system/entry[@name='product'])", ctxt); - def->system_version = - virXPathString("string(system/entry[@name='version'])", ctxt); - def->system_serial = - virXPathString("string(system/entry[@name='serial'])", ctxt); - tmpUUID = virXPathString("string(system/entry[@name='uuid'])", ctxt); + def->manufacturer = + virXPathString("string(entry[@name='manufacturer'])", ctxt); + def->product = + virXPathString("string(entry[@name='product'])", ctxt); + def->version = + virXPathString("string(entry[@name='version'])", ctxt); + def->serial = + virXPathString("string(entry[@name='serial'])", ctxt); + tmpUUID = virXPathString("string(entry[@name='uuid'])", ctxt); if (tmpUUID) { unsigned char uuidbuf[VIR_UUID_BUFLEN]; char uuidstr[VIR_UUID_STRING_BUFLEN]; if (virUUIDParse(tmpUUID, uuidbuf) < 0) { virReportError(VIR_ERR_XML_DETAIL, "%s", _("malformed <sysinfo> uuid element")); - goto error; + goto cleanup; } if (uuid_generated) { memcpy(domUUID, uuidbuf, VIR_UUID_BUFLEN); @@ -10989,7 +10965,7 @@ virSysinfoParseXML(xmlNodePtr node, virReportError(VIR_ERR_XML_DETAIL, "%s", _("UUID mismatch between <uuid> and " "<sysinfo>")); - goto error; + goto cleanup; } /* Although we've validated the UUID as good, virUUIDParse() is * lax with respect to allowing extraneous "-" and " ", but the @@ -10998,17 +10974,85 @@ virSysinfoParseXML(xmlNodePtr node, * properly so that it's used correctly later. */ virUUIDFormat(uuidbuf, uuidstr); - if (VIR_STRDUP(def->system_uuid, uuidstr) < 0) - goto error; + if (VIR_STRDUP(def->uuid, uuidstr) < 0) + goto cleanup; } - def->system_sku = - virXPathString("string(system/entry[@name='sku'])", ctxt); - def->system_family = - virXPathString("string(system/entry[@name='family'])", ctxt); + def->sku = + virXPathString("string(entry[@name='sku'])", ctxt); + def->family = + virXPathString("string(entry[@name='family'])", ctxt); + if (!def->manufacturer && !def->product && !def->version && + !def->serial && !def->uuid && !def->sku && !def->family) { + virSysinfoSystemDefFree(def); + def = NULL; + } + + *system = def; + def = NULL; + ret = 0; cleanup: - VIR_FREE(type); + virSysinfoSystemDefFree(def); VIR_FREE(tmpUUID); + return ret; +} + +static virSysinfoDefPtr +virSysinfoParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, + unsigned char *domUUID, + bool uuid_generated) +{ + virSysinfoDefPtr def; + xmlNodePtr oldnode, tmpnode; + char *type; + + 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; + } + + /* Extract system related metadata */ + if ((tmpnode = virXPathNode("./system[1]", ctxt)) != NULL) { + oldnode = ctxt->node; + ctxt->node = tmpnode; + if (virSysinfoSystemParseXML(tmpnode, ctxt, &def->system, + domUUID, uuid_generated) < 0) { + ctxt->node = oldnode; + goto error; + } + ctxt->node = oldnode; + } + + cleanup: + VIR_FREE(type); return def; error: diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 06ef7ae..bd619d3 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2181,6 +2181,7 @@ virSysinfoDefFree; virSysinfoFormat; virSysinfoRead; virSysinfoSetup; +virSysinfoSystemDefFree; # util/virsystemd.h diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7d58cb5..489cab3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6692,40 +6692,41 @@ static char *qemuBuildSmbiosBiosStr(virSysinfoBIOSDefPtr def) return NULL; } -static char *qemuBuildSmbiosSystemStr(virSysinfoDefPtr def, bool skip_uuid) +static char *qemuBuildSmbiosSystemStr(virSysinfoSystemDefPtr def, + bool skip_uuid) { virBuffer buf = VIR_BUFFER_INITIALIZER; - if ((def->system_manufacturer == NULL) && (def->system_sku == NULL) && - (def->system_product == NULL) && (def->system_version == NULL) && - (def->system_serial == NULL) && (def->system_family == NULL) && - (def->system_uuid == NULL || skip_uuid)) + if (!def || + (!def->manufacturer && !def->product && !def->version && + !def->serial && (!def->uuid || skip_uuid) && + def->sku && !def->family)) return NULL; virBufferAddLit(&buf, "type=1"); /* 1:Manufacturer */ - if (def->system_manufacturer) + if (def->manufacturer) virBufferAsprintf(&buf, ",manufacturer=%s", - def->system_manufacturer); + def->manufacturer); /* 1:Product Name */ - if (def->system_product) - virBufferAsprintf(&buf, ",product=%s", def->system_product); + if (def->product) + virBufferAsprintf(&buf, ",product=%s", def->product); /* 1:Version */ - if (def->system_version) - virBufferAsprintf(&buf, ",version=%s", def->system_version); + if (def->version) + virBufferAsprintf(&buf, ",version=%s", def->version); /* 1:Serial Number */ - if (def->system_serial) - virBufferAsprintf(&buf, ",serial=%s", def->system_serial); + if (def->serial) + virBufferAsprintf(&buf, ",serial=%s", def->serial); /* 1:UUID */ - if (def->system_uuid && !skip_uuid) - virBufferAsprintf(&buf, ",uuid=%s", def->system_uuid); + if (def->uuid && !skip_uuid) + virBufferAsprintf(&buf, ",uuid=%s", def->uuid); /* 1:SKU Number */ - if (def->system_sku) - virBufferAsprintf(&buf, ",sku=%s", def->system_sku); + if (def->sku) + virBufferAsprintf(&buf, ",sku=%s", def->sku); /* 1:Family */ - if (def->system_family) - virBufferAsprintf(&buf, ",family=%s", def->system_family); + if (def->family) + virBufferAsprintf(&buf, ",family=%s", def->family); if (virBufferCheckError(&buf) < 0) goto error; @@ -8919,7 +8920,7 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL); VIR_FREE(smbioscmd); } - smbioscmd = qemuBuildSmbiosSystemStr(source, skip_uuid); + smbioscmd = qemuBuildSmbiosSystemStr(source->system, skip_uuid); if (smbioscmd != NULL) { virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL); VIR_FREE(smbioscmd); diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 72b7bb7..4c939ec 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -76,6 +76,22 @@ void virSysinfoBIOSDefFree(virSysinfoBIOSDefPtr def) VIR_FREE(def); } +void virSysinfoSystemDefFree(virSysinfoSystemDefPtr def) +{ + if (def == NULL) + return; + + VIR_FREE(def->manufacturer); + VIR_FREE(def->product); + VIR_FREE(def->version); + VIR_FREE(def->serial); + VIR_FREE(def->uuid); + VIR_FREE(def->sku); + VIR_FREE(def->family); + VIR_FREE(def); +} + + /** * virSysinfoDefFree: * @def: a sysinfo structure @@ -91,14 +107,7 @@ void virSysinfoDefFree(virSysinfoDefPtr def) return; virSysinfoBIOSDefFree(def->bios); - - VIR_FREE(def->system_manufacturer); - VIR_FREE(def->system_product); - VIR_FREE(def->system_version); - VIR_FREE(def->system_serial); - VIR_FREE(def->system_uuid); - VIR_FREE(def->system_sku); - VIR_FREE(def->system_family); + virSysinfoSystemDefFree(def->system); for (i = 0; i < def->nprocessor; i++) { VIR_FREE(def->processor[i].processor_socket_destination); @@ -141,39 +150,55 @@ void virSysinfoDefFree(virSysinfoDefPtr def) #if defined(__powerpc__) static int -virSysinfoParseSystem(const char *base, virSysinfoDefPtr ret) +virSysinfoParseSystem(const char *base, virSysinfoSystemDefPtr *system) { + int ret = -1; char *eol = NULL; const char *cur; + virSysinfoSystemDefPtr def; if ((cur = strstr(base, "platform")) == NULL) return 0; + if (VIR_ALLOC(def) < 0) + return ret; + base = cur; /* Account for format 'platform : XXXX'*/ cur = strchr(cur, ':') + 1; eol = strchr(cur, '\n'); virSkipSpaces(&cur); - if (eol && VIR_STRNDUP(ret->system_family, cur, eol - cur) < 0) - return -1; + if (eol && VIR_STRNDUP(def->family, cur, eol - cur) < 0) + goto cleanup; if ((cur = strstr(base, "model")) != NULL) { cur = strchr(cur, ':') + 1; eol = strchr(cur, '\n'); virSkipSpaces(&cur); - if (eol && VIR_STRNDUP(ret->system_serial, cur, eol - cur) < 0) - return -1; + if (eol && VIR_STRNDUP(def->serial, cur, eol - cur) < 0) + goto cleanup; } if ((cur = strstr(base, "machine")) != NULL) { cur = strchr(cur, ':') + 1; eol = strchr(cur, '\n'); virSkipSpaces(&cur); - if (eol && VIR_STRNDUP(ret->system_version, cur, eol - cur) < 0) - return -1; + if (eol && VIR_STRNDUP(def->version, cur, eol - cur) < 0) + goto cleanup; } - return 0; + if (!def->manufacturer && !def->product && !def->version && + !def->serial && !def->uuid && !def->sku && !def->family) { + virSysinfoSystemDefFree(def); + def = NULL; + } + + *system = def; + def = NULL; + ret = 0; + cleanup: + virSysinfoSystemDefFree(def); + return ret; } static int @@ -243,7 +268,7 @@ virSysinfoRead(void) if (virSysinfoParseProcessor(outbuf, ret) < 0) goto no_memory; - if (virSysinfoParseSystem(outbuf, ret) < 0) + if (virSysinfoParseSystem(outbuf, &ret->system) < 0) goto no_memory; return ret; @@ -255,27 +280,32 @@ virSysinfoRead(void) #elif defined(__arm__) || defined(__aarch64__) static int -virSysinfoParseSystem(const char *base, virSysinfoDefPtr ret) +virSysinfoParseSystem(const char *base, virSysinfoSystemDefPtr *system) { + int ret = -1; char *eol = NULL; const char *cur; + virSysinfoSystemDefPtr def; if ((cur = strstr(base, "platform")) == NULL) return 0; + if (VIR_ALLOC(def) < 0) + return ret; + base = cur; /* Account for format 'platform : XXXX'*/ cur = strchr(cur, ':') + 1; eol = strchr(cur, '\n'); virSkipSpaces(&cur); - if (eol && VIR_STRNDUP(ret->system_family, cur, eol - cur) < 0) + if (eol && VIR_STRNDUP(def->family, cur, eol - cur) < 0) return -1; if ((cur = strstr(base, "model")) != NULL) { cur = strchr(cur, ':') + 1; eol = strchr(cur, '\n'); virSkipSpaces(&cur); - if (eol && VIR_STRNDUP(ret->system_serial, cur, eol - cur) < 0) + if (eol && VIR_STRNDUP(def->serial, cur, eol - cur) < 0) return -1; } @@ -283,11 +313,22 @@ virSysinfoParseSystem(const char *base, virSysinfoDefPtr ret) cur = strchr(cur, ':') + 1; eol = strchr(cur, '\n'); virSkipSpaces(&cur); - if (eol && VIR_STRNDUP(ret->system_version, cur, eol - cur) < 0) + if (eol && VIR_STRNDUP(def->version, cur, eol - cur) < 0) return -1; } - return 0; + if (!def->manufacturer && !def->product && !def->version && + !def->serial && !def->uuid && !def->sku && !def->family) { + virSysinfoSystemDefFree(def); + def = NULL; + } + + *system = def; + def = NULL; + ret = 0; + cleanup: + virSysinfoSystemDefFree(def); + return ret; } static int @@ -361,7 +402,7 @@ virSysinfoRead(void) if (virSysinfoParseProcessor(outbuf, ret) < 0) goto no_memory; - if (virSysinfoParseSystem(outbuf, ret) < 0) + if (virSysinfoParseSystem(outbuf, &ret->system) < 0) goto no_memory; return ret; @@ -413,17 +454,38 @@ virSysinfoParseLine(const char *base, const char *name, char **value) } static int -virSysinfoParseSystem(const char *base, virSysinfoDefPtr ret) +virSysinfoParseSystem(const char *base, virSysinfoSystemDefPtr *system) { - if (virSysinfoParseLine(base, "Manufacturer", - &ret->system_manufacturer) && - virSysinfoParseLine(base, "Type", - &ret->system_family) && - virSysinfoParseLine(base, "Sequence Code", - &ret->system_serial)) - return 0; - else - return -1; + int ret = -1; + virSysinfoSystemDefPtr def; + + if (VIR_ALLOC(def) < 0) + return ret; + + if (!virSysinfoParseLine(base, "Manufacturer", + &def->manufacturer)) + goto cleanup; + + if (!virSysinfoParseLine(base, "Type", + &def->family)) + goto cleanup; + + if (!virSysinfoParseLine(base, "Sequence Code", + &def->serial)) + goto cleanup; + + if (!def->manufacturer && !def->product && !def->version && + !def->serial && !def->uuid && !def->sku && !def->family) { + virSysinfoSystemDefFree(def); + def = NULL; + } + + *system = def; + def = NULL; + ret = 0; + cleanup: + virSysinfoSystemDefFree(def); + return ret; } static int @@ -500,7 +562,7 @@ virSysinfoRead(void) return NULL; } - if (virSysinfoParseSystem(outbuf, ret) < 0) + if (virSysinfoParseSystem(outbuf, &ret->system) < 0) goto no_memory; return ret; @@ -586,58 +648,74 @@ virSysinfoParseBIOS(const char *base, virSysinfoBIOSDefPtr *bios) } static int -virSysinfoParseSystem(const char *base, virSysinfoDefPtr ret) +virSysinfoParseSystem(const char *base, virSysinfoSystemDefPtr *system) { + int ret = -1; const char *cur, *eol = NULL; + virSysinfoSystemDefPtr def; if ((cur = strstr(base, "System Information")) == NULL) return 0; + if (VIR_ALLOC(def) < 0) + return ret; + base = cur; if ((cur = strstr(base, "Manufacturer: ")) != NULL) { cur += 14; eol = strchr(cur, '\n'); - if (eol && VIR_STRNDUP(ret->system_manufacturer, cur, eol - cur) < 0) - return -1; + if (eol && VIR_STRNDUP(def->manufacturer, cur, eol - cur) < 0) + goto cleanup; } if ((cur = strstr(base, "Product Name: ")) != NULL) { cur += 14; eol = strchr(cur, '\n'); - if (eol && VIR_STRNDUP(ret->system_product, cur, eol - cur) < 0) - return -1; + if (eol && VIR_STRNDUP(def->product, cur, eol - cur) < 0) + goto cleanup; } if ((cur = strstr(base, "Version: ")) != NULL) { cur += 9; eol = strchr(cur, '\n'); - if (eol && VIR_STRNDUP(ret->system_version, cur, eol - cur) < 0) - return -1; + if (eol && VIR_STRNDUP(def->version, cur, eol - cur) < 0) + goto cleanup; } if ((cur = strstr(base, "Serial Number: ")) != NULL) { cur += 15; eol = strchr(cur, '\n'); - if (eol && VIR_STRNDUP(ret->system_serial, cur, eol - cur) < 0) - return -1; + if (eol && VIR_STRNDUP(def->serial, cur, eol - cur) < 0) + goto cleanup; } if ((cur = strstr(base, "UUID: ")) != NULL) { cur += 6; eol = strchr(cur, '\n'); - if (eol && VIR_STRNDUP(ret->system_uuid, cur, eol - cur) < 0) - return -1; + if (eol && VIR_STRNDUP(def->uuid, cur, eol - cur) < 0) + goto cleanup; } if ((cur = strstr(base, "SKU Number: ")) != NULL) { cur += 12; eol = strchr(cur, '\n'); - if (eol && VIR_STRNDUP(ret->system_sku, cur, eol - cur) < 0) - return -1; + if (eol && VIR_STRNDUP(def->sku, cur, eol - cur) < 0) + goto cleanup; } if ((cur = strstr(base, "Family: ")) != NULL) { cur += 8; eol = strchr(cur, '\n'); - if (eol && VIR_STRNDUP(ret->system_family, cur, eol - cur) < 0) - return -1; + if (eol && VIR_STRNDUP(def->family, cur, eol - cur) < 0) + goto cleanup; } - return 0; + if (!def->manufacturer && !def->product && !def->version && + !def->serial && !def->uuid && !def->sku && !def->family) { + virSysinfoSystemDefFree(def); + def = NULL; + } + + *system = def; + def = NULL; + ret = 0; + cleanup: + virSysinfoSystemDefFree(def); + return ret; } static int @@ -876,7 +954,7 @@ virSysinfoRead(void) if (virSysinfoParseBIOS(outbuf, &ret->bios) < 0) goto error; - if (virSysinfoParseSystem(outbuf, ret) < 0) + if (virSysinfoParseSystem(outbuf, &ret->system) < 0) goto error; ret->nprocessor = 0; @@ -923,29 +1001,27 @@ virSysinfoBIOSFormat(virBufferPtr buf, virSysinfoBIOSDefPtr def) } static void -virSysinfoSystemFormat(virBufferPtr buf, virSysinfoDefPtr def) +virSysinfoSystemFormat(virBufferPtr buf, virSysinfoSystemDefPtr def) { - if (!def->system_manufacturer && !def->system_product && - !def->system_version && !def->system_serial && - !def->system_uuid && !def->system_sku && !def->system_family) + if (!def) return; virBufferAddLit(buf, "<system>\n"); virBufferAdjustIndent(buf, 2); virBufferEscapeString(buf, "<entry name='manufacturer'>%s</entry>\n", - def->system_manufacturer); + def->manufacturer); virBufferEscapeString(buf, "<entry name='product'>%s</entry>\n", - def->system_product); + def->product); virBufferEscapeString(buf, "<entry name='version'>%s</entry>\n", - def->system_version); + def->version); virBufferEscapeString(buf, "<entry name='serial'>%s</entry>\n", - def->system_serial); + def->serial); virBufferEscapeString(buf, "<entry name='uuid'>%s</entry>\n", - def->system_uuid); + def->uuid); virBufferEscapeString(buf, "<entry name='sku'>%s</entry>\n", - def->system_sku); + def->sku); virBufferEscapeString(buf, "<entry name='family'>%s</entry>\n", - def->system_family); + def->family); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</system>\n"); } @@ -1080,7 +1156,7 @@ virSysinfoFormat(virBufferPtr buf, virSysinfoDefPtr def) virBufferAdjustIndent(buf, 2); virSysinfoBIOSFormat(buf, def->bios); - virSysinfoSystemFormat(buf, def); + virSysinfoSystemFormat(buf, def->system); virSysinfoProcessorFormat(buf, def); virSysinfoMemoryFormat(buf, def); @@ -1128,6 +1204,43 @@ virSysinfoBIOSIsEqual(virSysinfoBIOSDefPtr src, return identical; } +static bool +virSysinfoSystemIsEqual(virSysinfoSystemDefPtr src, + virSysinfoSystemDefPtr 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(manufacturer, "system vendor"); + CHECK_FIELD(product, "system product"); + CHECK_FIELD(version, "system version"); + CHECK_FIELD(serial, "system serial"); + CHECK_FIELD(uuid, "system uuid"); + CHECK_FIELD(sku, "system sku"); + CHECK_FIELD(family, "system family"); +#undef CHECK_FIELD + + identical = true; + cleanup: + return identical; +} + bool virSysinfoIsEqual(virSysinfoDefPtr src, virSysinfoDefPtr dst) { @@ -1153,23 +1266,8 @@ bool virSysinfoIsEqual(virSysinfoDefPtr src, if (!virSysinfoBIOSIsEqual(src->bios, dst->bios)) 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(system_manufacturer, "system vendor"); - CHECK_FIELD(system_product, "system product"); - CHECK_FIELD(system_version, "system version"); - CHECK_FIELD(system_serial, "system serial"); - CHECK_FIELD(system_uuid, "system uuid"); - CHECK_FIELD(system_sku, "system sku"); - CHECK_FIELD(system_family, "system family"); - -#undef CHECK_FIELD + if (!virSysinfoSystemIsEqual(src->system, dst->system)) + goto cleanup; identical = true; diff --git a/src/util/virsysinfo.h b/src/util/virsysinfo.h index ec32f6c..c8cc1e8 100644 --- a/src/util/virsysinfo.h +++ b/src/util/virsysinfo.h @@ -74,20 +74,25 @@ struct _virSysinfoBIOSDef { char *release; }; +typedef struct _virSysinfoSystemDef virSysinfoSystemDef; +typedef virSysinfoSystemDef *virSysinfoSystemDefPtr; +struct _virSysinfoSystemDef { + char *manufacturer; + char *product; + char *version; + char *serial; + char *uuid; + char *sku; + char *family; +}; + typedef struct _virSysinfoDef virSysinfoDef; typedef virSysinfoDef *virSysinfoDefPtr; struct _virSysinfoDef { int type; virSysinfoBIOSDefPtr bios; - - char *system_manufacturer; - char *system_product; - char *system_version; - char *system_serial; - char *system_uuid; - char *system_sku; - char *system_family; + virSysinfoSystemDefPtr system; size_t nprocessor; virSysinfoProcessorDefPtr processor; @@ -99,6 +104,7 @@ struct _virSysinfoDef { virSysinfoDefPtr virSysinfoRead(void); void virSysinfoBIOSDefFree(virSysinfoBIOSDefPtr def); +void virSysinfoSystemDefFree(virSysinfoSystemDefPtr def); void virSysinfoDefFree(virSysinfoDefPtr def); int virSysinfoFormat(virBufferPtr buf, virSysinfoDefPtr def) -- 2.3.6
On 05/12/2015 10:56 AM, Michal Privoznik wrote:
Move all the system_* 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 | 152 ++++++++++++++++++---------- src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 41 ++++---- src/util/virsysinfo.c | 258 ++++++++++++++++++++++++++++++++--------------- src/util/virsysinfo.h | 22 ++-- 5 files changed, 312 insertions(+), 162 deletions(-)
Patch 3 adds an extra line in virSysinfoSystemIsEqual after the "while(0)" that should have been done here. ACK with that nit adjusted John
https://bugzilla.redhat.com/show_bug.cgi?id=1220527 This type of information defines attributes of a system baseboard. With one caveat: in qemu they call it family, while in the specification it's referred to as type. I'm sticking with the latter. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/formatdomain.html.in | 37 +++++- docs/schemas/domaincommon.rng | 24 ++++ src/conf/domain_conf.c | 57 +++++++++ src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 48 +++++++ src/util/virsysinfo.c | 160 +++++++++++++++++++++++- src/util/virsysinfo.h | 14 +++ tests/qemuxml2argvdata/qemuxml2argv-smbios.args | 2 + tests/qemuxml2argvdata/qemuxml2argv-smbios.xml | 9 ++ 9 files changed, 346 insertions(+), 6 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e0b6ba7..dc92aa3 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -375,6 +375,13 @@ <entry name='product'>Virt-Manager</entry> <entry name='version'>0.9.4</entry> </system> + <baseBoard> + <entry name='manufacturer'>LENOVO</entry> + <entry name='product'>20BE0061MC</entry> + <entry name='version'>0B98401 Pro</entry> + <entry name='serial'>W1KS427111E</entry> + <entry name='type'>Motherboard</entry> + </baseBoard> </sysinfo> ...</pre> @@ -435,11 +442,31 @@ <dt><code>family</code></dt> <dd>Identify the family a particular computer belongs to.</dd> </dl> - NB: Incorrectly supplied entries in either the <code>bios</code> - or <code>system</code> blocks will be ignored without error. - Other than <code>uuid</code> validation and <code>date</code> - format checking, all values are passed as strings to the - hypervisor driver. + </dd> + <dt><code>baseBoard</code></dt> + <dd> + This is block 2 of SMBIOS, with entry names drawn from: + <dl> + <dt><code>manufacturer</code></dt> + <dd>Manufacturer of BIOS</dd> + <dt><code>product</code></dt> + <dd>Product Name</dd> + <dt><code>version</code></dt> + <dd>Version of the product</dd> + <dt><code>serial</code></dt> + <dd>Serial number</dd> + <dt><code>asset</code></dt> + <dd>Asset tag</dd> + <dt><code>location</code></dt> + <dd>Location in chassis</dd> + <dt><code>type</code></dt> + <dd>Board type</dd> + </dl> + NB: Incorrectly supplied entries in either the <code>bios</code> or + <code>system</code> or <code>baseBoard</code> blocks will be + ignored without error. Other than <code>uuid</code> validation and + <code>date</code> format checking, all values are passed as strings + to the hypervisor driver. </dd> </dl> </dd> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index c151e92..2bc84b5 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4240,6 +4240,18 @@ </oneOrMore> </element> </optional> + <optional> + <element name="baseBoard"> + <oneOrMore> + <element name="entry"> + <attribute name="name"> + <ref name="sysinfo-baseBoard-name"/> + </attribute> + <ref name="sysinfo-value"/> + </element> + </oneOrMore> + </element> + </optional> </interleave> </element> </define> @@ -4265,6 +4277,18 @@ </choice> </define> + <define name="sysinfo-baseBoard-name"> + <choice> + <value>manufacturer</value> + <value>product</value> + <value>version</value> + <value>serial</value> + <value>asset</value> + <value>location</value> + <value>type</value> + </choice> + </define> + <define name="sysinfo-value"> <data type="string"> <param name='pattern'>[a-zA-Z0-9/\-_\. \(\)]+</param> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 209416d..0f41375 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10997,6 +10997,52 @@ virSysinfoSystemParseXML(xmlNodePtr node, return ret; } +static int +virSysinfoBaseBoardParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, + virSysinfoBaseBoardDefPtr *baseBoard) +{ + int ret = -1; + virSysinfoBaseBoardDefPtr def; + + if (!xmlStrEqual(node->name, BAD_CAST "baseBoard")) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("XML does not contain expected 'baseBoard' element")); + return ret; + } + + if (VIR_ALLOC(def) < 0) + goto cleanup; + + def->manufacturer = + virXPathString("string(entry[@name='manufacturer'])", ctxt); + def->product = + virXPathString("string(entry[@name='product'])", ctxt); + def->version = + virXPathString("string(entry[@name='version'])", ctxt); + def->serial = + virXPathString("string(entry[@name='serial'])", ctxt); + def->asset = + virXPathString("string(entry[@name='asset'])", ctxt); + def->location = + virXPathString("string(entry[@name='location'])", ctxt); + def->type = + virXPathString("string(entry[@name='type'])", ctxt); + + if (!def->manufacturer && !def->product && !def->version && + !def->serial && !def->asset && !def->location && !def->type) { + virSysinfoBaseBoardDefFree(def); + def = NULL; + } + + *baseBoard = def; + def = NULL; + ret = 0; + cleanup: + virSysinfoBaseBoardDefFree(def); + return ret; +} + static virSysinfoDefPtr virSysinfoParseXML(xmlNodePtr node, xmlXPathContextPtr ctxt, @@ -11051,6 +11097,17 @@ virSysinfoParseXML(xmlNodePtr node, ctxt->node = oldnode; } + /* Extract system base board metadata */ + if ((tmpnode = virXPathNode("./baseBoard[1]", ctxt)) != NULL) { + oldnode = ctxt->node; + ctxt->node = tmpnode; + if (virSysinfoBaseBoardParseXML(tmpnode, ctxt, &def->baseBoard) < 0) { + ctxt->node = oldnode; + goto error; + } + ctxt->node = oldnode; + } + cleanup: VIR_FREE(type); return def; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index bd619d3..3178fe9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2176,6 +2176,7 @@ virVasprintfInternal; # util/virsysinfo.h +virSysinfoBaseBoardDefFree; virSysinfoBIOSDefFree; virSysinfoDefFree; virSysinfoFormat; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 489cab3..c13089e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6738,6 +6738,48 @@ static char *qemuBuildSmbiosSystemStr(virSysinfoSystemDefPtr def, return NULL; } +static char *qemuBuildSmbiosBaseBoardStr(virSysinfoBaseBoardDefPtr def) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + + if (!def) + return NULL; + + virBufferAddLit(&buf, "type=2"); + + /* 1:Manufacturer */ + if (def->manufacturer) + virBufferAsprintf(&buf, ",manufacturer=%s", + def->manufacturer); + /* 1:Product Name */ + if (def->product) + virBufferAsprintf(&buf, ",product=%s", def->product); + /* 1:Version */ + if (def->version) + virBufferAsprintf(&buf, ",version=%s", def->version); + /* 1:Serial Number */ + if (def->serial) + virBufferAsprintf(&buf, ",serial=%s", def->serial); + /* 1:Asset Tag */ + if (def->asset) + virBufferAsprintf(&buf, ",asset=%s", def->asset); + /* 1:Location */ + if (def->location) + virBufferAsprintf(&buf, ",location=%s", def->location); + /* 1:Type */ + if (def->type) + virBufferAsprintf(&buf, ",family=%s", def->type); + + if (virBufferCheckError(&buf) < 0) + goto error; + + return virBufferContentAndReset(&buf); + + error: + virBufferFreeAndReset(&buf); + return NULL; +} + static char * qemuBuildClockArgStr(virDomainClockDefPtr def) { @@ -8925,6 +8967,12 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL); VIR_FREE(smbioscmd); } + + smbioscmd = qemuBuildSmbiosBaseBoardStr(source->baseBoard); + if (smbioscmd != NULL) { + virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL); + VIR_FREE(smbioscmd); + } } } diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 4c939ec..bb21e1b 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -91,6 +91,20 @@ void virSysinfoSystemDefFree(virSysinfoSystemDefPtr def) VIR_FREE(def); } +void virSysinfoBaseBoardDefFree(virSysinfoBaseBoardDefPtr def) +{ + if (def == NULL) + return; + + VIR_FREE(def->manufacturer); + VIR_FREE(def->product); + VIR_FREE(def->version); + VIR_FREE(def->serial); + VIR_FREE(def->asset); + VIR_FREE(def->location); + VIR_FREE(def->type); + VIR_FREE(def); +} /** * virSysinfoDefFree: @@ -108,6 +122,7 @@ void virSysinfoDefFree(virSysinfoDefPtr def) virSysinfoBIOSDefFree(def->bios); virSysinfoSystemDefFree(def->system); + virSysinfoBaseBoardDefFree(def->baseBoard); for (i = 0; i < def->nprocessor; i++) { VIR_FREE(def->processor[i].processor_socket_destination); @@ -719,6 +734,77 @@ virSysinfoParseSystem(const char *base, virSysinfoSystemDefPtr *system) } static int +virSysinfoParseBaseBoard(const char *base, virSysinfoBaseBoardDefPtr *baseBoard) +{ + int ret = -1; + const char *cur, *eol = NULL; + virSysinfoBaseBoardDefPtr def; + + if ((cur = strstr(base, "Base Board Information")) == NULL) + return 0; + + if (VIR_ALLOC(def) < 0) + return ret; + + base = cur; + if ((cur = strstr(base, "Manufacturer: ")) != NULL) { + cur += 14; + eol = strchr(cur, '\n'); + if (eol && VIR_STRNDUP(def->manufacturer, cur, eol - cur) < 0) + goto cleanup; + } + if ((cur = strstr(base, "Product Name: ")) != NULL) { + cur += 14; + eol = strchr(cur, '\n'); + if (eol && VIR_STRNDUP(def->product, cur, eol - cur) < 0) + goto cleanup; + } + if ((cur = strstr(base, "Version: ")) != NULL) { + cur += 9; + eol = strchr(cur, '\n'); + if (eol && VIR_STRNDUP(def->version, cur, eol - cur) < 0) + goto cleanup; + } + if ((cur = strstr(base, "Serial Number: ")) != NULL) { + cur += 15; + eol = strchr(cur, '\n'); + if (eol && VIR_STRNDUP(def->serial, cur, eol - cur) < 0) + goto cleanup; + } + if ((cur = strstr(base, "Asset Tag: ")) != NULL) { + cur += 11; + eol = strchr(cur, '\n'); + if (eol && VIR_STRNDUP(def->asset, cur, eol - cur) < 0) + goto cleanup; + } + if ((cur = strstr(base, "Location In Chassis: ")) != NULL) { + cur += 21; + eol = strchr(cur, '\n'); + if (eol && VIR_STRNDUP(def->location, cur, eol - cur) < 0) + goto cleanup; + } + if ((cur = strstr(base, "Type: ")) != NULL) { + cur += 6; + eol = strchr(cur, '\n'); + if (eol && VIR_STRNDUP(def->type, cur, eol - cur) < 0) + goto cleanup; + } + + if (!def->manufacturer && !def->product && !def->version && + !def->serial && !def->asset && !def->location && !def->type) { + virSysinfoBaseBoardDefFree(def); + def = NULL; + } + + *baseBoard = def; + def = NULL; + ret = 0; + cleanup: + virSysinfoBaseBoardDefFree(def); + return ret; +} + +static int virSysinfoParseProcessor(const char *base, virSysinfoDefPtr ret) { const char *cur, *tmp_base; @@ -940,7 +1026,7 @@ virSysinfoRead(void) return NULL; } - cmd = virCommandNewArgList(path, "-q", "-t", "0,1,4,17", NULL); + cmd = virCommandNewArgList(path, "-q", "-t", "0,1,2,4,17", NULL); VIR_FREE(path); virCommandSetOutputBuffer(cmd, &outbuf); if (virCommandRun(cmd, NULL) < 0) @@ -957,6 +1043,9 @@ virSysinfoRead(void) if (virSysinfoParseSystem(outbuf, &ret->system) < 0) goto error; + if (virSysinfoParseBaseBoard(outbuf, &ret->baseBoard) < 0) + goto error; + ret->nprocessor = 0; ret->processor = NULL; if (virSysinfoParseProcessor(outbuf, ret) < 0) @@ -1027,6 +1116,32 @@ virSysinfoSystemFormat(virBufferPtr buf, virSysinfoSystemDefPtr def) } static void +virSysinfoBaseBoardFormat(virBufferPtr buf, virSysinfoBaseBoardDefPtr def) +{ + if (!def) + return; + + virBufferAddLit(buf, "<baseBoard>\n"); + virBufferAdjustIndent(buf, 2); + virBufferEscapeString(buf, "<entry name='manufacturer'>%s</entry>\n", + def->manufacturer); + virBufferEscapeString(buf, "<entry name='product'>%s</entry>\n", + def->product); + virBufferEscapeString(buf, "<entry name='version'>%s</entry>\n", + def->version); + virBufferEscapeString(buf, "<entry name='serial'>%s</entry>\n", + def->serial); + virBufferEscapeString(buf, "<entry name='asset'>%s</entry>\n", + def->asset); + virBufferEscapeString(buf, "<entry name='location'>%s</entry>\n", + def->location); + virBufferEscapeString(buf, "<entry name='type'>%s</entry>\n", + def->type); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</baseBoard>\n"); +} + +static void virSysinfoProcessorFormat(virBufferPtr buf, virSysinfoDefPtr def) { size_t i; @@ -1157,6 +1272,7 @@ virSysinfoFormat(virBufferPtr buf, virSysinfoDefPtr def) virSysinfoBIOSFormat(buf, def->bios); virSysinfoSystemFormat(buf, def->system); + virSysinfoBaseBoardFormat(buf, def->baseBoard); virSysinfoProcessorFormat(buf, def); virSysinfoMemoryFormat(buf, def); @@ -1227,6 +1343,7 @@ virSysinfoSystemIsEqual(virSysinfoSystemDefPtr src, desc, NULLSTR(src->name), NULLSTR(dst->name)); \ } \ } while (0) + CHECK_FIELD(manufacturer, "system vendor"); CHECK_FIELD(product, "system product"); CHECK_FIELD(version, "system version"); @@ -1241,6 +1358,44 @@ virSysinfoSystemIsEqual(virSysinfoSystemDefPtr src, return identical; } +static bool +virSysinfoBaseBoardIsEqual(virSysinfoBaseBoardDefPtr src, + virSysinfoBaseBoardDefPtr dst) +{ + bool identical = false; + + if (!src && !dst) + return true; + + if ((src && !dst) || (!src && dst)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Target base board 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(manufacturer, "base board vendor"); + CHECK_FIELD(product, "base board product"); + CHECK_FIELD(version, "base board version"); + CHECK_FIELD(serial, "base board serial"); + CHECK_FIELD(asset, "base board asset"); + CHECK_FIELD(location, "base board location"); + CHECK_FIELD(type, "base board type"); +#undef CHECK_FIELD + + identical = true; + cleanup: + return identical; +} + bool virSysinfoIsEqual(virSysinfoDefPtr src, virSysinfoDefPtr dst) { @@ -1269,6 +1424,9 @@ bool virSysinfoIsEqual(virSysinfoDefPtr src, if (!virSysinfoSystemIsEqual(src->system, dst->system)) goto cleanup; + if (!virSysinfoBaseBoardIsEqual(src->baseBoard, dst->baseBoard)) + goto cleanup; + identical = true; cleanup: diff --git a/src/util/virsysinfo.h b/src/util/virsysinfo.h index c8cc1e8..fd727a6 100644 --- a/src/util/virsysinfo.h +++ b/src/util/virsysinfo.h @@ -86,6 +86,18 @@ struct _virSysinfoSystemDef { char *family; }; +typedef struct _virSysinfoBaseBoardDef virSysinfoBaseBoardDef; +typedef virSysinfoBaseBoardDef *virSysinfoBaseBoardDefPtr; +struct _virSysinfoBaseBoardDef { + char *manufacturer; + char *product; + char *version; + char *serial; + char *asset; + char *location; + char *type; +}; + typedef struct _virSysinfoDef virSysinfoDef; typedef virSysinfoDef *virSysinfoDefPtr; struct _virSysinfoDef { @@ -93,6 +105,7 @@ struct _virSysinfoDef { virSysinfoBIOSDefPtr bios; virSysinfoSystemDefPtr system; + virSysinfoBaseBoardDefPtr baseBoard; size_t nprocessor; virSysinfoProcessorDefPtr processor; @@ -105,6 +118,7 @@ virSysinfoDefPtr virSysinfoRead(void); void virSysinfoBIOSDefFree(virSysinfoBIOSDefPtr def); void virSysinfoSystemDefFree(virSysinfoSystemDefPtr def); +void virSysinfoBaseBoardDefFree(virSysinfoBaseBoardDefPtr def); void virSysinfoDefFree(virSysinfoDefPtr def); int virSysinfoFormat(virBufferPtr buf, virSysinfoDefPtr def) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smbios.args b/tests/qemuxml2argvdata/qemuxml2argv-smbios.args index e939aca..7336cbe 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smbios.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-smbios.args @@ -4,5 +4,7 @@ pc -m 214 -smp 1 -smbios 'type=0,vendor=LENOVO,version=6FET82WW (3.12 )' \ -smbios 'type=1,manufacturer=Fedora,product=Virt-Manager,version=0.8.2-3.fc14,\ serial=32dfcb37-5af1-552b-357c-be8c3aa38310,\ uuid=c7a5fdbd-edaf-9455-926a-d65c16db1809,sku=1234567890,family=Red Hat' \ +-smbios 'type=2,manufacturer=Hewlett-Packard,product=0B4Ch,version=D,\ +serial=CZC1065993,asset=CZC1065993,location=Upside down,family=Motherboard' \ -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \ /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml b/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml index a2caeea..924596c 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml @@ -18,6 +18,15 @@ <entry name='sku'>1234567890</entry> <entry name='family'>Red Hat</entry> </system> + <baseBoard> + <entry name='manufacturer'>Hewlett-Packard</entry> + <entry name='product'>0B4Ch</entry> + <entry name='version'>D</entry> + <entry name='serial'>CZC1065993</entry> + <entry name='asset'>CZC1065993</entry> + <entry name='location'>Upside down</entry> + <entry name='type'>Motherboard</entry> + </baseBoard> </sysinfo> <os> <type arch='i686' machine='pc'>hvm</type> -- 2.3.6
On 05/12/2015 10:56 AM, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1220527
This type of information defines attributes of a system baseboard. With one caveat: in qemu they call it family, while in the specification it's referred to as type. I'm sticking with the latter.
Hmm... It seems qemu_smbios_type2_opts has 'type' (and it's a number, more on that later...)... The "type1_opts" has 'family'... Or am I looking at the wrong place... I just pulled/built the latest sources from git://git.qemu.org/qemu.git and see: ./qemu-system-x86_64 --help | grep smbios -smbios file=binary -smbios type=0[,vendor=str][,version=str][,date=str][,release=%d.%d] -smbios type=1[,manufacturer=str][,product=str][,version=str][,serial=str] -smbios type=2[,manufacturer=str][,product=str][,version=str][,serial=str] -smbios type=3[,manufacturer=str][,version=str][,serial=str][,asset=str] -smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str] -smbios type=17[,loc_pfx=str][,bank=str][,manufacturer=str][,serial=str]
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/formatdomain.html.in | 37 +++++- docs/schemas/domaincommon.rng | 24 ++++ src/conf/domain_conf.c | 57 +++++++++ src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 48 +++++++ src/util/virsysinfo.c | 160 +++++++++++++++++++++++- src/util/virsysinfo.h | 14 +++ tests/qemuxml2argvdata/qemuxml2argv-smbios.args | 2 + tests/qemuxml2argvdata/qemuxml2argv-smbios.xml | 9 ++ 9 files changed, 346 insertions(+), 6 deletions(-)
According to how I read page 34 of the pdf file from .0: NOTE If more than one Type 2 structure is provided by an SMBIOS implementation, each structure shall include the Number of Contained Object Handles and Contained Object Handles fields to specify which system elements are contained on which boards. It seems we can have one of these in the SMBIOS... our .rng supports it, but we don't document it that way nor does the code handle more than one. Although I'll note that the Type0 (BIOS) and Type1 (SYSTEM) only support one, but the .rng has "oneOrMore"... If QEMU doesn't support more than 1, then we will need to document/ describe that. I assume there could be some other hypervisor that may want to support more than one. Also it seems that based on settings found in Type2, there could be a need/use for Type3 data.
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e0b6ba7..dc92aa3 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -375,6 +375,13 @@ <entry name='product'>Virt-Manager</entry> <entry name='version'>0.9.4</entry> </system> + <baseBoard> + <entry name='manufacturer'>LENOVO</entry> + <entry name='product'>20BE0061MC</entry> + <entry name='version'>0B98401 Pro</entry> + <entry name='serial'>W1KS427111E</entry> + <entry name='type'>Motherboard</entry> + </baseBoard>
According to how I read that .pdf - board type is read/passed as an ENUM not a STRING. Table 15 of the spec on page 36 describes the values (01 -> 0D currently).
</sysinfo> ...</pre>
@@ -435,11 +442,31 @@ <dt><code>family</code></dt> <dd>Identify the family a particular computer belongs to.</dd> </dl> - NB: Incorrectly supplied entries in either the <code>bios</code> - or <code>system</code> blocks will be ignored without error. - Other than <code>uuid</code> validation and <code>date</code> - format checking, all values are passed as strings to the - hypervisor driver. + </dd> + <dt><code>baseBoard</code></dt> + <dd> + This is block 2 of SMBIOS, with entry names drawn from: + <dl> + <dt><code>manufacturer</code></dt> + <dd>Manufacturer of BIOS</dd> + <dt><code>product</code></dt> + <dd>Product Name</dd> + <dt><code>version</code></dt> + <dd>Version of the product</dd> + <dt><code>serial</code></dt> + <dd>Serial number</dd> + <dt><code>asset</code></dt> + <dd>Asset tag</dd> + <dt><code>location</code></dt> + <dd>Location in chassis</dd> + <dt><code>type</code></dt> + <dd>Board type</dd> + </dl> + NB: Incorrectly supplied entries in either the <code>bios</code> or
s/in either/for/ s/or/, /
+ <code>system</code> or <code>baseBoard</code> blocks will be + ignored without error. Other than <code>uuid</code> validation and + <code>date</code> format checking, all values are passed as strings + to the hypervisor driver. </dd> </dl> </dd> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index c151e92..2bc84b5 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4240,6 +4240,18 @@ </oneOrMore> </element> </optional> + <optional> + <element name="baseBoard"> + <oneOrMore> + <element name="entry"> + <attribute name="name"> + <ref name="sysinfo-baseBoard-name"/> + </attribute> + <ref name="sysinfo-value"/> + </element> + </oneOrMore> + </element> + </optional> </interleave> </element> </define> @@ -4265,6 +4277,18 @@ </choice> </define>
+ <define name="sysinfo-baseBoard-name"> + <choice> + <value>manufacturer</value> + <value>product</value> + <value>version</value> + <value>serial</value> + <value>asset</value> + <value>location</value> + <value>type</value> + </choice> + </define> + <define name="sysinfo-value"> <data type="string"> <param name='pattern'>[a-zA-Z0-9/\-_\. \(\)]+</param> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 209416d..0f41375 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10997,6 +10997,52 @@ virSysinfoSystemParseXML(xmlNodePtr node, return ret; }
+static int +virSysinfoBaseBoardParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, + virSysinfoBaseBoardDefPtr *baseBoard) +{ + int ret = -1; + virSysinfoBaseBoardDefPtr def; + + if (!xmlStrEqual(node->name, BAD_CAST "baseBoard")) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("XML does not contain expected 'baseBoard' element")); + return ret; + } + + if (VIR_ALLOC(def) < 0) + goto cleanup; + + def->manufacturer = + virXPathString("string(entry[@name='manufacturer'])", ctxt); + def->product = + virXPathString("string(entry[@name='product'])", ctxt); + def->version = + virXPathString("string(entry[@name='version'])", ctxt); + def->serial = + virXPathString("string(entry[@name='serial'])", ctxt); + def->asset = + virXPathString("string(entry[@name='asset'])", ctxt); + def->location = + virXPathString("string(entry[@name='location'])", ctxt); + def->type = + virXPathString("string(entry[@name='type'])", ctxt);
While I suppose 'type' could be read as string, it is a BYTE ENUM value.
+ + if (!def->manufacturer && !def->product && !def->version && + !def->serial && !def->asset && !def->location && !def->type) { + virSysinfoBaseBoardDefFree(def); + def = NULL; + } + + *baseBoard = def; + def = NULL; + ret = 0; + cleanup: + virSysinfoBaseBoardDefFree(def); + return ret; +} + static virSysinfoDefPtr virSysinfoParseXML(xmlNodePtr node, xmlXPathContextPtr ctxt, @@ -11051,6 +11097,17 @@ virSysinfoParseXML(xmlNodePtr node, ctxt->node = oldnode; }
+ /* Extract system base board metadata */
And painfully - there can be more than one according the spec...
+ if ((tmpnode = virXPathNode("./baseBoard[1]", ctxt)) != NULL) { + oldnode = ctxt->node; + ctxt->node = tmpnode; + if (virSysinfoBaseBoardParseXML(tmpnode, ctxt, &def->baseBoard) < 0) { + ctxt->node = oldnode; + goto error; + } + ctxt->node = oldnode; + } + cleanup: VIR_FREE(type); return def; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index bd619d3..3178fe9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2176,6 +2176,7 @@ virVasprintfInternal;
# util/virsysinfo.h +virSysinfoBaseBoardDefFree; virSysinfoBIOSDefFree; virSysinfoDefFree; virSysinfoFormat; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 489cab3..c13089e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6738,6 +6738,48 @@ static char *qemuBuildSmbiosSystemStr(virSysinfoSystemDefPtr def, return NULL; }
+static char *qemuBuildSmbiosBaseBoardStr(virSysinfoBaseBoardDefPtr def) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + + if (!def) + return NULL; + + virBufferAddLit(&buf, "type=2"); + + /* 1:Manufacturer */ + if (def->manufacturer) + virBufferAsprintf(&buf, ",manufacturer=%s", + def->manufacturer); + /* 1:Product Name */ + if (def->product) + virBufferAsprintf(&buf, ",product=%s", def->product); + /* 1:Version */ + if (def->version) + virBufferAsprintf(&buf, ",version=%s", def->version); + /* 1:Serial Number */ + if (def->serial) + virBufferAsprintf(&buf, ",serial=%s", def->serial); + /* 1:Asset Tag */ + if (def->asset) + virBufferAsprintf(&buf, ",asset=%s", def->asset); + /* 1:Location */ + if (def->location) + virBufferAsprintf(&buf, ",location=%s", def->location); + /* 1:Type */ + if (def->type) + virBufferAsprintf(&buf, ",family=%s", def->type);
And painfully if we handle more than one definition, then we'll have to deal with that here. Again, unless I'm looking at the wrong place - there is a 'type', but I'm not sure how it's passed
+ + if (virBufferCheckError(&buf) < 0) + goto error; + + return virBufferContentAndReset(&buf); + + error: + virBufferFreeAndReset(&buf); + return NULL; +} + static char * qemuBuildClockArgStr(virDomainClockDefPtr def) { @@ -8925,6 +8967,12 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL); VIR_FREE(smbioscmd); } + + smbioscmd = qemuBuildSmbiosBaseBoardStr(source->baseBoard); + if (smbioscmd != NULL) { + virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL); + VIR_FREE(smbioscmd); + } } }
diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 4c939ec..bb21e1b 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -91,6 +91,20 @@ void virSysinfoSystemDefFree(virSysinfoSystemDefPtr def) VIR_FREE(def); }
+void virSysinfoBaseBoardDefFree(virSysinfoBaseBoardDefPtr def) +{ + if (def == NULL) + return; + + VIR_FREE(def->manufacturer); + VIR_FREE(def->product); + VIR_FREE(def->version); + VIR_FREE(def->serial); + VIR_FREE(def->asset); + VIR_FREE(def->location); + VIR_FREE(def->type); + VIR_FREE(def); +}
/** * virSysinfoDefFree: @@ -108,6 +122,7 @@ void virSysinfoDefFree(virSysinfoDefPtr def)
virSysinfoBIOSDefFree(def->bios); virSysinfoSystemDefFree(def->system); + virSysinfoBaseBoardDefFree(def->baseBoard);
for (i = 0; i < def->nprocessor; i++) { VIR_FREE(def->processor[i].processor_socket_destination); @@ -719,6 +734,77 @@ virSysinfoParseSystem(const char *base, virSysinfoSystemDefPtr *system) }
static int +virSysinfoParseBaseBoard(const char *base, virSysinfoBaseBoardDefPtr *baseBoard) +{ + int ret = -1; + const char *cur, *eol = NULL; + virSysinfoBaseBoardDefPtr def; + + if ((cur = strstr(base, "Base Board Information")) == NULL) + return 0; + + if (VIR_ALLOC(def) < 0) + return ret; + + base = cur; + if ((cur = strstr(base, "Manufacturer: ")) != NULL) { + cur += 14; + eol = strchr(cur, '\n'); + if (eol && VIR_STRNDUP(def->manufacturer, cur, eol - cur) < 0) + goto cleanup; + } + if ((cur = strstr(base, "Product Name: ")) != NULL) { + cur += 14; + eol = strchr(cur, '\n'); + if (eol && VIR_STRNDUP(def->product, cur, eol - cur) < 0) + goto cleanup; + } + if ((cur = strstr(base, "Version: ")) != NULL) { + cur += 9; + eol = strchr(cur, '\n'); + if (eol && VIR_STRNDUP(def->version, cur, eol - cur) < 0) + goto cleanup; + } + if ((cur = strstr(base, "Serial Number: ")) != NULL) { + cur += 15; + eol = strchr(cur, '\n'); + if (eol && VIR_STRNDUP(def->serial, cur, eol - cur) < 0) + goto cleanup; + } + if ((cur = strstr(base, "Asset Tag: ")) != NULL) { + cur += 11; + eol = strchr(cur, '\n'); + if (eol && VIR_STRNDUP(def->asset, cur, eol - cur) < 0) + goto cleanup; + } + if ((cur = strstr(base, "Location In Chassis: ")) != NULL) { + cur += 21; + eol = strchr(cur, '\n'); + if (eol && VIR_STRNDUP(def->location, cur, eol - cur) < 0) + goto cleanup; + } + if ((cur = strstr(base, "Type: ")) != NULL) { + cur += 6; + eol = strchr(cur, '\n'); + if (eol && VIR_STRNDUP(def->type, cur, eol - cur) < 0) + goto cleanup; + }
More than one issue as well...
+ + if (!def->manufacturer && !def->product && !def->version && + !def->serial && !def->asset && !def->location && !def->type) { + virSysinfoBaseBoardDefFree(def); + def = NULL; + } + + *baseBoard = def; + def = NULL; + ret = 0; + cleanup: + virSysinfoBaseBoardDefFree(def); + return ret; +} + +static int virSysinfoParseProcessor(const char *base, virSysinfoDefPtr ret) { const char *cur, *tmp_base; @@ -940,7 +1026,7 @@ virSysinfoRead(void) return NULL; }
- cmd = virCommandNewArgList(path, "-q", "-t", "0,1,4,17", NULL); + cmd = virCommandNewArgList(path, "-q", "-t", "0,1,2,4,17", NULL); VIR_FREE(path); virCommandSetOutputBuffer(cmd, &outbuf); if (virCommandRun(cmd, NULL) < 0) @@ -957,6 +1043,9 @@ virSysinfoRead(void) if (virSysinfoParseSystem(outbuf, &ret->system) < 0) goto error;
+ if (virSysinfoParseBaseBoard(outbuf, &ret->baseBoard) < 0) + goto error; +
How do we handle more than one?
ret->nprocessor = 0; ret->processor = NULL; if (virSysinfoParseProcessor(outbuf, ret) < 0) @@ -1027,6 +1116,32 @@ virSysinfoSystemFormat(virBufferPtr buf, virSysinfoSystemDefPtr def) }
static void +virSysinfoBaseBoardFormat(virBufferPtr buf, virSysinfoBaseBoardDefPtr def) +{ + if (!def) + return; + + virBufferAddLit(buf, "<baseBoard>\n"); + virBufferAdjustIndent(buf, 2); + virBufferEscapeString(buf, "<entry name='manufacturer'>%s</entry>\n", + def->manufacturer); + virBufferEscapeString(buf, "<entry name='product'>%s</entry>\n", + def->product); + virBufferEscapeString(buf, "<entry name='version'>%s</entry>\n", + def->version); + virBufferEscapeString(buf, "<entry name='serial'>%s</entry>\n", + def->serial); + virBufferEscapeString(buf, "<entry name='asset'>%s</entry>\n", + def->asset); + virBufferEscapeString(buf, "<entry name='location'>%s</entry>\n", + def->location); + virBufferEscapeString(buf, "<entry name='type'>%s</entry>\n", + def->type);
How do we handle more than one...
+ virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</baseBoard>\n"); +} + +static void virSysinfoProcessorFormat(virBufferPtr buf, virSysinfoDefPtr def) { size_t i; @@ -1157,6 +1272,7 @@ virSysinfoFormat(virBufferPtr buf, virSysinfoDefPtr def)
virSysinfoBIOSFormat(buf, def->bios); virSysinfoSystemFormat(buf, def->system); + virSysinfoBaseBoardFormat(buf, def->baseBoard); virSysinfoProcessorFormat(buf, def); virSysinfoMemoryFormat(buf, def);
@@ -1227,6 +1343,7 @@ virSysinfoSystemIsEqual(virSysinfoSystemDefPtr src, desc, NULLSTR(src->name), NULLSTR(dst->name)); \ } \ } while (0) +
^^ This should be in patch 2...
CHECK_FIELD(manufacturer, "system vendor"); CHECK_FIELD(product, "system product"); CHECK_FIELD(version, "system version"); @@ -1241,6 +1358,44 @@ virSysinfoSystemIsEqual(virSysinfoSystemDefPtr src, return identical; }
+static bool +virSysinfoBaseBoardIsEqual(virSysinfoBaseBoardDefPtr src, + virSysinfoBaseBoardDefPtr dst) +{ + bool identical = false; + + if (!src && !dst) + return true; + + if ((src && !dst) || (!src && dst)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Target base board 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(manufacturer, "base board vendor"); + CHECK_FIELD(product, "base board product"); + CHECK_FIELD(version, "base board version"); + CHECK_FIELD(serial, "base board serial"); + CHECK_FIELD(asset, "base board asset"); + CHECK_FIELD(location, "base board location"); + CHECK_FIELD(type, "base board type"); +#undef CHECK_FIELD + + identical = true; + cleanup: + return identical; +} + bool virSysinfoIsEqual(virSysinfoDefPtr src, virSysinfoDefPtr dst) { @@ -1269,6 +1424,9 @@ bool virSysinfoIsEqual(virSysinfoDefPtr src, if (!virSysinfoSystemIsEqual(src->system, dst->system)) goto cleanup;
+ if (!virSysinfoBaseBoardIsEqual(src->baseBoard, dst->baseBoard)) + goto cleanup; + identical = true;
cleanup: diff --git a/src/util/virsysinfo.h b/src/util/virsysinfo.h index c8cc1e8..fd727a6 100644 --- a/src/util/virsysinfo.h +++ b/src/util/virsysinfo.h @@ -86,6 +86,18 @@ struct _virSysinfoSystemDef { char *family; };
+typedef struct _virSysinfoBaseBoardDef virSysinfoBaseBoardDef; +typedef virSysinfoBaseBoardDef *virSysinfoBaseBoardDefPtr; +struct _virSysinfoBaseBoardDef { + char *manufacturer; + char *product; + char *version; + char *serial; + char *asset; + char *location; + char *type; +}; + typedef struct _virSysinfoDef virSysinfoDef; typedef virSysinfoDef *virSysinfoDefPtr; struct _virSysinfoDef { @@ -93,6 +105,7 @@ struct _virSysinfoDef {
virSysinfoBIOSDefPtr bios; virSysinfoSystemDefPtr system; + virSysinfoBaseBoardDefPtr baseBoard;
size_t nprocessor; virSysinfoProcessorDefPtr processor; @@ -105,6 +118,7 @@ virSysinfoDefPtr virSysinfoRead(void);
void virSysinfoBIOSDefFree(virSysinfoBIOSDefPtr def); void virSysinfoSystemDefFree(virSysinfoSystemDefPtr def); +void virSysinfoBaseBoardDefFree(virSysinfoBaseBoardDefPtr def); void virSysinfoDefFree(virSysinfoDefPtr def);
int virSysinfoFormat(virBufferPtr buf, virSysinfoDefPtr def) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smbios.args b/tests/qemuxml2argvdata/qemuxml2argv-smbios.args index e939aca..7336cbe 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smbios.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-smbios.args @@ -4,5 +4,7 @@ pc -m 214 -smp 1 -smbios 'type=0,vendor=LENOVO,version=6FET82WW (3.12 )' \ -smbios 'type=1,manufacturer=Fedora,product=Virt-Manager,version=0.8.2-3.fc14,\ serial=32dfcb37-5af1-552b-357c-be8c3aa38310,\ uuid=c7a5fdbd-edaf-9455-926a-d65c16db1809,sku=1234567890,family=Red Hat' \ +-smbios 'type=2,manufacturer=Hewlett-Packard,product=0B4Ch,version=D,\ +serial=CZC1065993,asset=CZC1065993,location=Upside down,family=Motherboard' \
Adjust here...
-nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \ /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml b/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml index a2caeea..924596c 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml @@ -18,6 +18,15 @@ <entry name='sku'>1234567890</entry> <entry name='family'>Red Hat</entry> </system> + <baseBoard> + <entry name='manufacturer'>Hewlett-Packard</entry> + <entry name='product'>0B4Ch</entry> + <entry name='version'>D</entry> + <entry name='serial'>CZC1065993</entry> + <entry name='asset'>CZC1065993</entry> + <entry name='location'>Upside down</entry> + <entry name='type'>Motherboard</entry>
This needs to be 01 -> 0D, not a string.... Also I don't see how qemu code actually processes this.
+ </baseBoard> </sysinfo> <os> <type arch='i686' machine='pc'>hvm</type>
If we're going to support it, then an example with two (or more entries). John
On 18.05.2015 20:48, John Ferlan wrote:
On 05/12/2015 10:56 AM, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1220527
This type of information defines attributes of a system baseboard. With one caveat: in qemu they call it family, while in the specification it's referred to as type. I'm sticking with the latter.
Hmm...
It seems qemu_smbios_type2_opts has 'type' (and it's a number, more on that later...)... The "type1_opts" has 'family'...
Or am I looking at the wrong place... I just pulled/built the latest sources from git://git.qemu.org/qemu.git and see:
A-ha! Looking at qemu-options.hx (more precisely on commit b155eb1d0409eff4d0e7f33c746c81434f0ea629) it shows that qemu supports @family. Well, digging deeper into the code shows it doesn't. I'll patch qemu firstly and then resend my patches.
./qemu-system-x86_64 --help | grep smbios -smbios file=binary -smbios type=0[,vendor=str][,version=str][,date=str][,release=%d.%d] -smbios type=1[,manufacturer=str][,product=str][,version=str][,serial=str] -smbios type=2[,manufacturer=str][,product=str][,version=str][,serial=str] -smbios type=3[,manufacturer=str][,version=str][,serial=str][,asset=str] -smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str] -smbios type=17[,loc_pfx=str][,bank=str][,manufacturer=str][,serial=str]
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/formatdomain.html.in | 37 +++++- docs/schemas/domaincommon.rng | 24 ++++ src/conf/domain_conf.c | 57 +++++++++ src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 48 +++++++ src/util/virsysinfo.c | 160 +++++++++++++++++++++++- src/util/virsysinfo.h | 14 +++ tests/qemuxml2argvdata/qemuxml2argv-smbios.args | 2 + tests/qemuxml2argvdata/qemuxml2argv-smbios.xml | 9 ++ 9 files changed, 346 insertions(+), 6 deletions(-)
According to how I read page 34 of the pdf file from .0:
NOTE If more than one Type 2 structure is provided by an SMBIOS implementation, each structure shall include the Number of Contained Object Handles and Contained Object Handles fields to specify which system elements are contained on which boards.
It seems we can have one of these in the SMBIOS... our .rng supports it, but we don't document it that way nor does the code handle more than one. Although I'll note that the Type0 (BIOS) and Type1 (SYSTEM) only support one, but the .rng has "oneOrMore"...
If QEMU doesn't support more than 1, then we will need to document/ describe that. I assume there could be some other hypervisor that may want to support more than one.
Also it seems that based on settings found in Type2, there could be a need/use for Type3 data.
Fortunately, this is filled in with some defaults by qemu. So we can save that for later. The rest of the comments will be addressed in v2. Michal
participants (2)
-
John Ferlan -
Michal Privoznik