[libvirt] [PATCH] qemu: add support for setting OEM strings SMBIOS data fields

Daniel P. Berrange posted 1 patch 6 years, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20171117164853.29166-1-berrange@redhat.com
docs/formatdomain.html.in                          | 13 ++++++
docs/schemas/domaincommon.rng                      |  9 ++++
src/conf/domain_conf.c                             | 53 ++++++++++++++++++++++
src/qemu/qemu_command.c                            | 28 ++++++++++++
src/util/virsysinfo.c                              | 33 ++++++++++++++
src/util/virsysinfo.h                              | 10 ++++
tests/qemuxml2argvdata/qemuxml2argv-smbios.args    |  2 +
tests/qemuxml2argvdata/qemuxml2argv-smbios.xml     |  5 ++
tests/qemuxml2xmloutdata/qemuxml2xmlout-smbios.xml |  5 ++
9 files changed, 158 insertions(+)
[libvirt] [PATCH] qemu: add support for setting OEM strings SMBIOS data fields
Posted by Daniel P. Berrange 6 years, 5 months ago
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 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>
---

NB, the QEMU side of this patch is queued but won't merge until 2.12 opens
up for dev work, so this libvirt patch will need to wait a little

 docs/formatdomain.html.in                          | 13 ++++++
 docs/schemas/domaincommon.rng                      |  9 ++++
 src/conf/domain_conf.c                             | 53 ++++++++++++++++++++++
 src/qemu/qemu_command.c                            | 28 ++++++++++++
 src/util/virsysinfo.c                              | 33 ++++++++++++++
 src/util/virsysinfo.h                              | 10 ++++
 tests/qemuxml2argvdata/qemuxml2argv-smbios.args    |  2 +
 tests/qemuxml2argvdata/qemuxml2argv-smbios.xml     |  5 ++
 tests/qemuxml2xmloutdata/qemuxml2xmlout-smbios.xml |  5 ++
 9 files changed, 158 insertions(+)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 8dbea6af71..bd7c542173 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -411,6 +411,10 @@
     &lt;entry name='version'&gt;0B98401 Pro&lt;/entry&gt;
     &lt;entry name='serial'&gt;W1KS427111E&lt;/entry&gt;
   &lt;/baseBoard&gt;
+  &lt;oemStrings&gt;
+    &lt;entry&gt;myappname:some arbitrary data&lt;/entry&gt;
+    &lt;entry&gt;otherappname:more arbitrary data&lt;/entry&gt;
+  &lt;/oemStrings&gt;
 &lt;/sysinfo&gt;
 ...</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 82fdfd5f73..98de7d2cb1 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4834,6 +4834,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 0e4f76f066..5d81fbb555 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14315,6 +14315,48 @@ 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;
+
+    if (!virXMLNodeNameEqual(node, "oemStrings")) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("XML does not contain expected 'system' element"));
+        return ret;
+    }
+
+    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,
@@ -14373,6 +14415,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/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 56729e4981..01fb8dff6e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6063,6 +6063,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,
@@ -6133,6 +6153,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/src/util/virsysinfo.c b/src/util/virsysinfo.c
index ab81b1f7f0..0ad3e6c70d 100644
--- a/src/util/virsysinfo.c
+++ b/src/util/virsysinfo.c
@@ -106,6 +106,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
@@ -155,6 +167,8 @@ void virSysinfoDefFree(virSysinfoDefPtr def)
     }
     VIR_FREE(def->memory);
 
+    virSysinfoOEMStringsDefFree(def->oemStrings);
+
     VIR_FREE(def);
 }
 
@@ -1263,6 +1277,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)
@@ -1293,6 +1325,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)
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smbios.args b/tests/qemuxml2argvdata/qemuxml2argv-smbios.args
index 3d94a109f9..d27d436a3a 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-smbios.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-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/qemuxml2argv-smbios.xml b/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml
index c12477dcb5..319bdf61df 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-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/qemuxml2xmlout-smbios.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smbios.xml
index d21f6275f0..cbe616c7da 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-smbios.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: add support for setting OEM strings SMBIOS data fields
Posted by Ján Tomko 6 years, 4 months ago
On Fri, Nov 17, 2017 at 04:48:53PM +0000, 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 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>
>---
>
>NB, the QEMU side of this patch is queued but won't merge until 2.12 opens
>up for dev work, so this libvirt patch will need to wait a little
>

Let me guess, there will be no way to tell whether QEMU supports this
option or not...

> docs/formatdomain.html.in                          | 13 ++++++
> docs/schemas/domaincommon.rng                      |  9 ++++
> src/conf/domain_conf.c                             | 53 ++++++++++++++++++++++
> src/qemu/qemu_command.c                            | 28 ++++++++++++
> src/util/virsysinfo.c                              | 33 ++++++++++++++
> src/util/virsysinfo.h                              | 10 ++++
> tests/qemuxml2argvdata/qemuxml2argv-smbios.args    |  2 +
> tests/qemuxml2argvdata/qemuxml2argv-smbios.xml     |  5 ++
> tests/qemuxml2xmloutdata/qemuxml2xmlout-smbios.xml |  5 ++
> 9 files changed, 158 insertions(+)
>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index 0e4f76f066..5d81fbb555 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -14315,6 +14315,48 @@ 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;
>+
>+    if (!virXMLNodeNameEqual(node, "oemStrings")) {
>+        virReportError(VIR_ERR_XML_ERROR, "%s",
>+                       _("XML does not contain expected 'system' element"));

This copy-and-paste error message with the wrong element name should not
happen, since we only call this function if we found an 'oemStrings'
node.

>+        return ret;
>+    }
>+
>+    nstrings = virXPathNodeSet("./entry", ctxt, &strings);
>+    if (nstrings < 0)
>+        return -1;
>+    if (nstrings == 0)
>+        return 0;
>+
>+    if (VIR_ALLOC(def) < 0)
>+        goto cleanup;
>+

ACK

Would look better with the XML changes and command line formatter
changes separated

Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: add support for setting OEM strings SMBIOS data fields
Posted by Daniel P. Berrange 6 years, 4 months ago
On Thu, Nov 23, 2017 at 05:42:32PM +0100, Ján Tomko wrote:
> On Fri, Nov 17, 2017 at 04:48:53PM +0000, 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 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>
> > ---
> > 
> > NB, the QEMU side of this patch is queued but won't merge until 2.12 opens
> > up for dev work, so this libvirt patch will need to wait a little
> > 
> 
> Let me guess, there will be no way to tell whether QEMU supports this
> option or not...

Indeed, it is not visible in QMP schema, nor command line help, as this
falls in one of the gaps in QEMU's capabilities reporting.

> 
> > docs/formatdomain.html.in                          | 13 ++++++
> > docs/schemas/domaincommon.rng                      |  9 ++++
> > src/conf/domain_conf.c                             | 53 ++++++++++++++++++++++
> > src/qemu/qemu_command.c                            | 28 ++++++++++++
> > src/util/virsysinfo.c                              | 33 ++++++++++++++
> > src/util/virsysinfo.h                              | 10 ++++
> > tests/qemuxml2argvdata/qemuxml2argv-smbios.args    |  2 +
> > tests/qemuxml2argvdata/qemuxml2argv-smbios.xml     |  5 ++
> > tests/qemuxml2xmloutdata/qemuxml2xmlout-smbios.xml |  5 ++
> > 9 files changed, 158 insertions(+)
> > 
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 0e4f76f066..5d81fbb555 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -14315,6 +14315,48 @@ 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;
> > +
> > +    if (!virXMLNodeNameEqual(node, "oemStrings")) {
> > +        virReportError(VIR_ERR_XML_ERROR, "%s",
> > +                       _("XML does not contain expected 'system' element"));
> 
> This copy-and-paste error message with the wrong element name should not
> happen, since we only call this function if we found an 'oemStrings'
> node.
> 
> > +        return ret;
> > +    }
> > +
> > +    nstrings = virXPathNodeSet("./entry", ctxt, &strings);
> > +    if (nstrings < 0)
> > +        return -1;
> > +    if (nstrings == 0)
> > +        return 0;
> > +
> > +    if (VIR_ALLOC(def) < 0)
> > +        goto cleanup;
> > +
> 
> ACK
> 
> Would look better with the XML changes and command line formatter
> changes separated
> 
> Jan



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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list