On Wed, 28 Feb 2024 16:41:22 +0530
Ani Sinha <anisinha@redhat.com> wrote:
> > On 27-Feb-2024, at 21:17, Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > Unfortunately having 2.0 machine type deprecated is not enough
> > to get rid of legacy SMBIOS handling since 'isapc' also uses
> > that and it's staying around.
> >
> > Hence add test for CLI options handling to be sure that it
> > ain't broken during SMBIOS code refactoring.
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > tests/data/smbios/type11_blob.legacy | Bin 0 -> 10 bytes
> > tests/qtest/bios-tables-test.c | 17 +++++++++++++++++
> > 2 files changed, 17 insertions(+)
> > create mode 100644 tests/data/smbios/type11_blob.legacy
> >
> > diff --git a/tests/data/smbios/type11_blob.legacy b/tests/data/smbios/type11_blob.legacy
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..aef463aab903405958b0a85f85c5980671c08bee
> > GIT binary patch
> > literal 10
> > Rcmd;PW!S(N;u;*n000Tp0s;U4
> >
> > literal 0
> > HcmV?d00001
> >
> > diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> > index a116f88e1d..d1ff4db7a2 100644
> > --- a/tests/qtest/bios-tables-test.c
> > +++ b/tests/qtest/bios-tables-test.c
> > @@ -2106,6 +2106,21 @@ static void test_acpi_pc_smbios_blob(void)
> > free_test_data(&data);
> > }
> >
> > +static void test_acpi_isapc_smbios_legacy(void)
> > +{
> > + uint8_t req_type11[] = { 1, 11 };
>
> Ok so looking at the code, it won’t let you specify smbios type other than 1 …
>
> See the following in smbios_set_defaults()
>
> if (smbios_legacy) {
> g_free(smbios_tables);
> /* in legacy mode, also complain if fields were given for types >1 */
> if (find_next_bit(have_fields_bitmap,
> SMBIOS_MAX_TYPE+1, 2) < SMBIOS_MAX_TYPE+1) {
> error_report("can't process fields for smbios "
> "types > 1 on machine versions < 2.1!");
> exit(1);
> }
> } else {
>
> BUT you lets you load a blob of type 11? Is that a bug in QEMU? Should we also add a similar check for have_binfile_bitmap?
I'd say bug (oversight) in QEMU, actually some of patches later in series
mention this issue in commit messages.
And I'm adding check only for type4 which I have to touch
as for other types it might work (type11) or not.
But I have not tested every possible type (only type 11 and 4)
the former lets test bloated SMBIOS blob and the later
needs refactoring to make this series work..
Everything else was ignored for this series and
can be done later on top.
As mentioned later in series I'd rather deprecate and
drop all legacy code instead of fixing it.
These tests are mostly for me to be sure that existing
CLI handling and blob building aren't broken during
refactoring.
> > + test_data data = {
> > + .machine = "isapc",
> > + .variant = ".pc_smbios_legacy",
> > + .required_struct_types = req_type11,
> > + .required_struct_types_len = ARRAY_SIZE(req_type11),
> > + };
> > +
> > + test_smbios("-smbios file=tests/data/smbios/type11_blob.legacy "
> > + "-smbios type=1,family=TEST", &data);
> > + free_test_data(&data);
> > +}
> > +
> > static void test_oem_fields(test_data *data)
> > {
> > int i;
> > @@ -2261,6 +2276,8 @@ int main(int argc, char *argv[])
> > test_acpi_pc_smbios_options);
> > qtest_add_func("acpi/piix4/smbios-blob",
> > test_acpi_pc_smbios_blob);
> > + qtest_add_func("acpi/piix4/smbios-legacy",
> > + test_acpi_isapc_smbios_legacy);
> > }
> > if (qtest_has_machine(MACHINE_Q35)) {
> > qtest_add_func("acpi/q35", test_acpi_q35_tcg);
> > --
> > 2.39.3
> >
>