On 02/08/2018 02:05 AM, Zhuangyanying wrote: > From: Zhuang Yanying <ann.zhuangyanying@xxxxxxxxxx> > > 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); > > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
