On 08/25/2015 11:01 AM, Peter Maydell wrote:
> On 25 August 2015 at 16:59, Wei Huang <wei@redhat.com> wrote:
>> On 08/25/2015 10:29 AM, Leif Lindholm wrote:
>>> Wei - is there actually any particular point in renaming this
>>> structure? In all versions of the specification before 3.0, this was
>>> only known as the "smbios entry point". Only with the introduction of
>>> SMBIOS 3.0 this was retrospectively renamed.

Hi Leif,

While fixing up the patches, I started to lean towards keeping -21 in
smbios-entry-point naming. Here are my points:

1) It is very easy to get confused the "union SmbiosEntryPoint" with
"struct smbios_entry_point" if we remove _21 from the original "struct
smbios_21_entry_point". A symmetric naming of "struct
smbios_21_entry_point" & "struct smbios_30_entry_point" actually is
easier to read;
2) SMBIOS 3.0 specification clearly separate 2.1 and 3.0 definitions. I
understand that people who started from old spec might be a bit confused
to read the code; But for those who pick up SMBIOS 3.0 (and beyond)
fresh, they will know the difference while reading the code.
3) The issues Peter found is x86 specific and we know that QEMU only
generates SMBIOS 2.1 tables for PC model. So we can fix the test code to
only use _21 struct. In the future, when we started to create test code
for ARM, it can be further merged.

Anyway, I attached the patch for your review. It addresses some of your
concerns regarding SMBIOS 2.1 usage cases by detailed comments (see
smbios.h).

>>
>> I can take this suggestion, with clear comment in header file so nobody
>> will get confused. Peter, please let me know if you object.
> 
> I don't object (though the opinion of the qemu smbios/acpi
> folk is probably more important than mine).
> 
> Please make sure you test the x86 platform has not been broken by
> this change (preferably more thoroughly than just running
> 'make check'...).

(I have tested the code with the following methods:

* dmidecode insider an ARM guest VM
* "make check" on both ARM and x86 hosts
* dmidecode insider a x86-64 Windows 7 VM
)


> 
> thanks
> -- PMM
>