Re: [Qemu-devel] [QEMU v6 PATCH 00/17] SMBIOS: build full tables in QEMU

[Gabriel L. Somlo]

On Tue, Apr 22, 2014 at 06:16:53PM +0200, Markus Armbruster wrote:
> >>   1. smbios_entry_add() gets called for every "-smbios file=..." and
> >>      "-smbios type=..." command line argument
> >> 
> >>         a. "type=..." is OK, we just remember the values for later
> >> 
> >>         b. "file=..." less so, because we have to load the blobs into
> >>            *something*.
> >
> > Remember the filenames for later ...
> >
> >>  In my v7 patch set I decided to load the blobs
> >>            into *both* the legacy and new-fangled aggregate tables.
> >> 
> >>   2. smbios_set_defaults() is called
> >> 
> >>         - by now we *already* know the machine version we have, so
> >>           we get to free the table we now know we won't need
> >>           (aggregate if we're on <= v2.0, legacy if 2.1 or newer,
> >>            see 1.b. above)
> >
> > ... and load the blobs here?
> 
> I don't like this technique, because it tends to degrade error messages.
> When you're processing the option, your error messages come out with
> nice location information automatically.  If you just store filenames,
> that's lost.  To preserve it, you need to do extra work.
> 
> Note that smbios.c already stores the -smbios type=... for later use by
> smbios_get_table()'s table building.  Could you store the blob, too?

Can you all please check out v7 of the patch set at

http://lists.nongnu.org/archive/html/qemu-devel/2014-04/msg03195.html

In particular, patch 7/7 (toward the bottom, where smbios_entry_add()
is being patched, here:

http://lists.nongnu.org/archive/html/qemu-devel/2014-04/msg03196.html

The existing code already stores the blobs, wrapped in a
SMBIOS_TABLE_ENTRY wrapper, in "smbios_entries".

The new code needs the blobs *without* any wrappers, just concatenated
together, in the new aggregate table I named "smbios_tables".

So I'm only reading the file blobs *once* (in the new code), and then
malloc room for a wrapper+blob in the legacy smbios_entries table, and
memcpy from where I already read them into "smbios_tables".

I think this is the cleanest way to have both legacy and new/aggregate
logic live side by side. Intrinsically, the new code wouldn't need to
"reuse" any of the old code, so trying to factor out any "common bits"
would actually be less clean, IMHO...

Once you've had a chance to look at the latest patch, please let me
know if you have any strong objections to how it's handling the
problem, or advice on how to do it more cleanly...

Thanks much,
--Gabriel