diff mbox series

[2/3] xl/libxl: Add ability to specify SMBIOS strings

Message ID 20220810194827.103428-3-jandryuk@gmail.com (mailing list archive)
State Superseded
Headers show
Series libxl smbios support | expand

Commit Message

Jason Andryuk Aug. 10, 2022, 7:48 p.m. UTC
hvm_xs_strings.h specifies xenstore entries which can be used to set or
override smbios strings.  hvmloader has support for reading them, but
xl/libxl support is not wired up.

Allow specifying the strings with the new xl.cfg option:
smbios=["bios_vendor=Xen Project","system_version=1.0"]

In terms of strings, the SMBIOS specification 3.5 says:
https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.5.0.pdf
"""
Strings must be encoded as UTF-8 with no byte order mark (BOM). For
compatibility with older SMBIOS parsers, US-ASCII characters should be
used.  NOTE There is no limit on the length of each individual text
string. However, the length of the entire structure table (including all
strings) must be reported in the Structure Table Length field of the
32-bit Structure Table Entry Point (see 5.2.1) and/or the Structure
Table Maximum Size field of the 64-bit Structure Table Entry Point (see
5.2.2).
"""

The strings aren't checked for utf-8 or length.  hvmloader has a sanity
check on the overall length.

The libxl_smbios_type enum starts at 1 since otherwise the 0th key is
not printed in the json output.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
The rendered man page and html don't have a newline at then end of the
new section.
"""
           battery_device_name=STRING
       ms_vm_genid="OPTION"
"""

however the txt format is correct:
"""
        battery_device_name=STRING

    ms_vm_genid="OPTION"
"""

I'm at a loss as to why this is happening.
---
 docs/man/xl.cfg.5.pod.in             | 45 ++++++++++++++++++++++++
 tools/golang/xenlight/helpers.gen.go | 51 ++++++++++++++++++++++++++++
 tools/golang/xenlight/types.gen.go   | 27 +++++++++++++++
 tools/include/libxl.h                |  5 +++
 tools/libs/light/libxl_dom.c         | 20 +++++++++++
 tools/libs/light/libxl_types.idl     | 26 ++++++++++++++
 tools/xl/xl_parse.c                  | 44 +++++++++++++++++++++++-
 7 files changed, 217 insertions(+), 1 deletion(-)

Comments

Anthony PERARD Sept. 8, 2022, 12:40 p.m. UTC | #1
On Wed, Aug 10, 2022 at 03:48:26PM -0400, Jason Andryuk wrote:
> hvm_xs_strings.h specifies xenstore entries which can be used to set or
> override smbios strings.  hvmloader has support for reading them, but
> xl/libxl support is not wired up.
> 
> Allow specifying the strings with the new xl.cfg option:
> smbios=["bios_vendor=Xen Project","system_version=1.0"]
> 
> In terms of strings, the SMBIOS specification 3.5 says:
> https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.5.0.pdf
> """
> Strings must be encoded as UTF-8 with no byte order mark (BOM). For
> compatibility with older SMBIOS parsers, US-ASCII characters should be
> used.  NOTE There is no limit on the length of each individual text
> string. However, the length of the entire structure table (including all
> strings) must be reported in the Structure Table Length field of the
> 32-bit Structure Table Entry Point (see 5.2.1) and/or the Structure
> Table Maximum Size field of the 64-bit Structure Table Entry Point (see
> 5.2.2).
> """
> 
> The strings aren't checked for utf-8 or length.  hvmloader has a sanity
> check on the overall length.
> 
> The libxl_smbios_type enum starts at 1 since otherwise the 0th key is
> not printed in the json output.
> 
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> ---
> The rendered man page and html don't have a newline at then end of the
> new section.
> """
>            battery_device_name=STRING
>        ms_vm_genid="OPTION"
> """
> 
> however the txt format is correct:
> """
>         battery_device_name=STRING
> 
>     ms_vm_genid="OPTION"
> """
> 
> I'm at a loss as to why this is happening.

Maybe it's because =back just cancel =over, and pod2man just keeps going
as if it's the same list. Adding some text for the =item or after =back
would help, but that's done in the next patch, so I guess we can live
with that for one commit.

> ---
> diff --git a/tools/libs/light/libxl_dom.c b/tools/libs/light/libxl_dom.c
> index 2abaab439c..9034933ea8 100644
> --- a/tools/libs/light/libxl_dom.c
> +++ b/tools/libs/light/libxl_dom.c
> @@ -771,6 +771,26 @@ static int hvm_build_set_xs_values(libxl__gc *gc,
>              goto err;
>      }
>  
> +    for (int i = 0; i < info->u.hvm.num_smbios; i++) {
> +        char *p;
> +        path = GCSPRINTF("/local/domain/%d/"HVM_XS_BIOS_STRINGS"/%s", domid,
> +                   libxl_smbios_type_to_string(info->u.hvm.smbios[i].key));
> +
> +        /* libxl defines are all "_", but the HVM_XS_ strings are "-". */

"libxl defines are all "_"" seems a bit weird to me as a comment in
the source code, maybe a better comment would the conversion we need to
do, something like:

    Convert libxl_smbios_type string to xenstore path that hvmloader
    will use, as defined by HVM_XS_*. That is convert the '_' to '-'.

> +        p = strrchr(path, '/');
> +        for ( ; *p; p++) {
> +            if (*p == '_')
> +                *p = '-';
> +        }
> +
> +        LOGD(DEBUG, domid, "Writing %d %s %s\n", i, path,

I don't think printing the value of i would be useful here. Also adding
a '=' before the value and putting the value between double-quote would
be a bit better.

> +             info->u.hvm.smbios[i].value);
> +        ret = libxl__xs_printf(gc, XBT_NULL, path, "%s",
> +                               info->u.hvm.smbios[i].value);
> +        if (ret)
> +            goto err;
> +    }
> +
>      /* Only one module can be passed. PVHv2 guests do not support this. */
>      if (dom->acpi_modules[0].guest_addr_out && 
>          info->type == LIBXL_DOMAIN_TYPE_HVM) {
> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> index 1b5381cef0..4f3f962773 100644
> --- a/tools/xl/xl_parse.c
> +++ b/tools/xl/xl_parse.c
> @@ -1220,8 +1220,9 @@ void parse_config_data(const char *config_source,
>      XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids, *vtpms,
>                     *usbctrls, *usbdevs, *p9devs, *vdispls, *pvcallsifs_devs;
>      XLU_ConfigList *channels, *ioports, *irqs, *iomem, *viridian, *dtdevs,
> -                   *mca_caps;
> +                   *mca_caps, *smbios;
>      int num_ioports, num_irqs, num_iomem, num_cpus, num_viridian, num_mca_caps;
> +    int num_smbios;
>      int pci_power_mgmt = 0;
>      int pci_msitranslate = 0;
>      int pci_permissive = 0;
> @@ -1783,6 +1784,47 @@ void parse_config_data(const char *config_source,
>          xlu_cfg_replace_string(config, "acpi_firmware",
>                                 &b_info->u.hvm.acpi_firmware, 0);
>  
> +        switch (xlu_cfg_get_list(config, "smbios", &smbios, &num_smbios, 0))
> +        {
> +        case 0: /* Success */
> +            b_info->u.hvm.num_smbios = num_smbios;
> +            b_info->u.hvm.smbios = xcalloc(num_smbios, sizeof(libxl_smbios));
> +            for (i = 0; i < num_smbios; i++) {
> +                char *option_untrimmed, *value_untrimmed;
> +                char *option, *value;
> +                libxl_smbios_type v;
> +
> +                buf = xlu_cfg_get_listitem(smbios, i);
> +                if (!buf) continue;

Isn't this an error? It seems that xlu_cfg_get_listitem would return 0
if 'i' is outside of the array or if the value isn't a string. I think
it would also mean that "smbios[i].key" and ".value" would have
uninitialized value and potentially garbage.

> +
> +                if (split_string_into_pair(buf, "=",
> +                                           &option_untrimmed,
> +                                           &value_untrimmed)) {
> +                    fprintf(stderr, "xl: failed to split \"%s\" into pair\n",
> +                            buf);
> +                    exit(EXIT_FAILURE);
> +                }
> +                trim(isspace, option_untrimmed, &option);
> +                trim(isspace, value_untrimmed, &value);

I think you could free "option_untrimmed" and "value_untrimmed", as
"option" and "value" are newly allocated strings. Also, "option" won't
be needed after it's been converted to an enum value.
> +
> +                e = libxl_smbios_type_from_string(option, &v);
> +                if (e) {
> +                    fprintf(stderr,
> +                            "xl: unknown smbios type '%s'\n",
> +                            buf);
> +                    exit(-ERROR_FAIL);
> +                }
> +
> +                b_info->u.hvm.smbios[i].key = v;
> +                b_info->u.hvm.smbios[i].value = value;
> +            }
> +            break;
> +        case ESRCH: break; /* Option not present */

Could you move the "break" to a new line? It will make it easier to read
that ESRCH is just ignore instead of throwing an error.

> +        default:
> +            fprintf(stderr,"xl: Unable to parse smbios options.\n");
> +            exit(-ERROR_FAIL);
> +        }
> +
>          if (!xlu_cfg_get_string(config, "ms_vm_genid", &buf, 0)) {
>              if (!strcmp(buf, "generate")) {
>                  e = libxl_ms_vm_genid_generate(ctx, &b_info->u.hvm.ms_vm_genid);

Thanks,
Jason Andryuk Sept. 8, 2022, 4:35 p.m. UTC | #2
On Thu, Sep 8, 2022 at 8:40 AM Anthony PERARD <anthony.perard@citrix.com> wrote:

> > ---
> > diff --git a/tools/libs/light/libxl_dom.c b/tools/libs/light/libxl_dom.c
> > index 2abaab439c..9034933ea8 100644
> > --- a/tools/libs/light/libxl_dom.c
> > +++ b/tools/libs/light/libxl_dom.c
> > @@ -771,6 +771,26 @@ static int hvm_build_set_xs_values(libxl__gc *gc,
> >              goto err;
> >      }
> >
> > +    for (int i = 0; i < info->u.hvm.num_smbios; i++) {
> > +        char *p;
> > +        path = GCSPRINTF("/local/domain/%d/"HVM_XS_BIOS_STRINGS"/%s", domid,
> > +                   libxl_smbios_type_to_string(info->u.hvm.smbios[i].key));
> > +
> > +        /* libxl defines are all "_", but the HVM_XS_ strings are "-". */
>
> "libxl defines are all "_"" seems a bit weird to me as a comment in
> the source code, maybe a better comment would the conversion we need to
> do, something like:
>
>     Convert libxl_smbios_type string to xenstore path that hvmloader
>     will use, as defined by HVM_XS_*. That is convert the '_' to '-'.

I'll use this.  All your other comments sound good, and I'll change accordingly.

Thanks for taking a look.

-Jason
diff mbox series

Patch

diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index 6d98d73d76..7edf5d23f3 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -2061,6 +2061,51 @@  number of vendor defined SMBIOS structures (type 128 - 255). Since SMBIOS
 structures do not present their overall size, each entry in the file must be
 preceded by a 32b integer indicating the size of the following structure.
 
+=item B<smbios=[ "SMBIOS_SPEC_STRING", "SMBIOS_SPEC_STRING", ...]>
+
+Specifies the SMBIOS values to be provided to the guest.  These set or
+override specific entries in the tables provided to the guest.
+
+Each B<SMBIOS_SPEC_STRING> is a C<KEY=VALUE> string from the following list:
+
+=over 4
+
+=item B<bios_vendor=STRING>
+
+=item B<bios_version=STRING>
+
+=item B<system_manufacturer=STRING>
+
+=item B<system_product_name=STRING>
+
+=item B<system_version=STRING>
+
+=item B<system_serial_number=STRING>
+
+=item B<baseboard_manufacturer=STRING>
+
+=item B<baseboard_product_name=STRING>
+
+=item B<baseboard_version=STRING>
+
+=item B<baseboard_serial_number=STRING>
+
+=item B<baseboard_asset_tag=STRING>
+
+=item B<baseboard_location_in_chassis=STRING>
+
+=item B<enclosure_manufacturer=STRING>
+
+=item B<enclosure_serial_number=STRING>
+
+=item B<enclosure_asset_tag=STRING>
+
+=item B<battery_manufacturer=STRING>
+
+=item B<battery_device_name=STRING>
+
+=back
+
 =item B<ms_vm_genid="OPTION">
 
 Provide a VM generation ID to the guest.
diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go
index fa3cf2ab76..cae14ec6f5 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -589,6 +589,38 @@  xc.build_id = C.CString(x.BuildId)}
  return nil
  }
 
+// NewSmbios returns an instance of Smbios initialized with defaults.
+func NewSmbios() (*Smbios, error) {
+var (
+x Smbios
+xc C.libxl_smbios)
+
+C.libxl_smbios_init(&xc)
+defer C.libxl_smbios_dispose(&xc)
+
+if err := x.fromC(&xc); err != nil {
+return nil, err }
+
+return &x, nil}
+
+func (x *Smbios) fromC(xc *C.libxl_smbios) error {
+ x.Key = SmbiosType(xc.key)
+x.Value = C.GoString(xc.value)
+
+ return nil}
+
+func (x *Smbios) toC(xc *C.libxl_smbios) (err error){defer func(){
+if err != nil{
+C.libxl_smbios_dispose(xc)}
+}()
+
+xc.key = C.libxl_smbios_type(x.Key)
+if x.Value != "" {
+xc.value = C.CString(x.Value)}
+
+ return nil
+ }
+
 // NewDomainCreateInfo returns an instance of DomainCreateInfo initialized with defaults.
 func NewDomainCreateInfo() (*DomainCreateInfo, error) {
 var (
@@ -1189,6 +1221,15 @@  return fmt.Errorf("converting field Altp2M: %v", err)
 }
 x.SystemFirmware = C.GoString(tmp.system_firmware)
 x.SmbiosFirmware = C.GoString(tmp.smbios_firmware)
+x.Smbios = nil
+if n := int(tmp.num_smbios); n > 0 {
+cSmbios := (*[1<<28]C.libxl_smbios)(unsafe.Pointer(tmp.smbios))[:n:n]
+x.Smbios = make([]Smbios, n)
+for i, v := range cSmbios {
+if err := x.Smbios[i].fromC(&v); err != nil {
+return fmt.Errorf("converting field Smbios: %v", err) }
+}
+}
 x.AcpiFirmware = C.GoString(tmp.acpi_firmware)
 x.Hdtype = Hdtype(tmp.hdtype)
 if err := x.Nographic.fromC(&tmp.nographic);err != nil {
@@ -1501,6 +1542,16 @@  if tmp.SystemFirmware != "" {
 hvm.system_firmware = C.CString(tmp.SystemFirmware)}
 if tmp.SmbiosFirmware != "" {
 hvm.smbios_firmware = C.CString(tmp.SmbiosFirmware)}
+if numSmbios := len(tmp.Smbios); numSmbios > 0 {
+hvm.smbios = (*C.libxl_smbios)(C.malloc(C.ulong(numSmbios)*C.sizeof_libxl_smbios))
+hvm.num_smbios = C.int(numSmbios)
+cSmbios := (*[1<<28]C.libxl_smbios)(unsafe.Pointer(hvm.smbios))[:numSmbios:numSmbios]
+for i,v := range tmp.Smbios {
+if err := v.toC(&cSmbios[i]); err != nil {
+return fmt.Errorf("converting field Smbios: %v", err)
+}
+}
+}
 if tmp.AcpiFirmware != "" {
 hvm.acpi_firmware = C.CString(tmp.AcpiFirmware)}
 hvm.hdtype = C.libxl_hdtype(tmp.Hdtype)
diff --git a/tools/golang/xenlight/types.gen.go b/tools/golang/xenlight/types.gen.go
index a0be7ada8c..e084c3540b 100644
--- a/tools/golang/xenlight/types.gen.go
+++ b/tools/golang/xenlight/types.gen.go
@@ -386,6 +386,32 @@  Commandline string
 BuildId string
 }
 
+type SmbiosType int
+const(
+SmbiosTypeBiosVendor SmbiosType = 1
+SmbiosTypeBiosVersion SmbiosType = 2
+SmbiosTypeSystemManufacturer SmbiosType = 3
+SmbiosTypeSystemProductName SmbiosType = 4
+SmbiosTypeSystemVersion SmbiosType = 5
+SmbiosTypeSystemSerialNumber SmbiosType = 6
+SmbiosTypeBaseboardManufacturer SmbiosType = 7
+SmbiosTypeBaseboardProductName SmbiosType = 8
+SmbiosTypeBaseboardVersion SmbiosType = 9
+SmbiosTypeBaseboardSerialNumber SmbiosType = 10
+SmbiosTypeBaseboardAssetTag SmbiosType = 11
+SmbiosTypeBaseboardLocationInChassis SmbiosType = 12
+SmbiosTypeEnclosureManufacturer SmbiosType = 13
+SmbiosTypeEnclosureSerialNumber SmbiosType = 14
+SmbiosTypeEnclosureAssetTag SmbiosType = 15
+SmbiosTypeBatteryManufacturer SmbiosType = 16
+SmbiosTypeBatteryDeviceName SmbiosType = 17
+)
+
+type Smbios struct {
+Key SmbiosType
+Value string
+}
+
 type DomainCreateInfo struct {
 Type DomainType
 Hap Defbool
@@ -568,6 +594,7 @@  NestedHvm Defbool
 Altp2M Defbool
 SystemFirmware string
 SmbiosFirmware string
+Smbios []Smbios
 AcpiFirmware string
 Hdtype Hdtype
 Nographic Defbool
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index 2321a648a5..023613d35b 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -174,6 +174,11 @@ 
  */
 #define LIBXL_HAVE_BUILDINFO_HVM_MS_VM_GENID 1
 
+/*
+ * libxl_domain_build_info has the u.hvm.smbios field.
+ */
+#define LIBXL_HAVE_BUILDINFO_HVM_SMBIOS 1
+
 /*
  * LIBXL_HAVE_VCPUINFO_SOFT_AFFINITY indicates that a 'cpumap_soft'
  * field (of libxl_bitmap type) is present in libxl_vcpuinfo,
diff --git a/tools/libs/light/libxl_dom.c b/tools/libs/light/libxl_dom.c
index 2abaab439c..9034933ea8 100644
--- a/tools/libs/light/libxl_dom.c
+++ b/tools/libs/light/libxl_dom.c
@@ -771,6 +771,26 @@  static int hvm_build_set_xs_values(libxl__gc *gc,
             goto err;
     }
 
+    for (int i = 0; i < info->u.hvm.num_smbios; i++) {
+        char *p;
+        path = GCSPRINTF("/local/domain/%d/"HVM_XS_BIOS_STRINGS"/%s", domid,
+                   libxl_smbios_type_to_string(info->u.hvm.smbios[i].key));
+
+        /* libxl defines are all "_", but the HVM_XS_ strings are "-". */
+        p = strrchr(path, '/');
+        for ( ; *p; p++) {
+            if (*p == '_')
+                *p = '-';
+        }
+
+        LOGD(DEBUG, domid, "Writing %d %s %s\n", i, path,
+             info->u.hvm.smbios[i].value);
+        ret = libxl__xs_printf(gc, XBT_NULL, path, "%s",
+                               info->u.hvm.smbios[i].value);
+        if (ret)
+            goto err;
+    }
+
     /* Only one module can be passed. PVHv2 guests do not support this. */
     if (dom->acpi_modules[0].guest_addr_out && 
         info->type == LIBXL_DOMAIN_TYPE_HVM) {
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index d634f304cd..d04207748e 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -418,6 +418,31 @@  libxl_version_info = Struct("version_info", [
     ("build_id",          string),
     ], dir=DIR_OUT)
 
+libxl_smbios_type = Enumeration("smbios_type", [
+    (1,  "bios_vendor"),
+    (2,  "bios_version"),
+    (3,  "system_manufacturer"),
+    (4,  "system_product_name"),
+    (5,  "system_version"),
+    (6,  "system_serial_number"),
+    (7,  "baseboard_manufacturer"),
+    (8,  "baseboard_product_name"),
+    (9,  "baseboard_version"),
+    (10, "baseboard_serial_number"),
+    (11, "baseboard_asset_tag"),
+    (12, "baseboard_location_in_chassis"),
+    (13, "enclosure_manufacturer"),
+    (14, "enclosure_serial_number"),
+    (15, "enclosure_asset_tag"),
+    (16, "battery_manufacturer"),
+    (17, "battery_device_name"),
+    ])
+
+libxl_smbios = Struct("smbios", [
+    ("key",          libxl_smbios_type),
+    ("value",        string),
+    ], dir=DIR_IN)
+
 libxl_domain_create_info = Struct("domain_create_info",[
     ("type",         libxl_domain_type),
     ("hap",          libxl_defbool),
@@ -604,6 +629,7 @@  libxl_domain_build_info = Struct("domain_build_info",[
                                        ("altp2m",           libxl_defbool),
                                        ("system_firmware",  string),
                                        ("smbios_firmware",  string),
+                                       ("smbios",           Array(libxl_smbios, "num_smbios")),
                                        ("acpi_firmware",    string),
                                        ("hdtype",           libxl_hdtype),
                                        ("nographic",        libxl_defbool),
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 1b5381cef0..4f3f962773 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -1220,8 +1220,9 @@  void parse_config_data(const char *config_source,
     XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids, *vtpms,
                    *usbctrls, *usbdevs, *p9devs, *vdispls, *pvcallsifs_devs;
     XLU_ConfigList *channels, *ioports, *irqs, *iomem, *viridian, *dtdevs,
-                   *mca_caps;
+                   *mca_caps, *smbios;
     int num_ioports, num_irqs, num_iomem, num_cpus, num_viridian, num_mca_caps;
+    int num_smbios;
     int pci_power_mgmt = 0;
     int pci_msitranslate = 0;
     int pci_permissive = 0;
@@ -1783,6 +1784,47 @@  void parse_config_data(const char *config_source,
         xlu_cfg_replace_string(config, "acpi_firmware",
                                &b_info->u.hvm.acpi_firmware, 0);
 
+        switch (xlu_cfg_get_list(config, "smbios", &smbios, &num_smbios, 0))
+        {
+        case 0: /* Success */
+            b_info->u.hvm.num_smbios = num_smbios;
+            b_info->u.hvm.smbios = xcalloc(num_smbios, sizeof(libxl_smbios));
+            for (i = 0; i < num_smbios; i++) {
+                char *option_untrimmed, *value_untrimmed;
+                char *option, *value;
+                libxl_smbios_type v;
+
+                buf = xlu_cfg_get_listitem(smbios, i);
+                if (!buf) continue;
+
+                if (split_string_into_pair(buf, "=",
+                                           &option_untrimmed,
+                                           &value_untrimmed)) {
+                    fprintf(stderr, "xl: failed to split \"%s\" into pair\n",
+                            buf);
+                    exit(EXIT_FAILURE);
+                }
+                trim(isspace, option_untrimmed, &option);
+                trim(isspace, value_untrimmed, &value);
+
+                e = libxl_smbios_type_from_string(option, &v);
+                if (e) {
+                    fprintf(stderr,
+                            "xl: unknown smbios type '%s'\n",
+                            buf);
+                    exit(-ERROR_FAIL);
+                }
+
+                b_info->u.hvm.smbios[i].key = v;
+                b_info->u.hvm.smbios[i].value = value;
+            }
+            break;
+        case ESRCH: break; /* Option not present */
+        default:
+            fprintf(stderr,"xl: Unable to parse smbios options.\n");
+            exit(-ERROR_FAIL);
+        }
+
         if (!xlu_cfg_get_string(config, "ms_vm_genid", &buf, 0)) {
             if (!strcmp(buf, "generate")) {
                 e = libxl_ms_vm_genid_generate(ctx, &b_info->u.hvm.ms_vm_genid);