Re: [PATCH v2 1/3] hw/smbios: support loading OEM strings values from a file
Hi Daniel,
On 09/23/20 12:41, Daniel P. Berrangé wrote:
> Some applications want to pass quite large values for the OEM strings
> entries. Rather than having huge strings on the command line, it would
> be better to load them from a file, as supported with -fw_cfg.
>
> This introduces the "path" parameter allowing for:
>
> $ echo -n "thisthing" > mydata.txt
> $ qemu-system-x86_64 \
> -smbios type=11,value=something \
> -smbios type=11,path=mydata.txt \
> -smbios type=11,value=somemore \
> ...other args...
>
> Now in the guest
>
> $ dmidecode -t 11
> Getting SMBIOS data from sysfs.
> SMBIOS 2.8 present.
>
> Handle 0x0E00, DMI type 11, 5 bytes
> OEM Strings
> String 1: something
> String 2: thisthing
> String 3: somemore
>
> Reviewed-by: Philippe Mathieu-Daudé
> Signed-off-by: Daniel P. Berrangé
> ---
> hw/smbios/smbios.c | 72 +-
> 1 file changed, 59 insertions(+), 13 deletions(-)
>
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index 7cc950b41c..8450fad285 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -110,7 +110,7 @@ static struct {
>
> static struct {
> size_t nvalues;
> -const char **values;
> +char **values;
> } type11;
>
> static struct {
> @@ -314,6 +314,11 @@ static const QemuOptDesc qemu_smbios_type11_opts[] = {
> .type = QEMU_OPT_STRING,
> .help = "OEM string data",
> },
> +{
> +.name = "path",
> +.type = QEMU_OPT_STRING,
> +.help = "OEM string data from file",
> +},
> };
>
> static const QemuOptDesc qemu_smbios_type17_opts[] = {
> @@ -641,6 +646,8 @@ static void smbios_build_type_11_table(void)
>
> for (i = 0; i < type11.nvalues; i++) {
> SMBIOS_TABLE_SET_STR_LIST(11, type11.values[i]);
> +g_free(type11.values[i]);
> +type11.values[i] = NULL;
> }
>
> SMBIOS_BUILD_TABLE_POST;
> @@ -940,9 +947,8 @@ static void save_opt(const char **dest, QemuOpts *opts,
> const char *name)
>
>
> struct opt_list {
> -const char *name;
> size_t *ndest;
> -const char ***dest;
> +char ***dest;
> };
>
> static int save_opt_one(void *opaque,
> @@ -951,23 +957,61 @@ static int save_opt_one(void *opaque,
> {
> struct opt_list *opt = opaque;
>
> -if (!g_str_equal(name, opt->name)) {
> -return 0;
> +if (g_str_equal(name, "path")) {
> +g_autoptr(GByteArray) data = g_byte_array_new();
> +g_autofree char *buf = g_new(char, 4096);
> +ssize_t ret;
> +int fd = qemu_open(value, O_RDONLY);
This line now fails to compile, due to commit c490af57cb45 ("util:
introduce qemu_open and qemu_create with error reporting", 2020-09-16).
... I guess I could test the patch with qemu_open_old(), but that
wouldn't allow for a valid Tested-by.
Thanks,
Laszlo
> +if (fd < 0) {
> +error_setg(errp, "Unable to open %s: %s", value,
> strerror(errno));
> +return -1;
> +}
> +
> +while (1) {
> +ret = read(fd, buf, 4096);
> +if (ret == 0) {
> +break;
> +}
> +if (ret < 0) {
> +error_setg(errp, "Unable to read from %s: %s",
> + value, strerror(errno));
> +return -1;
> +}
> +if (memchr(buf, '\0', ret)) {
> +error_setg(errp, "NUL in OEM strings value in %s", value);
> +return -1;
> +}
> +g_byte_array_append(data, (guint8 *)buf, ret);
> +}
> +
> +close(fd);
> +
> +*opt->dest = g_renew(char *, *opt->dest, (*opt->ndest) + 1);
> +(*opt->dest)[*opt->ndest] = (char *)g_byte_array_free(data, FALSE);
> +(*opt->ndest)++;
> +data = NULL;
> + } else if (g_str_equal(name, "value")) {
> +*opt->dest = g_renew(char *, *opt->dest, (*opt->ndest) + 1);
> +(*opt->dest)[*opt->ndest] = g_strdup(value);
> +(*opt->ndest)++;
> +} else if (!g_str_equal(name, "type")) {
> +error_setg(errp, "Unexpected option %s", name);
> +return -1;
> }
>
> -*opt->dest = g_renew(const char *, *opt->dest, (*opt->ndest) + 1);
> -(*opt->dest)[*opt->ndest] = value;
> -(*opt->ndest)++;
> return 0;
> }
>
> -static void save_opt_list(size_t *ndest, const char ***dest,
> - QemuOpts *opts, const char *name)
> +static bool save_opt_list(size_t *ndest, char ***dest, QemuOpts *opts,
> + Error **errp)
> {
> struct opt_list opt = {
> -name, ndest, dest,
> +ndest, dest,
> };
> -qemu_opt_foreach(opts, save_opt_one, &opt, NULL);
> +if (!qemu_opt_foreach(opts, save_opt_one, &opt, errp)) {
> +return false;
> +}
> +return true;
> }
>
> void smbios_entry_add(QemuOpts *opts, Error **errp)
> @@ -1149,7 +1193,9 @@ void smbios_entry
[PATCH v2 1/3] hw/smbios: support loading OEM strings values from a file
Some applications want to pass quite large values for the OEM strings
entries. Rather than having huge strings on the command line, it would
be better to load them from a file, as supported with -fw_cfg.
This introduces the "path" parameter allowing for:
$ echo -n "thisthing" > mydata.txt
$ qemu-system-x86_64 \
-smbios type=11,value=something \
-smbios type=11,path=mydata.txt \
-smbios type=11,value=somemore \
...other args...
Now in the guest
$ dmidecode -t 11
Getting SMBIOS data from sysfs.
SMBIOS 2.8 present.
Handle 0x0E00, DMI type 11, 5 bytes
OEM Strings
String 1: something
String 2: thisthing
String 3: somemore
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Daniel P. Berrangé
---
hw/smbios/smbios.c | 72 +-
1 file changed, 59 insertions(+), 13 deletions(-)
diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index 7cc950b41c..8450fad285 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -110,7 +110,7 @@ static struct {
static struct {
size_t nvalues;
-const char **values;
+char **values;
} type11;
static struct {
@@ -314,6 +314,11 @@ static const QemuOptDesc qemu_smbios_type11_opts[] = {
.type = QEMU_OPT_STRING,
.help = "OEM string data",
},
+{
+.name = "path",
+.type = QEMU_OPT_STRING,
+.help = "OEM string data from file",
+},
};
static const QemuOptDesc qemu_smbios_type17_opts[] = {
@@ -641,6 +646,8 @@ static void smbios_build_type_11_table(void)
for (i = 0; i < type11.nvalues; i++) {
SMBIOS_TABLE_SET_STR_LIST(11, type11.values[i]);
+g_free(type11.values[i]);
+type11.values[i] = NULL;
}
SMBIOS_BUILD_TABLE_POST;
@@ -940,9 +947,8 @@ static void save_opt(const char **dest, QemuOpts *opts,
const char *name)
struct opt_list {
-const char *name;
size_t *ndest;
-const char ***dest;
+char ***dest;
};
static int save_opt_one(void *opaque,
@@ -951,23 +957,61 @@ static int save_opt_one(void *opaque,
{
struct opt_list *opt = opaque;
-if (!g_str_equal(name, opt->name)) {
-return 0;
+if (g_str_equal(name, "path")) {
+g_autoptr(GByteArray) data = g_byte_array_new();
+g_autofree char *buf = g_new(char, 4096);
+ssize_t ret;
+int fd = qemu_open(value, O_RDONLY);
+if (fd < 0) {
+error_setg(errp, "Unable to open %s: %s", value, strerror(errno));
+return -1;
+}
+
+while (1) {
+ret = read(fd, buf, 4096);
+if (ret == 0) {
+break;
+}
+if (ret < 0) {
+error_setg(errp, "Unable to read from %s: %s",
+ value, strerror(errno));
+return -1;
+}
+if (memchr(buf, '\0', ret)) {
+error_setg(errp, "NUL in OEM strings value in %s", value);
+return -1;
+}
+g_byte_array_append(data, (guint8 *)buf, ret);
+}
+
+close(fd);
+
+*opt->dest = g_renew(char *, *opt->dest, (*opt->ndest) + 1);
+(*opt->dest)[*opt->ndest] = (char *)g_byte_array_free(data, FALSE);
+(*opt->ndest)++;
+data = NULL;
+ } else if (g_str_equal(name, "value")) {
+*opt->dest = g_renew(char *, *opt->dest, (*opt->ndest) + 1);
+(*opt->dest)[*opt->ndest] = g_strdup(value);
+(*opt->ndest)++;
+} else if (!g_str_equal(name, "type")) {
+error_setg(errp, "Unexpected option %s", name);
+return -1;
}
-*opt->dest = g_renew(const char *, *opt->dest, (*opt->ndest) + 1);
-(*opt->dest)[*opt->ndest] = value;
-(*opt->ndest)++;
return 0;
}
-static void save_opt_list(size_t *ndest, const char ***dest,
- QemuOpts *opts, const char *name)
+static bool save_opt_list(size_t *ndest, char ***dest, QemuOpts *opts,
+ Error **errp)
{
struct opt_list opt = {
-name, ndest, dest,
+ndest, dest,
};
-qemu_opt_foreach(opts, save_opt_one, &opt, NULL);
+if (!qemu_opt_foreach(opts, save_opt_one, &opt, errp)) {
+return false;
+}
+return true;
}
void smbios_entry_add(QemuOpts *opts, Error **errp)
@@ -1149,7 +1193,9 @@ void smbios_entry_add(QemuOpts *opts, Error **errp)
if (!qemu_opts_validate(opts, qemu_smbios_type11_opts, errp)) {
return;
}
-save_opt_list(&type11.nvalues, &type11.values, opts, "value");
+if (!save_opt_list(&type11.nvalues, &type11.values, opts, errp)) {
+return;
+}
return;
case 17:
if (!qemu_opts_validate(opts, qemu_smbios_type17_opts, errp)) {
--
2.26.2
