[libvirt] [PATCH v2 0/2] Support SMBIOS OEM strings

A followup to https://www.redhat.com/archives/libvir-list/2017-November/msg00720.html The QEMU patch is now merged (for next 2.12 release). Since v2: - Remove redundant error message report - Split QEMU from XML parts of patch Daniel P. Berrange (2): conf: add support for setting OEM strings SMBIOS data fields qemu: add support for generating SMBIOS OEM strings command line docs/formatdomain.html.in | 13 ++++++++++ docs/schemas/domaincommon.rng | 9 +++++++ src/conf/domain_conf.c | 47 +++++++++++++++++++++++++++++++++++++ src/qemu/qemu_command.c | 28 ++++++++++++++++++++++ src/util/virsysinfo.c | 33 ++++++++++++++++++++++++++ src/util/virsysinfo.h | 10 ++++++++ tests/qemuxml2argvdata/smbios.args | 2 ++ tests/qemuxml2argvdata/smbios.xml | 5 ++++ tests/qemuxml2xmloutdata/smbios.xml | 5 ++++ 9 files changed, 152 insertions(+) -- 2.14.3

The OEM strings table in SMBIOS allows the vendor to pass arbitrary strings into the guest OS. This can be used as a way to pass data to an application like cloud-init, or potentially as an alternative to the kernel command line for OS installers where you can't modify the install ISO image to change the kernel args. As an example, consider if cloud-init and anaconda supported OEM strings you could use something like <oemStrings> <entry>cloud-init:ds=nocloud-net;s=http://10.10.0.1:8000/</entry> <entry>anaconda:method=http://dl.fedoraproject.org/pub/fedora/linux/releases/25/x86_64/os</entry> </oemStrings> use of a application specific prefix as illustrated above is recommended, but not mandated, so that an app can reliably identify which of the many OEM strings are targetted at it. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- docs/formatdomain.html.in | 13 ++++++++++++ docs/schemas/domaincommon.rng | 9 +++++++++ src/conf/domain_conf.c | 47 +++++++++++++++++++++++++++++++++++++++++++ src/util/virsysinfo.c | 33 ++++++++++++++++++++++++++++++ src/util/virsysinfo.h | 10 +++++++++ 5 files changed, 112 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index d272cc1ba6..6af2d26209 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -411,6 +411,10 @@ <entry name='version'>0B98401 Pro</entry> <entry name='serial'>W1KS427111E</entry> </baseBoard> + <oemStrings> + <entry>myappname:some arbitrary data</entry> + <entry>otherappname:more arbitrary data</entry> + </oemStrings> </sysinfo> ...</pre> @@ -498,6 +502,15 @@ validation and <code>date</code> format checking, all values are passed as strings to the hypervisor driver. </dd> + <dt><code>oemStrings</code></dt> + <dd> + This is block 11 of SMBIOS. This element should appear once and + can have multiple <code>entry</code> child elements, each providing + arbitrary string data. There are no restrictions on what data can + be provided in the entries, however, if the data is intended to be + consumed by an application in the guest, it is recommended to use + the application name as a prefix in the string. + </dd> </dl> </dd> </dl> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index f22c932f6c..a154b5a462 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4857,6 +4857,15 @@ </oneOrMore> </element> </zeroOrMore> + <optional> + <element name="oemStrings"> + <oneOrMore> + <element name="entry"> + <ref name="sysinfo-value"/> + </element> + </oneOrMore> + </element> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a1c25060f9..6c0ad1ed7d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14461,6 +14461,42 @@ virSysinfoBaseBoardParseXML(xmlXPathContextPtr ctxt, return ret; } +static int +virSysinfoOEMStringsParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, + virSysinfoOEMStringsDefPtr *oem) +{ + int ret = -1; + virSysinfoOEMStringsDefPtr def; + xmlNodePtr *strings = NULL; + int nstrings; + size_t i; + + nstrings = virXPathNodeSet("./entry", ctxt, &strings); + if (nstrings < 0) + return -1; + if (nstrings == 0) + return 0; + + if (VIR_ALLOC(def) < 0) + goto cleanup; + + if (VIR_ALLOC_N(def->values, nstrings) < 0) + goto cleanup; + + def->nvalues = nstrings; + for (i = 0; i < nstrings; i++) + def->values[i] = virXMLNodeContentString(strings[i]); + + *oem = def; + def = NULL; + ret = 0; + cleanup: + VIR_FREE(strings); + virSysinfoOEMStringsDefFree(def); + return ret; +} + static virSysinfoDefPtr virSysinfoParseXML(xmlNodePtr node, xmlXPathContextPtr ctxt, @@ -14519,6 +14555,17 @@ virSysinfoParseXML(xmlNodePtr node, if (virSysinfoBaseBoardParseXML(ctxt, &def->baseBoard, &def->nbaseBoard) < 0) goto error; + /* Extract system related metadata */ + if ((tmpnode = virXPathNode("./oemStrings[1]", ctxt)) != NULL) { + oldnode = ctxt->node; + ctxt->node = tmpnode; + if (virSysinfoOEMStringsParseXML(tmpnode, ctxt, &def->oemStrings) < 0) { + ctxt->node = oldnode; + goto error; + } + ctxt->node = oldnode; + } + cleanup: VIR_FREE(type); return def; diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 1fbdd778f9..60765c38de 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -108,6 +108,18 @@ void virSysinfoBaseBoardDefClear(virSysinfoBaseBoardDefPtr def) VIR_FREE(def->location); } +void virSysinfoOEMStringsDefFree(virSysinfoOEMStringsDefPtr def) +{ + size_t i; + + if (def == NULL) + return; + + for (i = 0; i < def->nvalues; i++) + VIR_FREE(def->values[i]); + VIR_FREE(def->values); +} + /** * virSysinfoDefFree: * @def: a sysinfo structure @@ -157,6 +169,8 @@ void virSysinfoDefFree(virSysinfoDefPtr def) } VIR_FREE(def->memory); + virSysinfoOEMStringsDefFree(def->oemStrings); + VIR_FREE(def); } @@ -1294,6 +1308,24 @@ virSysinfoMemoryFormat(virBufferPtr buf, virSysinfoDefPtr def) } } +static void +virSysinfoOEMStringsFormat(virBufferPtr buf, virSysinfoOEMStringsDefPtr def) +{ + size_t i; + + if (!def) + return; + + virBufferAddLit(buf, "<oemStrings>\n"); + virBufferAdjustIndent(buf, 2); + for (i = 0; i < def->nvalues; i++) { + virBufferEscapeString(buf, "<entry>%s</entry>\n", + def->values[i]); + } + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</oemStrings>\n"); +} + /** * virSysinfoFormat: * @buf: buffer to append output to (may use auto-indentation) @@ -1324,6 +1356,7 @@ virSysinfoFormat(virBufferPtr buf, virSysinfoDefPtr def) virSysinfoBaseBoardFormat(&childrenBuf, def->baseBoard, def->nbaseBoard); virSysinfoProcessorFormat(&childrenBuf, def); virSysinfoMemoryFormat(&childrenBuf, def); + virSysinfoOEMStringsFormat(&childrenBuf, def->oemStrings); virBufferAsprintf(buf, "<sysinfo type='%s'", type); if (virBufferUse(&childrenBuf)) { diff --git a/src/util/virsysinfo.h b/src/util/virsysinfo.h index 1e51d2cafa..ecb3a36eb8 100644 --- a/src/util/virsysinfo.h +++ b/src/util/virsysinfo.h @@ -98,6 +98,13 @@ struct _virSysinfoBaseBoardDef { /* XXX board type */ }; +typedef struct _virSysinfoOEMStringsDef virSysinfoOEMStringsDef; +typedef virSysinfoOEMStringsDef *virSysinfoOEMStringsDefPtr; +struct _virSysinfoOEMStringsDef { + size_t nvalues; + char **values; +}; + typedef struct _virSysinfoDef virSysinfoDef; typedef virSysinfoDef *virSysinfoDefPtr; struct _virSysinfoDef { @@ -114,6 +121,8 @@ struct _virSysinfoDef { size_t nmemory; virSysinfoMemoryDefPtr memory; + + virSysinfoOEMStringsDefPtr oemStrings; }; virSysinfoDefPtr virSysinfoRead(void); @@ -121,6 +130,7 @@ virSysinfoDefPtr virSysinfoRead(void); void virSysinfoBIOSDefFree(virSysinfoBIOSDefPtr def); void virSysinfoSystemDefFree(virSysinfoSystemDefPtr def); void virSysinfoBaseBoardDefClear(virSysinfoBaseBoardDefPtr def); +void virSysinfoOEMStringsDefFree(virSysinfoOEMStringsDefPtr def); void virSysinfoDefFree(virSysinfoDefPtr def); int virSysinfoFormat(virBufferPtr buf, virSysinfoDefPtr def) -- 2.14.3

On 01/17/2018 12:37 PM, Daniel P. Berrange wrote:
The OEM strings table in SMBIOS allows the vendor to pass arbitrary strings into the guest OS. This can be used as a way to pass data to an application like cloud-init, or potentially as an alternative to the kernel command line for OS installers where you can't modify the install ISO image to change the kernel args.
As an example, consider if cloud-init and anaconda supported OEM strings you could use something like
<oemStrings> <entry>cloud-init:ds=nocloud-net;s=http://10.10.0.1:8000/</entry> <entry>anaconda:method=http://dl.fedoraproject.org/pub/fedora/linux/releases/25/x86_64/os</entry> </oemStrings>
use of a application specific prefix as illustrated above is recommended, but not mandated, so that an app can reliably identify which of the many OEM strings are targetted at it.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- docs/formatdomain.html.in | 13 ++++++++++++ docs/schemas/domaincommon.rng | 9 +++++++++ src/conf/domain_conf.c | 47 +++++++++++++++++++++++++++++++++++++++++++ src/util/virsysinfo.c | 33 ++++++++++++++++++++++++++++++ src/util/virsysinfo.h | 10 +++++++++ 5 files changed, 112 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index d272cc1ba6..6af2d26209 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -411,6 +411,10 @@ <entry name='version'>0B98401 Pro</entry> <entry name='serial'>W1KS427111E</entry> </baseBoard> + <oemStrings> + <entry>myappname:some arbitrary data</entry> + <entry>otherappname:more arbitrary data</entry> + </oemStrings> </sysinfo> ...</pre>
@@ -498,6 +502,15 @@ validation and <code>date</code> format checking, all values are passed as strings to the hypervisor driver. </dd> + <dt><code>oemStrings</code></dt> + <dd> + This is block 11 of SMBIOS. This element should appear once and + can have multiple <code>entry</code> child elements, each providing + arbitrary string data. There are no restrictions on what data can + be provided in the entries, however, if the data is intended to be
s/, however/; however
+ consumed by an application in the guest, it is recommended to use + the application name as a prefix in the string.
Add the <span class="since">Since 4.1.0</span>
+ </dd> </dl> </dd> </dl> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index f22c932f6c..a154b5a462 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4857,6 +4857,15 @@ </oneOrMore> </element> </zeroOrMore> + <optional> + <element name="oemStrings"> + <oneOrMore> + <element name="entry"> + <ref name="sysinfo-value"/> + </element> + </oneOrMore> + </element> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a1c25060f9..6c0ad1ed7d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14461,6 +14461,42 @@ virSysinfoBaseBoardParseXML(xmlXPathContextPtr ctxt, return ret; }
We've been doing the 2 blank lines between functions more recently - although it's not officially in the hacking doc.
+static int +virSysinfoOEMStringsParseXML(xmlNodePtr node,
@node is not used in this context so the compiler tells me ;-)
+ xmlXPathContextPtr ctxt, + virSysinfoOEMStringsDefPtr *oem) +{ + int ret = -1; + virSysinfoOEMStringsDefPtr def; + xmlNodePtr *strings = NULL; + int nstrings; + size_t i; + + nstrings = virXPathNodeSet("./entry", ctxt, &strings); + if (nstrings < 0) + return -1; + if (nstrings == 0) + return 0; + + if (VIR_ALLOC(def) < 0) + goto cleanup; + + if (VIR_ALLOC_N(def->values, nstrings) < 0) + goto cleanup; + + def->nvalues = nstrings; + for (i = 0; i < nstrings; i++) + def->values[i] = virXMLNodeContentString(strings[i]); + + *oem = def; + def = NULL; + ret = 0; + cleanup: + VIR_FREE(strings); + virSysinfoOEMStringsDefFree(def);
Coverity notes @def is leaked... [1]...
+ return ret; +} + static virSysinfoDefPtr virSysinfoParseXML(xmlNodePtr node, xmlXPathContextPtr ctxt, @@ -14519,6 +14555,17 @@ virSysinfoParseXML(xmlNodePtr node, if (virSysinfoBaseBoardParseXML(ctxt, &def->baseBoard, &def->nbaseBoard) < 0) goto error;
+ /* Extract system related metadata */ + if ((tmpnode = virXPathNode("./oemStrings[1]", ctxt)) != NULL) { + oldnode = ctxt->node; + ctxt->node = tmpnode; + if (virSysinfoOEMStringsParseXML(tmpnode, ctxt, &def->oemStrings) < 0) { + ctxt->node = oldnode; + goto error; + } + ctxt->node = oldnode; + } + cleanup: VIR_FREE(type); return def; diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 1fbdd778f9..60765c38de 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -108,6 +108,18 @@ void virSysinfoBaseBoardDefClear(virSysinfoBaseBoardDefPtr def) VIR_FREE(def->location); }
+void virSysinfoOEMStringsDefFree(virSysinfoOEMStringsDefPtr def) +{ + size_t i; + + if (def == NULL) + return; + + for (i = 0; i < def->nvalues; i++) + VIR_FREE(def->values[i]); + VIR_FREE(def->values);
[1] ... because @def is not VIR_FREE'd here. With the adjustments, Reviewed-by: John Ferlan <jferlan@redhat.com> John
+} + /** * virSysinfoDefFree: * @def: a sysinfo structure @@ -157,6 +169,8 @@ void virSysinfoDefFree(virSysinfoDefPtr def) } VIR_FREE(def->memory);
+ virSysinfoOEMStringsDefFree(def->oemStrings); + VIR_FREE(def); }
@@ -1294,6 +1308,24 @@ virSysinfoMemoryFormat(virBufferPtr buf, virSysinfoDefPtr def) } }
+static void +virSysinfoOEMStringsFormat(virBufferPtr buf, virSysinfoOEMStringsDefPtr def) +{ + size_t i; + + if (!def) + return; + + virBufferAddLit(buf, "<oemStrings>\n"); + virBufferAdjustIndent(buf, 2); + for (i = 0; i < def->nvalues; i++) { + virBufferEscapeString(buf, "<entry>%s</entry>\n", + def->values[i]); + } + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</oemStrings>\n"); +} + /** * virSysinfoFormat: * @buf: buffer to append output to (may use auto-indentation) @@ -1324,6 +1356,7 @@ virSysinfoFormat(virBufferPtr buf, virSysinfoDefPtr def) virSysinfoBaseBoardFormat(&childrenBuf, def->baseBoard, def->nbaseBoard); virSysinfoProcessorFormat(&childrenBuf, def); virSysinfoMemoryFormat(&childrenBuf, def); + virSysinfoOEMStringsFormat(&childrenBuf, def->oemStrings);
virBufferAsprintf(buf, "<sysinfo type='%s'", type); if (virBufferUse(&childrenBuf)) { diff --git a/src/util/virsysinfo.h b/src/util/virsysinfo.h index 1e51d2cafa..ecb3a36eb8 100644 --- a/src/util/virsysinfo.h +++ b/src/util/virsysinfo.h @@ -98,6 +98,13 @@ struct _virSysinfoBaseBoardDef { /* XXX board type */ };
+typedef struct _virSysinfoOEMStringsDef virSysinfoOEMStringsDef; +typedef virSysinfoOEMStringsDef *virSysinfoOEMStringsDefPtr; +struct _virSysinfoOEMStringsDef { + size_t nvalues; + char **values; +}; + typedef struct _virSysinfoDef virSysinfoDef; typedef virSysinfoDef *virSysinfoDefPtr; struct _virSysinfoDef { @@ -114,6 +121,8 @@ struct _virSysinfoDef {
size_t nmemory; virSysinfoMemoryDefPtr memory; + + virSysinfoOEMStringsDefPtr oemStrings; };
virSysinfoDefPtr virSysinfoRead(void); @@ -121,6 +130,7 @@ virSysinfoDefPtr virSysinfoRead(void); void virSysinfoBIOSDefFree(virSysinfoBIOSDefPtr def); void virSysinfoSystemDefFree(virSysinfoSystemDefPtr def); void virSysinfoBaseBoardDefClear(virSysinfoBaseBoardDefPtr def); +void virSysinfoOEMStringsDefFree(virSysinfoOEMStringsDefPtr def); void virSysinfoDefFree(virSysinfoDefPtr def);
int virSysinfoFormat(virBufferPtr buf, virSysinfoDefPtr def)

On Thu, Jan 25, 2018 at 09:03:38AM -0500, John Ferlan wrote:
On 01/17/2018 12:37 PM, Daniel P. Berrange wrote:
The OEM strings table in SMBIOS allows the vendor to pass arbitrary strings into the guest OS. This can be used as a way to pass data to an application like cloud-init, or potentially as an alternative to the kernel command line for OS installers where you can't modify the install ISO image to change the kernel args.
As an example, consider if cloud-init and anaconda supported OEM strings you could use something like
<oemStrings> <entry>cloud-init:ds=nocloud-net;s=http://10.10.0.1:8000/</entry> <entry>anaconda:method=http://dl.fedoraproject.org/pub/fedora/linux/releases/25/x86_64/os</entry> </oemStrings>
use of a application specific prefix as illustrated above is recommended, but not mandated, so that an app can reliably identify which of the many OEM strings are targetted at it.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- docs/formatdomain.html.in | 13 ++++++++++++ docs/schemas/domaincommon.rng | 9 +++++++++ src/conf/domain_conf.c | 47 +++++++++++++++++++++++++++++++++++++++++++ src/util/virsysinfo.c | 33 ++++++++++++++++++++++++++++++ src/util/virsysinfo.h | 10 +++++++++ 5 files changed, 112 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index d272cc1ba6..6af2d26209 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -411,6 +411,10 @@ <entry name='version'>0B98401 Pro</entry> <entry name='serial'>W1KS427111E</entry> </baseBoard> + <oemStrings> + <entry>myappname:some arbitrary data</entry> + <entry>otherappname:more arbitrary data</entry> + </oemStrings> </sysinfo> ...</pre>
@@ -498,6 +502,15 @@ validation and <code>date</code> format checking, all values are passed as strings to the hypervisor driver. </dd> + <dt><code>oemStrings</code></dt> + <dd> + This is block 11 of SMBIOS. This element should appear once and + can have multiple <code>entry</code> child elements, each providing + arbitrary string data. There are no restrictions on what data can + be provided in the entries, however, if the data is intended to be
s/, however/; however
Using a ; instead of , before "however" is rather wierd / unusual. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On 01/25/2018 09:26 AM, Daniel P. Berrangé wrote:
On Thu, Jan 25, 2018 at 09:03:38AM -0500, John Ferlan wrote:
On 01/17/2018 12:37 PM, Daniel P. Berrange wrote:
The OEM strings table in SMBIOS allows the vendor to pass arbitrary strings into the guest OS. This can be used as a way to pass data to an application like cloud-init, or potentially as an alternative to the kernel command line for OS installers where you can't modify the install ISO image to change the kernel args.
As an example, consider if cloud-init and anaconda supported OEM strings you could use something like
<oemStrings> <entry>cloud-init:ds=nocloud-net;s=http://10.10.0.1:8000/</entry> <entry>anaconda:method=http://dl.fedoraproject.org/pub/fedora/linux/releases/25/x86_64/os</entry> </oemStrings>
use of a application specific prefix as illustrated above is recommended, but not mandated, so that an app can reliably identify which of the many OEM strings are targetted at it.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- docs/formatdomain.html.in | 13 ++++++++++++ docs/schemas/domaincommon.rng | 9 +++++++++ src/conf/domain_conf.c | 47 +++++++++++++++++++++++++++++++++++++++++++ src/util/virsysinfo.c | 33 ++++++++++++++++++++++++++++++ src/util/virsysinfo.h | 10 +++++++++ 5 files changed, 112 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index d272cc1ba6..6af2d26209 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -411,6 +411,10 @@ <entry name='version'>0B98401 Pro</entry> <entry name='serial'>W1KS427111E</entry> </baseBoard> + <oemStrings> + <entry>myappname:some arbitrary data</entry> + <entry>otherappname:more arbitrary data</entry> + </oemStrings> </sysinfo> ...</pre>
@@ -498,6 +502,15 @@ validation and <code>date</code> format checking, all values are passed as strings to the hypervisor driver. </dd> + <dt><code>oemStrings</code></dt> + <dd> + This is block 11 of SMBIOS. This element should appear once and + can have multiple <code>entry</code> child elements, each providing + arbitrary string data. There are no restrictions on what data can + be provided in the entries, however, if the data is intended to be
s/, however/; however
Using a ; instead of , before "however" is rather wierd / unusual.
Regards, Daniel
Strange - that's the way I've been taught when joining compound sentences. Even Google I think agrees with usage of the semi-colon in this case. Consider the following: If this, however, was some other way to describe the usage, then using a comma would be right; however, you're joining two sentences and using it a compound sentence. In the long run, I don't really care that much - I'm not an english/grammar major ;-) John

This wires up the previously added OEM strings XML schema to be able to generate comamnd line args for QEMU. This requires QEMU >= 2.12 release containing this patch: commit 2d6dcbf93fb01b4a7f45a93d276d4d74b16392dd Author: Daniel P. Berrange <berrange@redhat.com> Date: Sat Oct 28 21:51:36 2017 +0100 smbios: support setting OEM strings table Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_command.c | 28 ++++++++++++++++++++++++++++ tests/qemuxml2argvdata/smbios.args | 2 ++ tests/qemuxml2argvdata/smbios.xml | 5 +++++ tests/qemuxml2xmloutdata/smbios.xml | 5 +++++ 4 files changed, 40 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b8aede32d2..96bd0ad8ee 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6142,6 +6142,26 @@ qemuBuildSmbiosBaseBoardStr(virSysinfoBaseBoardDefPtr def) } +static char * +qemuBuildSmbiosOEMStringsStr(virSysinfoOEMStringsDefPtr def) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + size_t i; + + if (!def) + return NULL; + + virBufferAddLit(&buf, "type=11"); + + for (i = 0; i < def->nvalues; i++) { + virBufferAddLit(&buf, ",value="); + virQEMUBuildBufferEscapeComma(&buf, def->values[i]); + } + + return virBufferContentAndReset(&buf); +} + + static int qemuBuildSmbiosCommandLine(virCommandPtr cmd, virQEMUDriverPtr driver, @@ -6212,6 +6232,14 @@ qemuBuildSmbiosCommandLine(virCommandPtr cmd, virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL); VIR_FREE(smbioscmd); } + + if (source->oemStrings) { + if (!(smbioscmd = qemuBuildSmbiosOEMStringsStr(source->oemStrings))) + return -1; + + virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL); + VIR_FREE(smbioscmd); + } } return 0; diff --git a/tests/qemuxml2argvdata/smbios.args b/tests/qemuxml2argvdata/smbios.args index 3d94a109f9..d27d436a3a 100644 --- a/tests/qemuxml2argvdata/smbios.args +++ b/tests/qemuxml2argvdata/smbios.args @@ -17,6 +17,8 @@ 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' \ +-smbios 'type=11,value=Hello,value=World,value=This is,,\ + more tricky value=escaped' \ -nographic \ -nodefaults \ -chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ diff --git a/tests/qemuxml2argvdata/smbios.xml b/tests/qemuxml2argvdata/smbios.xml index c12477dcb5..319bdf61df 100644 --- a/tests/qemuxml2argvdata/smbios.xml +++ b/tests/qemuxml2argvdata/smbios.xml @@ -26,6 +26,11 @@ <entry name='asset'>CZC1065993</entry> <entry name='location'>Upside down</entry> </baseBoard> + <oemStrings> + <entry>Hello</entry> + <entry>World</entry> + <entry>This is, more tricky value=escaped</entry> + </oemStrings> </sysinfo> <os> <type arch='i686' machine='pc'>hvm</type> diff --git a/tests/qemuxml2xmloutdata/smbios.xml b/tests/qemuxml2xmloutdata/smbios.xml index d21f6275f0..cbe616c7da 100644 --- a/tests/qemuxml2xmloutdata/smbios.xml +++ b/tests/qemuxml2xmloutdata/smbios.xml @@ -26,6 +26,11 @@ <entry name='asset'>CZC1065993</entry> <entry name='location'>Upside down</entry> </baseBoard> + <oemStrings> + <entry>Hello</entry> + <entry>World</entry> + <entry>This is, more tricky value=escaped</entry> + </oemStrings> </sysinfo> <os> <type arch='i686' machine='pc'>hvm</type> -- 2.14.3

On 01/17/2018 12:37 PM, Daniel P. Berrange wrote:
This wires up the previously added OEM strings XML schema to be able to generate comamnd line args for QEMU. This requires QEMU >= 2.12 release containing this patch:
commit 2d6dcbf93fb01b4a7f45a93d276d4d74b16392dd Author: Daniel P. Berrange <berrange@redhat.com> Date: Sat Oct 28 21:51:36 2017 +0100
smbios: support setting OEM strings table
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_command.c | 28 ++++++++++++++++++++++++++++ tests/qemuxml2argvdata/smbios.args | 2 ++ tests/qemuxml2argvdata/smbios.xml | 5 +++++ tests/qemuxml2xmloutdata/smbios.xml | 5 +++++ 4 files changed, 40 insertions(+)
Would a QEMU < 2.12 cause a failure if the 'type=11' OEM strings were provided or do we need to add a capability in virQEMUCapsInitQMPMonitor based on version? What's here looks good and the provisional Reviewed-by: John Ferlan <jferlan@redhat.com> if we don't need the capability. If we need it, then probably need to update the patch. John

On Thu, Jan 25, 2018 at 09:04:05AM -0500, John Ferlan wrote:
On 01/17/2018 12:37 PM, Daniel P. Berrange wrote:
This wires up the previously added OEM strings XML schema to be able to generate comamnd line args for QEMU. This requires QEMU >= 2.12 release containing this patch:
commit 2d6dcbf93fb01b4a7f45a93d276d4d74b16392dd Author: Daniel P. Berrange <berrange@redhat.com> Date: Sat Oct 28 21:51:36 2017 +0100
smbios: support setting OEM strings table
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_command.c | 28 ++++++++++++++++++++++++++++ tests/qemuxml2argvdata/smbios.args | 2 ++ tests/qemuxml2argvdata/smbios.xml | 5 +++++ tests/qemuxml2xmloutdata/smbios.xml | 5 +++++ 4 files changed, 40 insertions(+)
Would a QEMU < 2.12 cause a failure if the 'type=11' OEM strings were provided or do we need to add a capability in virQEMUCapsInitQMPMonitor based on version?
Yes it will $ qemu-system-x86_64 -smbios type=11 qemu-system-x86_64: -smbios type=11: Don't know how to build fields for SMBIOS type 11 Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (3)
-
Daniel P. Berrange
-
Daniel P. Berrangé
-
John Ferlan