[libvirt] [RFC] virSysinfo: Introduce SMBIOS type 3 support
John Ferlan
jferlan at redhat.com
Tue Feb 13 15:20:31 UTC 2018
On 02/08/2018 02:05 AM, Zhuangyanying wrote:
> From: Zhuang Yanying <ann.zhuangyanying at huawei.com>
>
> Some applications inside VM need to access SMBIOS Chassis Asset Tag,
> which should be emulated. It has already been realized in qemu,
>
> SMBIOS: Build aggregate smbios tables and entry point
> https://git.qemu.org/?p=qemu.git;a=commit;h=c97294ec1b9e36887e119589d456557d72ab37b5
>
> but not in libvirt. we realize it here.
> As an example, you could use something like
> <chassis>
> <entry name='manufacturer'>LENOVO</entry>
> <entry name='version'>0B98401 Pro</entry>
> <entry name='serial'>W1KS427111E</entry>
> <entry name='asset'>344dfTYH#</entry>
> </chassis>
> ---
> src/conf/domain_conf.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_command.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++
> src/util/virsysinfo.c | 37 +++++++++++++++++++++++++++++++++++
> src/util/virsysinfo.h | 13 +++++++++++++
> 4 files changed, 151 insertions(+)
>
Looks good so far - a few minor notes here, but you need a few more
adjustments as well.
I suggest looking at commit id 'a9a27e60' which added SMBIOS type 2
(baseboard) support in order to get an idea of the files that should be
changed along with those that you did change.
In particular, describing the <chassis> in formatdomain.html.in,
ensuring that the schema domaincommon.rng is updated to describe the new
XML, and finally xml2xml as well as xml2argv test adjustments that would
list the type 3 information for xml2xml and the command line for xml2argv.
NB: It's also desirable to create 2 patches from this one. The first
being the domain_conf, virsysinfo, docs, rng, and xml2xml with the
second being just the qemu_command and xml2argv. Finally a 3rd patch to
adjust docs/news.xml to describe the change.
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 34aae82..b0d5d68 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -14506,6 +14506,47 @@ virSysinfoBaseBoardParseXML(xmlXPathContextPtr ctxt,
> return ret;
> }
>
More recent style is two empty lines between functions.
> +static int
> +virSysinfoChassisParseXML(xmlNodePtr node,
> + xmlXPathContextPtr ctxt,
> + virSysinfoChassisDefPtr *chassisdef)
> +{
> + int ret = -1;
> + virSysinfoChassisDefPtr def;
> +
> + if (!xmlStrEqual(node->name, BAD_CAST "chassis")) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("XML does not contain expected 'chassis' element"));
> + return ret;
> + }
> +
> + if (VIR_ALLOC(def) < 0)
> + goto cleanup;
> +
> + def->manufacturer =
> + virXPathString("string(entry[@name='manufacturer'])", 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->sku =
> + virXPathString("string(entry[@name='sku'])", ctxt);
> +
> + if (!def->manufacturer && !def->version &&
> + !def->serial && !def->asset && !def->sku) {
> + virSysinfoChassisDefFree(def);
> + def = NULL;
> + }
> +
> + *chassisdef = def;
> + def = NULL;
> + ret = 0;
> + cleanup:
> + virSysinfoChassisDefFree(def);
> + return ret;
> +}
>
Again 2 empty lines.
> static int
> virSysinfoOEMStringsParseXML(xmlXPathContextPtr ctxt,
> @@ -14600,6 +14641,17 @@ virSysinfoParseXML(xmlNodePtr node,
> if (virSysinfoBaseBoardParseXML(ctxt, &def->baseBoard, &def->nbaseBoard) < 0)
> goto error;
>
> + /* Extract chassis metadata */
> + if ((tmpnode = virXPathNode("./chassis[1]", ctxt)) != NULL) {
> + oldnode = ctxt->node;
> + ctxt->node = tmpnode;
> + if (virSysinfoChassisParseXML(tmpnode, ctxt, &def->chassis) < 0) {
> + ctxt->node = oldnode;
> + goto error;
> + }
> + ctxt->node = oldnode;
> + }
> +
> /* Extract system related metadata */
> if ((tmpnode = virXPathNode("./oemStrings[1]", ctxt)) != NULL) {
> oldnode = ctxt->node;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 24b434a..cf88e75 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -5813,6 +5813,49 @@ qemuBuildSmbiosBaseBoardStr(virSysinfoBaseBoardDefPtr def)
> return NULL;
> }
>
Same w/ empty lines.
> +static char *
> +qemuBuildSmbiosChassisStr(virSysinfoChassisDefPtr def)
> +{
> + virBuffer buf = VIR_BUFFER_INITIALIZER;
> +
> + if (!def)
> + return NULL;
> +
> + virBufferAddLit(&buf, "type=3");
> +
> + /* 2:Manufacturer */
> + virBufferAddLit(&buf, ",manufacturer=");
> + virQEMUBuildBufferEscapeComma(&buf, def->manufacturer);
> + /* 2:Version */
> + if (def->version) {
> + virBufferAddLit(&buf, ",version=");
> + virQEMUBuildBufferEscapeComma(&buf, def->version);
> + }
> + /* 2:Serial Number */
> + if (def->serial) {
> + virBufferAddLit(&buf, ",serial=");
> + virQEMUBuildBufferEscapeComma(&buf, def->serial);
> + }
> + /* 2:Asset Tag */
> + if (def->asset) {
> + virBufferAddLit(&buf, ",asset=");
> + virQEMUBuildBufferEscapeComma(&buf, def->asset);
> + }
> + /* 2:Sku */
> + if (def->sku) {
> + virBufferAddLit(&buf, ",sku=");
> + virQEMUBuildBufferEscapeComma(&buf, def->sku);
> + }
> +
> + if (virBufferCheckError(&buf) < 0)
> + goto error;
> +
> + return virBufferContentAndReset(&buf);
> +
> + error:
> + virBufferFreeAndReset(&buf);
> + return NULL;
> +}
>
again.
> static char *
> qemuBuildSmbiosOEMStringsStr(virSysinfoOEMStringsDefPtr def)
> @@ -5905,6 +5948,12 @@ qemuBuildSmbiosCommandLine(virCommandPtr cmd,
> VIR_FREE(smbioscmd);
> }
>
> + smbioscmd = qemuBuildSmbiosChassisStr(source->chassis);
> + if (smbioscmd != NULL) {
> + virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL);
> + VIR_FREE(smbioscmd);
> + }
> +
> if (source->oemStrings) {
> if (!(smbioscmd = qemuBuildSmbiosOEMStringsStr(source->oemStrings)))
> return -1;
> diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c
> index e8d3371..af434e0 100644
> --- a/src/util/virsysinfo.c
> +++ b/src/util/virsysinfo.c
> @@ -108,6 +108,18 @@ void virSysinfoBaseBoardDefClear(virSysinfoBaseBoardDefPtr def)
> VIR_FREE(def->location);
> }
>
> +void virSysinfoChassisDefFree(virSysinfoChassisDefPtr def)
> +{
> + if (def == NULL)
> + return;
> +
> + VIR_FREE(def->manufacturer);
> + VIR_FREE(def->version);
> + VIR_FREE(def->serial);
> + VIR_FREE(def->asset);
> + VIR_FREE(def->sku);
VIR_FREE(def);
Since this isn't a DefClear function like the BaseboardDefClear
> +}
> +
> void virSysinfoOEMStringsDefFree(virSysinfoOEMStringsDefPtr def)
> {
> size_t i;
> @@ -143,6 +155,8 @@ void virSysinfoDefFree(virSysinfoDefPtr def)
> virSysinfoBaseBoardDefClear(def->baseBoard + i);
> VIR_FREE(def->baseBoard);
>
> + virSysinfoChassisDefFree(def->chassis);
> +
> for (i = 0; i < def->nprocessor; i++) {
> VIR_FREE(def->processor[i].processor_socket_destination);
> VIR_FREE(def->processor[i].processor_type);
> @@ -1203,6 +1217,28 @@ virSysinfoBaseBoardFormat(virBufferPtr buf,
> }
>
2 empty lines
> static void
> +virSysinfoChassisFormat(virBufferPtr buf, virSysinfoChassisDefPtr def)
Use multiple lines for args, eg.
virSysinfoChassisFormat(virBufferPtr buf,
virSysinfoChassisDefPtr def)
> +{
> + if (!def)
> + return;
> +
> + virBufferAddLit(buf, "<chassis>\n");
> + virBufferAdjustIndent(buf, 2);
> + virBufferEscapeString(buf, "<entry name='manufacturer'>%s</entry>\n",
> + def->manufacturer);
> + 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='sku'>%s</entry>\n",
> + def->sku);
> + virBufferAdjustIndent(buf, -2);
> + virBufferAddLit(buf, "</chassis>\n");
> +}
> +
2 empty lines
> +static void
> virSysinfoProcessorFormat(virBufferPtr buf, virSysinfoDefPtr def)
> {
> size_t i;
> @@ -1354,6 +1390,7 @@ virSysinfoFormat(virBufferPtr buf, virSysinfoDefPtr def)
> virSysinfoBIOSFormat(&childrenBuf, def->bios);
> virSysinfoSystemFormat(&childrenBuf, def->system);
> virSysinfoBaseBoardFormat(&childrenBuf, def->baseBoard, def->nbaseBoard);
> + virSysinfoChassisFormat(&childrenBuf, def->chassis);
> virSysinfoProcessorFormat(&childrenBuf, def);
> virSysinfoMemoryFormat(&childrenBuf, def);
> virSysinfoOEMStringsFormat(&childrenBuf, def->oemStrings);
> diff --git a/src/util/virsysinfo.h b/src/util/virsysinfo.h
> index ecb3a36..00a15db 100644
> --- a/src/util/virsysinfo.h
> +++ b/src/util/virsysinfo.h
> @@ -98,6 +98,16 @@ struct _virSysinfoBaseBoardDef {
> /* XXX board type */
> };
>
> +typedef struct _virSysinfoChassisDef virSysinfoChassisDef;
> +typedef virSysinfoChassisDef *virSysinfoChassisDefPtr;
> +struct _virSysinfoChassisDef {
> + char *manufacturer;
> + char *version;
> + char *serial;
> + char *asset;
> + char *sku;
> +};
> +
> typedef struct _virSysinfoOEMStringsDef virSysinfoOEMStringsDef;
> typedef virSysinfoOEMStringsDef *virSysinfoOEMStringsDefPtr;
> struct _virSysinfoOEMStringsDef {
> @@ -116,6 +126,8 @@ struct _virSysinfoDef {
> size_t nbaseBoard;
> virSysinfoBaseBoardDefPtr baseBoard;
>
> + virSysinfoChassisDefPtr chassis;
> +
> size_t nprocessor;
> virSysinfoProcessorDefPtr processor;
>
> @@ -130,6 +142,7 @@ virSysinfoDefPtr virSysinfoRead(void);
> void virSysinfoBIOSDefFree(virSysinfoBIOSDefPtr def);
> void virSysinfoSystemDefFree(virSysinfoSystemDefPtr def);
> void virSysinfoBaseBoardDefClear(virSysinfoBaseBoardDefPtr def);
> +void virSysinfoChassisDefFree(virSysinfoChassisDefPtr def);
Don't forget to update 'src/libvirt_private.syms' to appropriately add
the API (alphabetically after virSysinfoBIOSDefFree)
John
> void virSysinfoOEMStringsDefFree(virSysinfoOEMStringsDefPtr def);
> void virSysinfoDefFree(virSysinfoDefPtr def);
>
>
More information about the libvir-list
mailing list