[libvirt] [PATCH v2 1/2] conf: add support for setting OEM strings SMBIOS data fields

John Ferlan jferlan at redhat.com
Thu Jan 25 14:03:38 UTC 2018



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 at 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 at 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)
> 




More information about the libvir-list mailing list