Re: [PATCH 1/2] qemu: Allow SMBIOS entries to be loaded and provided to the VM BIOS

Anthony Liguori
2009-04-06T19:50:07Z

Alex Williamson wrote:
> Create a new -smbios options that takes binary SMBIOS entries
> to provide to the VM BIOS. The binary can be easily generated
> using something like:
>
> dmidecode -t 1 -u | grep $'^\t\t[^"]' | xargs -n1 | \
> perl -lne 'printf "%c", hex($_)' > smbios_type_1.bin
>
> For some inventory tools, this makes the VM report the system
> information for the host. One entry per binary file, multiple
> files can be chained together as:
>
> -smbios file1,file2,...
>
> or specified independently:
>
> -smbios file1 -smbios file2
>
> Signed-off-by: Alex Williamson <alex.williamson@hp.com>
>

Hi Alex,

I know we have to support blobs because of OEM specific smbios entries,
but there are a number of common ones that it would probably be good to
specify in a less user-unfriendly way. What do you think?

Anyway, comments below.

> diff --git a/hw/acpi.c b/hw/acpi.c
> index 52f50a0..0bd93bf 100644
> --- a/hw/acpi.c
> +++ b/hw/acpi.c
> @@ -915,3 +915,69 @@ out:
> }
> return -1;
> }
> +
> +char *smbios_entries;
> +size_t smbios_entries_len;
>

I think an accessor would be better than making these variables global.

> +int smbios_entry_add(const char *t)
> +{
>

acpi.c is hardware emulation, I'd rather see the command line parsing
done somewhere else (like vl.c).

> + struct stat s;
> + char file[1024], *p, *f, *n;
> + int fd, r;
> + size_t len, off;
> +
> + f = (char *)t;
> + do {
> + n = strchr(f, ',');
> + if (n) {
> + strncpy(file, f, (n - f));
> + file[n - f] = '\0';
> + f = n + 1;
> + } else {
> + strcpy(file, f);
> + f += strlen(file);
> + }
>

I'm happy to just require multiple -smbios options. I dislike
overloading with ','s even though we do it a lot in QEMU.

> + fd = open(file, O_RDONLY);
> + if (fd < 0)
> + return -1;
> +
> + if (fstat(fd, &s) < 0) {
> + close(fd);
> + return -1;
> + }
>

May want to look at load_image/get_image_size.

--
Regards,

Anthony Liguori