On Wed, Jan 15, 2020 at 05:25:50PM +0800, Guoheyi wrote: > > 在 2020/1/15 14:30, Michael S. Tsirkin 写道: > > Problem is IASL disassembler still doesn't work on all hosts > > we want to support. And its output isn't really stable enough > > to act as a golden master. > > > > Until we have a better tool, I propose the contributor just follows all > > steps 1-6. The reason they have been listed as maintainer action items > > is really just so that multiple patches affecting same ACPI table > > can be applied, with maintainer resolving conflicts himself. > > But this job can be pushed to contributors if as in the case of ARM > > maintainer isn't really interested in reading ACPI code anyway. > > > > So I propose the following patch - comments? > > > > Signed-off-by: Michael S. Tsirkin <m...@redhat.com> > > > > > > diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c > > index f1ac2d7e96..3a6a3e7257 100644 > > --- a/tests/qtest/bios-tables-test.c > > +++ b/tests/qtest/bios-tables-test.c > > @@ -16,7 +16,10 @@ > > * 1. add empty files for new tables, if any, under tests/data/acpi > > * 2. list any changed files in tests/bios-tables-test-allowed-diff.h > > * 3. commit the above *before* making changes that affect the tables > > - * Maintainer: > > + * > > + * Contributor or ACPI Maintainer (steps 4-7 need to be redone to resolve > > conflicts > > + * in binary commit created in step 6): > > + * > > * After 1-3 above tests will pass but ignore differences with the > > expected files. > > * You will also notice that tests/bios-tables-test-allowed-diff.h lists > > * a bunch of files. This is your hint that you need to do the below: > > @@ -28,13 +31,17 @@ > > * output. If not - disassemble them yourself in any way you like. > > * Look at the differences - make sure they make sense and match what the > > * changes you are merging are supposed to do. > > + * Save the changes, preferably in form of ASL diff for the the commit log > > in > NIT: 2 "the" before commit log > > + * step 6. > > * > > * 5. From build directory, run: > > * $(SRC_PATH)/tests/data/acpi/rebuild-expected-aml.sh > > - * 6. Now commit any changes. > > - * 7. Before doing a pull request, make sure > > tests/bios-tables-test-allowed-diff.h > > - * is empty - this will ensure following changes to ACPI tables will > > - * be noticed. > > + * 6. Now commit any changes to the expected binary, include diff from > > step 4 > > + * in commit log. > > + * 7. Before sending patches to the list (Contributor) > > + * or before doing a pull request (Maintainer), make sure > > + * tests/bios-tables-test-allowed-diff.h is empty - this will ensure > > + * following changes to ACPI tables will be noticed. > > */ > > For contributors doing the full work, does that mean the patchset sent to > the list contains the following parts? > > 1. patch 1: list changed files in tests/bios-tables-test-allowed-diff.h. > > 2. patches 2-n: real changes, may contain multiple patches. > > 3. patch n+1: update golden master binaries and empty > tests/bios-tables-test-allowed-diff.h > > Thanks, > > Heyi
Here's a hopefully better patch. Peter does this address the issue? Signed-off-by: Michael S. Tsirkin <m...@redhat.com> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c index f1ac2d7e96..3ab4872bd7 100644 --- a/tests/qtest/bios-tables-test.c +++ b/tests/qtest/bios-tables-test.c @@ -16,7 +16,10 @@ * 1. add empty files for new tables, if any, under tests/data/acpi * 2. list any changed files in tests/bios-tables-test-allowed-diff.h * 3. commit the above *before* making changes that affect the tables - * Maintainer: + * + * Contributor or ACPI Maintainer (steps 4-7 need to be redone to resolve conflicts + * in binary commit created in step 6): + * * After 1-3 above tests will pass but ignore differences with the expected files. * You will also notice that tests/bios-tables-test-allowed-diff.h lists * a bunch of files. This is your hint that you need to do the below: @@ -28,13 +31,23 @@ * output. If not - disassemble them yourself in any way you like. * Look at the differences - make sure they make sense and match what the * changes you are merging are supposed to do. + * Save the changes, preferably in form of ASL diff for the commit log in + * step 6. * * 5. From build directory, run: * $(SRC_PATH)/tests/data/acpi/rebuild-expected-aml.sh - * 6. Now commit any changes. - * 7. Before doing a pull request, make sure tests/bios-tables-test-allowed-diff.h - * is empty - this will ensure following changes to ACPI tables will - * be noticed. + * 6. Now commit any changes to the expected binary, include diff from step 4 + * in commit log. + * 7. Before sending patches to the list (Contributor) + * or before doing a pull request (Maintainer), make sure + * tests/bios-tables-test-allowed-diff.h is empty - this will ensure + * following changes to ACPI tables will be noticed. + * + * The resulting patchset/pull request then looks like this: + * - patch 1: list changed files in tests/bios-tables-test-allowed-diff.h. + * - patches 2 - n: real changes, may contain multiple patches. + * - patch n + 1: update golden master binaries and empty + * tests/bios-tables-test-allowed-diff.h */ #include "qemu/osdep.h" diff --git a/roms/seabios b/roms/seabios index f21b5a4aeb..c9ba5276e3 160000 --- a/roms/seabios +++ b/roms/seabios @@ -1 +1 @@ -Subproject commit f21b5a4aeb020f2a5e2c6503f906a9349dd2f069 +Subproject commit c9ba5276e3217ac6a1ec772dbebf568ba3a8a55d
