| Message ID | 1633626876-12115-10-git-send-email-eric.devolder@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | acpi: Error Record Serialization Table, ERST, support for QEMU | expand |
The ordering of the patches is wrong! (a) First, you need this patch so that the test framework will ignore changes to the table blobs that you specify here to explicitly ignore. (b) Then you need the patch that actually contains the test you wrote (patch 8). Now because you have previously ignored the changes to ACPI tables that the test would modify, the tests would continue to ignore them and hence it would not fail "make check". (c) Lastly, you need patch 10 where you empty out the list of table blobs to ignore and update the blobs. Please also make sure you only update those table blobs that you explicitly ignored in step (a). On Thu, 7 Oct 2021, Eric DeVolder wrote: > Following the guidelines in tests/qtest/bios-tables-test.c, this > change adds empty placeholder files per step 1 for the new ERST > table, and excludes resulting changed files in bios-tables-test-allowed-diff.h > per step 2. > > Signed-off-by: Eric DeVolder <eric.devolder@oracle.com> > Acked-by: Igor Mammedov <imammedo@redhat.com> > --- > tests/data/acpi/microvm/ERST.pcie | 0 > tests/data/acpi/pc/DSDT.acpierst | 0 > tests/data/acpi/pc/ERST | 0 > tests/data/acpi/q35/DSDT.acpierst | 0 > tests/data/acpi/q35/ERST | 0 > tests/qtest/bios-tables-test-allowed-diff.h | 5 +++++ > 6 files changed, 5 insertions(+) > create mode 100644 tests/data/acpi/microvm/ERST.pcie > create mode 100644 tests/data/acpi/pc/DSDT.acpierst > create mode 100644 tests/data/acpi/pc/ERST > create mode 100644 tests/data/acpi/q35/DSDT.acpierst > create mode 100644 tests/data/acpi/q35/ERST > > diff --git a/tests/data/acpi/microvm/ERST.pcie b/tests/data/acpi/microvm/ERST.pcie > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/data/acpi/pc/DSDT.acpierst b/tests/data/acpi/pc/DSDT.acpierst > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/data/acpi/pc/ERST b/tests/data/acpi/pc/ERST > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/data/acpi/q35/DSDT.acpierst b/tests/data/acpi/q35/DSDT.acpierst > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/data/acpi/q35/ERST b/tests/data/acpi/q35/ERST > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h > index dfb8523..c06241a 100644 > --- a/tests/qtest/bios-tables-test-allowed-diff.h > +++ b/tests/qtest/bios-tables-test-allowed-diff.h > @@ -1 +1,6 @@ > /* List of comma-separated changed AML files to ignore */ > +"tests/data/acpi/pc/DSDT.acpierst", > +"tests/data/acpi/pc/ERST", > +"tests/data/acpi/q35/DSDT.acpierst", > +"tests/data/acpi/q35/ERST", > +"tests/data/acpi/microvm/ERST.pcie", > -- > 1.8.3.1 > >
On 10/7/21 20:17, Ani Sinha wrote: > The ordering of the patches is wrong! > > (a) First, you need this patch so that the test framework will ignore > changes > to the table blobs that you specify here to explicitly ignore. > > (b) Then you need the patch that actually contains the test you wrote > (patch > 8). Now because you have previously ignored the changes to ACPI tables > that the test would modify, the tests would continue to ignore them and > hence it would not fail "make check". > > (c) Lastly, you need patch 10 where you empty out the list of table blobs > to > ignore and update the blobs. Thanks for explaining; it sounds like the steps outlined in bios-tables-test.c. I moved this patch based on previous review, perhaps I misunderstand what was being requested. At any rate, I've moved this to be the first patch, per bios-tables-test.c > > Please also make sure you only update those table blobs that you > explicitly ignored in step (a). Done! > > > On Thu, 7 Oct 2021, Eric DeVolder wrote: > >> Following the guidelines in tests/qtest/bios-tables-test.c, this >> change adds empty placeholder files per step 1 for the new ERST >> table, and excludes resulting changed files in bios-tables-test-allowed-diff.h >> per step 2. >> >> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com> >> Acked-by: Igor Mammedov <imammedo@redhat.com> >> --- >> tests/data/acpi/microvm/ERST.pcie | 0 >> tests/data/acpi/pc/DSDT.acpierst | 0 >> tests/data/acpi/pc/ERST | 0 >> tests/data/acpi/q35/DSDT.acpierst | 0 >> tests/data/acpi/q35/ERST | 0 >> tests/qtest/bios-tables-test-allowed-diff.h | 5 +++++ >> 6 files changed, 5 insertions(+) >> create mode 100644 tests/data/acpi/microvm/ERST.pcie >> create mode 100644 tests/data/acpi/pc/DSDT.acpierst >> create mode 100644 tests/data/acpi/pc/ERST >> create mode 100644 tests/data/acpi/q35/DSDT.acpierst >> create mode 100644 tests/data/acpi/q35/ERST >> >> diff --git a/tests/data/acpi/microvm/ERST.pcie b/tests/data/acpi/microvm/ERST.pcie >> new file mode 100644 >> index 0000000..e69de29 >> diff --git a/tests/data/acpi/pc/DSDT.acpierst b/tests/data/acpi/pc/DSDT.acpierst >> new file mode 100644 >> index 0000000..e69de29 >> diff --git a/tests/data/acpi/pc/ERST b/tests/data/acpi/pc/ERST >> new file mode 100644 >> index 0000000..e69de29 >> diff --git a/tests/data/acpi/q35/DSDT.acpierst b/tests/data/acpi/q35/DSDT.acpierst >> new file mode 100644 >> index 0000000..e69de29 >> diff --git a/tests/data/acpi/q35/ERST b/tests/data/acpi/q35/ERST >> new file mode 100644 >> index 0000000..e69de29 >> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h >> index dfb8523..c06241a 100644 >> --- a/tests/qtest/bios-tables-test-allowed-diff.h >> +++ b/tests/qtest/bios-tables-test-allowed-diff.h >> @@ -1 +1,6 @@ >> /* List of comma-separated changed AML files to ignore */ >> +"tests/data/acpi/pc/DSDT.acpierst", >> +"tests/data/acpi/pc/ERST", >> +"tests/data/acpi/q35/DSDT.acpierst", >> +"tests/data/acpi/q35/ERST", >> +"tests/data/acpi/microvm/ERST.pcie", >> -- >> 1.8.3.1 >> >>
diff --git a/tests/data/acpi/microvm/ERST.pcie b/tests/data/acpi/microvm/ERST.pcie new file mode 100644 index 0000000..e69de29 diff --git a/tests/data/acpi/pc/DSDT.acpierst b/tests/data/acpi/pc/DSDT.acpierst new file mode 100644 index 0000000..e69de29 diff --git a/tests/data/acpi/pc/ERST b/tests/data/acpi/pc/ERST new file mode 100644 index 0000000..e69de29 diff --git a/tests/data/acpi/q35/DSDT.acpierst b/tests/data/acpi/q35/DSDT.acpierst new file mode 100644 index 0000000..e69de29 diff --git a/tests/data/acpi/q35/ERST b/tests/data/acpi/q35/ERST new file mode 100644 index 0000000..e69de29 diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index dfb8523..c06241a 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1 +1,6 @@ /* List of comma-separated changed AML files to ignore */ +"tests/data/acpi/pc/DSDT.acpierst", +"tests/data/acpi/pc/ERST", +"tests/data/acpi/q35/DSDT.acpierst", +"tests/data/acpi/q35/ERST", +"tests/data/acpi/microvm/ERST.pcie",