diff mbox

[v2,2/7] smbios: Convert to QemuOpts

Message ID 1376659114-6630-3-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster Aug. 16, 2013, 1:18 p.m. UTC 
From: Markus Armbruster <armbru@redhat.com>

So that it can be set in config file for -readconfig.

This tightens parsing of -smbios, and makes it more consistent with
other options: unknown parameters are rejected, numbers with trailing
junk are rejected, when a parameter is given multiple times, last
rather than first wins, ...

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 arch_init.c                |   4 +-
 hw/i386/smbios.c           | 211 ++++++++++++++++++++++++++++++++++++---------
 include/hw/i386/smbios.h   |   4 +-
 include/sysemu/arch_init.h |   2 +-
 vl.c                       |   3 +-
 5 files changed, 180 insertions(+), 44 deletions(-)

Comments

Michael S. Tsirkin Sept. 28, 2013, 8:47 p.m. UTC | #1 
On Fri, Aug 16, 2013 at 03:18:29PM +0200, armbru@redhat.com wrote:
> From: Markus Armbruster <armbru@redhat.com>
> 
> So that it can be set in config file for -readconfig.
> 
> This tightens parsing of -smbios, and makes it more consistent with
> other options: unknown parameters are rejected, numbers with trailing
> junk are rejected, when a parameter is given multiple times, last
> rather than first wins, ...
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>  arch_init.c                |   4 +-
>  hw/i386/smbios.c           | 211 ++++++++++++++++++++++++++++++++++++---------
>  include/hw/i386/smbios.h   |   4 +-
>  include/sysemu/arch_init.h |   2 +-
>  vl.c                       |   3 +-
>  5 files changed, 180 insertions(+), 44 deletions(-)
> 
> diff --git a/arch_init.c b/arch_init.c
> index 1ddead0..96477fd 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -1133,10 +1133,10 @@ void do_acpitable_option(const QemuOpts *opts)
>  #endif
>  }
>  
> -void do_smbios_option(const char *optarg)
> +void do_smbios_option(QemuOpts *opts)
>  {
>  #ifdef TARGET_I386
> -    smbios_entry_add(optarg);
> +    smbios_entry_add(opts);
>  #endif
>  }
>  
> diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
> index 0608aee..a113f8b 100644
> --- a/hw/i386/smbios.c
> +++ b/hw/i386/smbios.c
> @@ -2,9 +2,11 @@
>   * SMBIOS Support
>   *
>   * Copyright (C) 2009 Hewlett-Packard Development Company, L.P.
> + * Copyright (C) 2013 Red Hat, Inc.
>   *
>   * Authors:
>   *  Alex Williamson <alex.williamson@hp.com>
> + *  Markus Armbruster <armbru@redhat.com>
>   *
>   * This work is licensed under the terms of the GNU GPL, version 2.  See
>   * the COPYING file in the top-level directory.
> @@ -13,6 +15,7 @@
>   * GNU GPL, version 2 or (at your option) any later version.
>   */
>  
> +#include "qemu/config-file.h"
>  #include "qemu/error-report.h"
>  #include "sysemu/sysemu.h"
>  #include "hw/i386/smbios.h"
> @@ -41,11 +44,100 @@ struct smbios_table {
>  #define SMBIOS_FIELD_ENTRY 0
>  #define SMBIOS_TABLE_ENTRY 1
>  
> -
>  static uint8_t *smbios_entries;
>  static size_t smbios_entries_len;
>  static int smbios_type4_count = 0;
>  
> +static QemuOptsList qemu_smbios_opts = {
> +    .name = "smbios",
> +    .head = QTAILQ_HEAD_INITIALIZER(qemu_smbios_opts.head),
> +    .desc = {
> +        /*
> +         * no elements => accept any params
> +         * validation will happen later
> +         */
> +        { /* end of list */ }
> +    }
> +};
> +
> +static const QemuOptDesc qemu_smbios_file_opts[] = {
> +    {
> +        .name = "file",
> +        .type = QEMU_OPT_STRING,
> +        .help = "binary file containing an SMBIOS element",
> +    },
> +    { /* end of list */ }
> +};
> +
> +static const QemuOptDesc qemu_smbios_type0_opts[] = {
> +    {
> +        .name = "type",
> +        .type = QEMU_OPT_NUMBER,
> +        .help = "SMBIOS element type",
> +    },{
> +        .name = "vendor",
> +        .type = QEMU_OPT_STRING,
> +        .help = "vendor name",
> +    },{
> +        .name = "version",
> +        .type = QEMU_OPT_STRING,
> +        .help = "version number",
> +    },{
> +        .name = "date",
> +        .type = QEMU_OPT_STRING,
> +        .help = "release date",
> +    },{
> +        .name = "release",
> +        .type = QEMU_OPT_STRING,
> +        .help = "revision number",
> +    },
> +    { /* end of list */ }
> +};
> +
> +static const QemuOptDesc qemu_smbios_type1_opts[] = {
> +    {
> +        .name = "type",
> +        .type = QEMU_OPT_NUMBER,
> +        .help = "SMBIOS element type",
> +    },{
> +        .name = "manufacturer",
> +        .type = QEMU_OPT_STRING,
> +        .help = "manufacturer name",
> +    },{
> +        .name = "product",
> +        .type = QEMU_OPT_STRING,
> +        .help = "product name",
> +    },{
> +        .name = "version",
> +        .type = QEMU_OPT_STRING,
> +        .help = "version number",
> +    },{
> +        .name = "serial",
> +        .type = QEMU_OPT_STRING,
> +        .help = "serial number",
> +    },{
> +        .name = "uuid",
> +        .type = QEMU_OPT_STRING,
> +        .help = "UUID",
> +    },{
> +        .name = "sku",
> +        .type = QEMU_OPT_STRING,
> +        .help = "SKU number",
> +    },{
> +        .name = "family",
> +        .type = QEMU_OPT_STRING,
> +        .help = "family name",
> +    },
> +    { /* end of list */ }
> +};
> +
> +static void smbios_register_config(void)
> +{
> +    qemu_add_opts(&qemu_smbios_opts);
> +}
> +
> +machine_init(smbios_register_config);
> +
>  static void smbios_validate_table(void)
>  {
>      if (smbios_type4_count && smbios_type4_count != smp_cpus) {
> @@ -124,23 +216,30 @@ void smbios_add_field(int type, int offset, const void *data, size_t len)
>              cpu_to_le16(le16_to_cpu(*(uint16_t *)smbios_entries) + 1);
>  }
>  
> -static void smbios_build_type_0_fields(const char *t)
> +static void smbios_build_type_0_fields(QemuOpts *opts)
>  {
> -    char buf[1024];
> +    const char *val;
>      unsigned char major, minor;
>  
> -    if (get_param_value(buf, sizeof(buf), "vendor", t))
> +    val = qemu_opt_get(opts, "vendor");
> +    if (val) {
>          smbios_add_field(0, offsetof(struct smbios_type_0, vendor_str),
> -                         buf, strlen(buf) + 1);
> -    if (get_param_value(buf, sizeof(buf), "version", t))
> +                         val, strlen(val) + 1);
> +    }
> +    val = qemu_opt_get(opts, "version");
> +    if (val) {
>          smbios_add_field(0, offsetof(struct smbios_type_0, bios_version_str),
> -                         buf, strlen(buf) + 1);
> -    if (get_param_value(buf, sizeof(buf), "date", t))
> +                         val, strlen(val) + 1);
> +    }
> +    val = qemu_opt_get(opts, "date");
> +    if (val) {
>          smbios_add_field(0, offsetof(struct smbios_type_0,
>                                       bios_release_date_str),
> -                         buf, strlen(buf) + 1);
> -    if (get_param_value(buf, sizeof(buf), "release", t)) {
> -        if (sscanf(buf, "%hhu.%hhu", &major, &minor) != 2) {
> +                         val, strlen(val) + 1);
> +    }
> +    val = qemu_opt_get(opts, "release");
> +    if (val) {
> +        if (sscanf(val, "%hhu.%hhu", &major, &minor) != 2) {
>              error_report("Invalid release");
>              exit(1);
>          }
> @@ -153,47 +252,69 @@ static void smbios_build_type_0_fields(const char *t)
>      }
>  }
>  
> -static void smbios_build_type_1_fields(const char *t)
> +static void smbios_build_type_1_fields(QemuOpts *opts)
>  {
> -    char buf[1024];
> +    const char *val;
>  
> -    if (get_param_value(buf, sizeof(buf), "manufacturer", t))
> +    val = qemu_opt_get(opts, "manufacturer");
> +    if (val) {
>          smbios_add_field(1, offsetof(struct smbios_type_1, manufacturer_str),
> -                         buf, strlen(buf) + 1);
> -    if (get_param_value(buf, sizeof(buf), "product", t))
> +                         val, strlen(val) + 1);
> +    }
> +    val = qemu_opt_get(opts, "product");
> +    if (val) {
>          smbios_add_field(1, offsetof(struct smbios_type_1, product_name_str),
> -                         buf, strlen(buf) + 1);
> -    if (get_param_value(buf, sizeof(buf), "version", t))
> +                         val, strlen(val) + 1);
> +    }
> +    val = qemu_opt_get(opts, "version");
> +    if (val) {
>          smbios_add_field(1, offsetof(struct smbios_type_1, version_str),
> -                         buf, strlen(buf) + 1);
> -    if (get_param_value(buf, sizeof(buf), "serial", t))
> +                         val, strlen(val) + 1);
> +    }
> +    val = qemu_opt_get(opts, "serial");
> +    if (val) {
>          smbios_add_field(1, offsetof(struct smbios_type_1, serial_number_str),
> -                         buf, strlen(buf) + 1);
> -    if (get_param_value(buf, sizeof(buf), "uuid", t)) {
> -        if (qemu_uuid_parse(buf, qemu_uuid) != 0) {
> +                         val, strlen(val) + 1);
> +    }
> +    val = qemu_opt_get(opts, "uuid");
> +    if (val) {
> +        if (qemu_uuid_parse(val, qemu_uuid) != 0) {
>              error_report("Invalid UUID");
>              exit(1);
>          }
>      }
> -    if (get_param_value(buf, sizeof(buf), "sku", t))
> +    val = qemu_opt_get(opts, "sku");
> +    if (val) {
>          smbios_add_field(1, offsetof(struct smbios_type_1, sku_number_str),
> -                         buf, strlen(buf) + 1);
> -    if (get_param_value(buf, sizeof(buf), "family", t))
> +                         val, strlen(val) + 1);
> +    }
> +    val = qemu_opt_get(opts, "family");
> +    if (val) {
>          smbios_add_field(1, offsetof(struct smbios_type_1, family_str),
> -                         buf, strlen(buf) + 1);
> +                         val, strlen(val) + 1);
> +    }
>  }
>  
> -void smbios_entry_add(const char *t)
> +void smbios_entry_add(QemuOpts *opts)
>  {
> -    char buf[1024];
> +    Error *local_err = NULL;
> +    const char *val;
>  
> -    if (get_param_value(buf, sizeof(buf), "file", t)) {
> +    val = qemu_opt_get(opts, "file");
> +    if (val) {
>          struct smbios_structure_header *header;
>          struct smbios_table *table;
> -        int size = get_image_size(buf);
> +        int size;
> +
> +        qemu_opts_validate(opts, qemu_smbios_file_opts, &local_err);
> +        if (local_err) {
> +            error_report("%s", error_get_pretty(local_err));
> +            exit(1);
> +        }
>  
> +        size = get_image_size(val);
>          if (size == -1 || size < sizeof(struct smbios_structure_header)) {
> -            error_report("Cannot read SMBIOS file %s", buf);
> +            error_report("Cannot read SMBIOS file %s", val);
>              exit(1);
>          }
>  
> @@ -208,8 +329,8 @@ void smbios_entry_add(const char *t)
>          table->header.type = SMBIOS_TABLE_ENTRY;
>          table->header.length = cpu_to_le16(sizeof(*table) + size);
>  
> -        if (load_image(buf, table->data) != size) {
> -            error_report("Failed to load SMBIOS file %s", buf);
> +        if (load_image(val, table->data) != size) {
> +            error_report("Failed to load SMBIOS file %s", val);
>              exit(1);
>          }
>  
> @@ -225,17 +346,29 @@ void smbios_entry_add(const char *t)
>          return;
>      }
>  
> -    if (get_param_value(buf, sizeof(buf), "type", t)) {
> -        unsigned long type = strtoul(buf, NULL, 0);
> +    val = qemu_opt_get(opts, "type");
> +    if (val) {
> +        unsigned long type = strtoul(val, NULL, 0);
> +
>          switch (type) {
>          case 0:
> -            smbios_build_type_0_fields(t);
> +            qemu_opts_validate(opts, qemu_smbios_type0_opts, &local_err);
> +            if (local_err) {
> +                error_report("%s", error_get_pretty(local_err));
> +                exit(1);
> +            }
> +            smbios_build_type_0_fields(opts);
>              return;
>          case 1:
> -            smbios_build_type_1_fields(t);
> +            qemu_opts_validate(opts, qemu_smbios_type1_opts, &local_err);
> +            if (local_err) {
> +                error_report("%s", error_get_pretty(local_err));
> +                exit(1);
> +            }
> +            smbios_build_type_1_fields(opts);
>              return;
>          default:
> -            error_report("Don't know how to build fields for SMBIOS type %ld",
> +            error_report("Don't know how to build fields for SMBIOS type %" PRIu64,
>                           type);
>              exit(1);
>          }

This triggers a build failure:

/scm/qemu/hw/i386/smbios.c: In function ‘smbios_entry_add’:
/scm/qemu/hw/i386/smbios.c:382:26: error: format ‘%llu’ expects argument
of type ‘long long unsigned int’, but argument 2 has type ‘long unsigned
int’ [-Werror=format=]
                          type);
                          ^

It's a long value, why are you printing it with PRIu64?
%ld seems right.
I reverted just this chunk.

> diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
> index 56c6108..d9f43b7 100644
> --- a/include/hw/i386/smbios.h
> +++ b/include/hw/i386/smbios.h
> @@ -13,7 +13,9 @@
>   *
>   */
>  
> -void smbios_entry_add(const char *t);
> +#include "qemu/option.h"
> +
> +void smbios_entry_add(QemuOpts *opts);
>  void smbios_add_field(int type, int offset, const void *data, size_t len);
>  uint8_t *smbios_get_table(size_t *length);
>  
> diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
> index dece913..be71bca 100644
> --- a/include/sysemu/arch_init.h
> +++ b/include/sysemu/arch_init.h
> @@ -28,7 +28,7 @@ extern const uint32_t arch_type;
>  
>  void select_soundhw(const char *optarg);
>  void do_acpitable_option(const QemuOpts *opts);
> -void do_smbios_option(const char *optarg);
> +void do_smbios_option(QemuOpts *opts);
>  void cpudef_init(void);
>  void audio_init(void);
>  int tcg_available(void);
> diff --git a/vl.c b/vl.c
> index f422a1c..f63ffd2 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3565,7 +3565,8 @@ int main(int argc, char **argv, char **envp)
>                  do_acpitable_option(opts);
>                  break;
>              case QEMU_OPTION_smbios:
> -                do_smbios_option(optarg);
> +                opts = qemu_opts_parse(qemu_find_opts("smbios"), optarg, 0);
> +                do_smbios_option(opts);
>                  break;
>              case QEMU_OPTION_enable_kvm:
>                  olist = qemu_find_opts("machine");
> -- 
> 1.8.1.4
>
Markus Armbruster Sept. 30, 2013, 8:48 a.m. UTC | #2 
"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Fri, Aug 16, 2013 at 03:18:29PM +0200, armbru@redhat.com wrote:
>> From: Markus Armbruster <armbru@redhat.com>
>> 
>> So that it can be set in config file for -readconfig.
>> 
>> This tightens parsing of -smbios, and makes it more consistent with
>> other options: unknown parameters are rejected, numbers with trailing
>> junk are rejected, when a parameter is given multiple times, last
>> rather than first wins, ...
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
[...]
>> diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
>> index 0608aee..a113f8b 100644
>> --- a/hw/i386/smbios.c
>> +++ b/hw/i386/smbios.c
[...]
>> @@ -225,17 +346,29 @@ void smbios_entry_add(const char *t)
>>          return;
>>      }
>>  
>> -    if (get_param_value(buf, sizeof(buf), "type", t)) {
>> -        unsigned long type = strtoul(buf, NULL, 0);
>> +    val = qemu_opt_get(opts, "type");
>> +    if (val) {
>> +        unsigned long type = strtoul(val, NULL, 0);
>> +
>>          switch (type) {
>>          case 0:
>> -            smbios_build_type_0_fields(t);
>> +            qemu_opts_validate(opts, qemu_smbios_type0_opts, &local_err);
>> +            if (local_err) {
>> +                error_report("%s", error_get_pretty(local_err));
>> +                exit(1);
>> +            }
>> +            smbios_build_type_0_fields(opts);
>>              return;
>>          case 1:
>> -            smbios_build_type_1_fields(t);
>> +            qemu_opts_validate(opts, qemu_smbios_type1_opts, &local_err);
>> +            if (local_err) {
>> +                error_report("%s", error_get_pretty(local_err));
>> +                exit(1);
>> +            }
>> +            smbios_build_type_1_fields(opts);
>>              return;
>>          default:
>> -            error_report("Don't know how to build fields for SMBIOS type %ld",
>> +            error_report("Don't know how to build fields for SMBIOS type %" PRIu64,
>>                           type);
>>              exit(1);
>>          }
>
> This triggers a build failure:
>
> /scm/qemu/hw/i386/smbios.c: In function ‘smbios_entry_add’:
> /scm/qemu/hw/i386/smbios.c:382:26: error: format ‘%llu’ expects argument
> of type ‘long long unsigned int’, but argument 2 has type ‘long unsigned
> int’ [-Werror=format=]
>                           type);
>                           ^
>
> It's a long value, why are you printing it with PRIu64?
> %ld seems right.

Yup.

> I reverted just this chunk.

Thanks for catching this.  My compiler doesn't :(

[...]
diff mbox

Patch

diff --git a/arch_init.c b/arch_init.c
index 1ddead0..96477fd 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -1133,10 +1133,10 @@  void do_acpitable_option(const QemuOpts *opts)
 #endif
 }
 
-void do_smbios_option(const char *optarg)
+void do_smbios_option(QemuOpts *opts)
 {
 #ifdef TARGET_I386
-    smbios_entry_add(optarg);
+    smbios_entry_add(opts);
 #endif
 }
 
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 0608aee..a113f8b 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -2,9 +2,11 @@ 
  * SMBIOS Support
  *
  * Copyright (C) 2009 Hewlett-Packard Development Company, L.P.
+ * Copyright (C) 2013 Red Hat, Inc.
  *
  * Authors:
  *  Alex Williamson <alex.williamson@hp.com>
+ *  Markus Armbruster <armbru@redhat.com>
  *
  * This work is licensed under the terms of the GNU GPL, version 2.  See
  * the COPYING file in the top-level directory.
@@ -13,6 +15,7 @@ 
  * GNU GPL, version 2 or (at your option) any later version.
  */
 
+#include "qemu/config-file.h"
 #include "qemu/error-report.h"
 #include "sysemu/sysemu.h"
 #include "hw/i386/smbios.h"
@@ -41,11 +44,100 @@  struct smbios_table {
 #define SMBIOS_FIELD_ENTRY 0
 #define SMBIOS_TABLE_ENTRY 1
 
-
 static uint8_t *smbios_entries;
 static size_t smbios_entries_len;
 static int smbios_type4_count = 0;
 
+static QemuOptsList qemu_smbios_opts = {
+    .name = "smbios",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_smbios_opts.head),
+    .desc = {
+        /*
+         * no elements => accept any params
+         * validation will happen later
+         */
+        { /* end of list */ }
+    }
+};
+
+static const QemuOptDesc qemu_smbios_file_opts[] = {
+    {
+        .name = "file",
+        .type = QEMU_OPT_STRING,
+        .help = "binary file containing an SMBIOS element",
+    },
+    { /* end of list */ }
+};
+
+static const QemuOptDesc qemu_smbios_type0_opts[] = {
+    {
+        .name = "type",
+        .type = QEMU_OPT_NUMBER,
+        .help = "SMBIOS element type",
+    },{
+        .name = "vendor",
+        .type = QEMU_OPT_STRING,
+        .help = "vendor name",
+    },{
+        .name = "version",
+        .type = QEMU_OPT_STRING,
+        .help = "version number",
+    },{
+        .name = "date",
+        .type = QEMU_OPT_STRING,
+        .help = "release date",
+    },{
+        .name = "release",
+        .type = QEMU_OPT_STRING,
+        .help = "revision number",
+    },
+    { /* end of list */ }
+};
+
+static const QemuOptDesc qemu_smbios_type1_opts[] = {
+    {
+        .name = "type",
+        .type = QEMU_OPT_NUMBER,
+        .help = "SMBIOS element type",
+    },{
+        .name = "manufacturer",
+        .type = QEMU_OPT_STRING,
+        .help = "manufacturer name",
+    },{
+        .name = "product",
+        .type = QEMU_OPT_STRING,
+        .help = "product name",
+    },{
+        .name = "version",
+        .type = QEMU_OPT_STRING,
+        .help = "version number",
+    },{
+        .name = "serial",
+        .type = QEMU_OPT_STRING,
+        .help = "serial number",
+    },{
+        .name = "uuid",
+        .type = QEMU_OPT_STRING,
+        .help = "UUID",
+    },{
+        .name = "sku",
+        .type = QEMU_OPT_STRING,
+        .help = "SKU number",
+    },{
+        .name = "family",
+        .type = QEMU_OPT_STRING,
+        .help = "family name",
+    },
+    { /* end of list */ }
+};
+
+static void smbios_register_config(void)
+{
+    qemu_add_opts(&qemu_smbios_opts);
+}
+
+machine_init(smbios_register_config);
+
 static void smbios_validate_table(void)
 {
     if (smbios_type4_count && smbios_type4_count != smp_cpus) {
@@ -124,23 +216,30 @@  void smbios_add_field(int type, int offset, const void *data, size_t len)
             cpu_to_le16(le16_to_cpu(*(uint16_t *)smbios_entries) + 1);
 }
 
-static void smbios_build_type_0_fields(const char *t)
+static void smbios_build_type_0_fields(QemuOpts *opts)
 {
-    char buf[1024];
+    const char *val;
     unsigned char major, minor;
 
-    if (get_param_value(buf, sizeof(buf), "vendor", t))
+    val = qemu_opt_get(opts, "vendor");
+    if (val) {
         smbios_add_field(0, offsetof(struct smbios_type_0, vendor_str),
-                         buf, strlen(buf) + 1);
-    if (get_param_value(buf, sizeof(buf), "version", t))
+                         val, strlen(val) + 1);
+    }
+    val = qemu_opt_get(opts, "version");
+    if (val) {
         smbios_add_field(0, offsetof(struct smbios_type_0, bios_version_str),
-                         buf, strlen(buf) + 1);
-    if (get_param_value(buf, sizeof(buf), "date", t))
+                         val, strlen(val) + 1);
+    }
+    val = qemu_opt_get(opts, "date");
+    if (val) {
         smbios_add_field(0, offsetof(struct smbios_type_0,
                                      bios_release_date_str),
-                         buf, strlen(buf) + 1);
-    if (get_param_value(buf, sizeof(buf), "release", t)) {
-        if (sscanf(buf, "%hhu.%hhu", &major, &minor) != 2) {
+                         val, strlen(val) + 1);
+    }
+    val = qemu_opt_get(opts, "release");
+    if (val) {
+        if (sscanf(val, "%hhu.%hhu", &major, &minor) != 2) {
             error_report("Invalid release");
             exit(1);
         }
@@ -153,47 +252,69 @@  static void smbios_build_type_0_fields(const char *t)
     }
 }
 
-static void smbios_build_type_1_fields(const char *t)
+static void smbios_build_type_1_fields(QemuOpts *opts)
 {
-    char buf[1024];
+    const char *val;
 
-    if (get_param_value(buf, sizeof(buf), "manufacturer", t))
+    val = qemu_opt_get(opts, "manufacturer");
+    if (val) {
         smbios_add_field(1, offsetof(struct smbios_type_1, manufacturer_str),
-                         buf, strlen(buf) + 1);
-    if (get_param_value(buf, sizeof(buf), "product", t))
+                         val, strlen(val) + 1);
+    }
+    val = qemu_opt_get(opts, "product");
+    if (val) {
         smbios_add_field(1, offsetof(struct smbios_type_1, product_name_str),
-                         buf, strlen(buf) + 1);
-    if (get_param_value(buf, sizeof(buf), "version", t))
+                         val, strlen(val) + 1);
+    }
+    val = qemu_opt_get(opts, "version");
+    if (val) {
         smbios_add_field(1, offsetof(struct smbios_type_1, version_str),
-                         buf, strlen(buf) + 1);
-    if (get_param_value(buf, sizeof(buf), "serial", t))
+                         val, strlen(val) + 1);
+    }
+    val = qemu_opt_get(opts, "serial");
+    if (val) {
         smbios_add_field(1, offsetof(struct smbios_type_1, serial_number_str),
-                         buf, strlen(buf) + 1);
-    if (get_param_value(buf, sizeof(buf), "uuid", t)) {
-        if (qemu_uuid_parse(buf, qemu_uuid) != 0) {
+                         val, strlen(val) + 1);
+    }
+    val = qemu_opt_get(opts, "uuid");
+    if (val) {
+        if (qemu_uuid_parse(val, qemu_uuid) != 0) {
             error_report("Invalid UUID");
             exit(1);
         }
     }
-    if (get_param_value(buf, sizeof(buf), "sku", t))
+    val = qemu_opt_get(opts, "sku");
+    if (val) {
         smbios_add_field(1, offsetof(struct smbios_type_1, sku_number_str),
-                         buf, strlen(buf) + 1);
-    if (get_param_value(buf, sizeof(buf), "family", t))
+                         val, strlen(val) + 1);
+    }
+    val = qemu_opt_get(opts, "family");
+    if (val) {
         smbios_add_field(1, offsetof(struct smbios_type_1, family_str),
-                         buf, strlen(buf) + 1);
+                         val, strlen(val) + 1);
+    }
 }
 
-void smbios_entry_add(const char *t)
+void smbios_entry_add(QemuOpts *opts)
 {
-    char buf[1024];
+    Error *local_err = NULL;
+    const char *val;
 
-    if (get_param_value(buf, sizeof(buf), "file", t)) {
+    val = qemu_opt_get(opts, "file");
+    if (val) {
         struct smbios_structure_header *header;
         struct smbios_table *table;
-        int size = get_image_size(buf);
+        int size;
+
+        qemu_opts_validate(opts, qemu_smbios_file_opts, &local_err);
+        if (local_err) {
+            error_report("%s", error_get_pretty(local_err));
+            exit(1);
+        }
 
+        size = get_image_size(val);
         if (size == -1 || size < sizeof(struct smbios_structure_header)) {
-            error_report("Cannot read SMBIOS file %s", buf);
+            error_report("Cannot read SMBIOS file %s", val);
             exit(1);
         }
 
@@ -208,8 +329,8 @@  void smbios_entry_add(const char *t)
         table->header.type = SMBIOS_TABLE_ENTRY;
         table->header.length = cpu_to_le16(sizeof(*table) + size);
 
-        if (load_image(buf, table->data) != size) {
-            error_report("Failed to load SMBIOS file %s", buf);
+        if (load_image(val, table->data) != size) {
+            error_report("Failed to load SMBIOS file %s", val);
             exit(1);
         }
 
@@ -225,17 +346,29 @@  void smbios_entry_add(const char *t)
         return;
     }
 
-    if (get_param_value(buf, sizeof(buf), "type", t)) {
-        unsigned long type = strtoul(buf, NULL, 0);
+    val = qemu_opt_get(opts, "type");
+    if (val) {
+        unsigned long type = strtoul(val, NULL, 0);
+
         switch (type) {
         case 0:
-            smbios_build_type_0_fields(t);
+            qemu_opts_validate(opts, qemu_smbios_type0_opts, &local_err);
+            if (local_err) {
+                error_report("%s", error_get_pretty(local_err));
+                exit(1);
+            }
+            smbios_build_type_0_fields(opts);
             return;
         case 1:
-            smbios_build_type_1_fields(t);
+            qemu_opts_validate(opts, qemu_smbios_type1_opts, &local_err);
+            if (local_err) {
+                error_report("%s", error_get_pretty(local_err));
+                exit(1);
+            }
+            smbios_build_type_1_fields(opts);
             return;
         default:
-            error_report("Don't know how to build fields for SMBIOS type %ld",
+            error_report("Don't know how to build fields for SMBIOS type %" PRIu64,
                          type);
             exit(1);
         }
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index 56c6108..d9f43b7 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -13,7 +13,9 @@ 
  *
  */
 
-void smbios_entry_add(const char *t);
+#include "qemu/option.h"
+
+void smbios_entry_add(QemuOpts *opts);
 void smbios_add_field(int type, int offset, const void *data, size_t len);
 uint8_t *smbios_get_table(size_t *length);
 
diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
index dece913..be71bca 100644
--- a/include/sysemu/arch_init.h
+++ b/include/sysemu/arch_init.h
@@ -28,7 +28,7 @@  extern const uint32_t arch_type;
 
 void select_soundhw(const char *optarg);
 void do_acpitable_option(const QemuOpts *opts);
-void do_smbios_option(const char *optarg);
+void do_smbios_option(QemuOpts *opts);
 void cpudef_init(void);
 void audio_init(void);
 int tcg_available(void);
diff --git a/vl.c b/vl.c
index f422a1c..f63ffd2 100644
--- a/vl.c
+++ b/vl.c
@@ -3565,7 +3565,8 @@  int main(int argc, char **argv, char **envp)
                 do_acpitable_option(opts);
                 break;
             case QEMU_OPTION_smbios:
-                do_smbios_option(optarg);
+                opts = qemu_opts_parse(qemu_find_opts("smbios"), optarg, 0);
+                do_smbios_option(opts);
                 break;
             case QEMU_OPTION_enable_kvm:
                 olist = qemu_find_opts("machine");