On Wed, Jun 28, 2023 at 12:05:46PM +0530, Ani Sinha wrote: > > > > On 26-Jun-2023, at 6:49 PM, Michael S. Tsirkin <[email protected]> wrote: > > > > On Mon, Jun 26, 2023 at 06:33:14PM +0530, Ani Sinha wrote: > >> > >> > >>> On 26-Jun-2023, at 6:28 PM, Michael S. Tsirkin <[email protected]> wrote: > >>> > >>> On Mon, May 22, 2023 at 04:00:39PM +0530, Ani Sinha wrote: > >>>> Currently the meson based QEMU build process locates the iasl binary > >>>> from the > >>>> current PATH and other locations [1] and uses that to set CONFIG_IASL in > >>>> config-host.h header.This is then used at compile time by > >>>> bios-tables-test to > >>>> set iasl path. > >>>> > >>>> This has two disadvantages: > >>>> - If iasl was not previously installed in the PATH, one has to install > >>>> iasl > >>>> and rebuild QEMU in order to regenerate the header and pick up the found > >>>> iasl location. One cannot simply use the existing bios-tables-test > >>>> binary > >>>> because CONFIG_IASL is only set during the QEMU build time by meson and > >>>> then bios-tables-test has to be rebuilt with CONFIG_IASL set in order to > >>>> use iasl. > >>>> - Sometimes, the stock iasl that comes with distributions is simply not > >>>> good > >>>> enough because it does not support the latest ACPI changes - newly > >>>> introduced tables or new table attributes etc. In order to test ACPI > >>>> code > >>>> in QEMU, one has to clone the latest acpica upstream repository and > >>>> rebuild iasl in order to get support for it. In those cases, one may > >>>> want > >>>> the test to use the iasl binary from a non-standard location. > >>>> > >>>> In order to overcome the above two disadvantages, we set a default iasl > >>>> path > >>>> as "/usr/bin/iasl". bios-tables-test also checks for the environment > >>>> variable > >>>> IASL_PATH that can be set by the developer. IASL_PATH passed from the > >>>> environment overrides the default path. This way developers can point > >>>> IASL_PATH environment variable to a possibly a non-standard custom build > >>>> binary and quickly run bios-tables-test without rebuilding. If the > >>>> default > >>>> path of iasl changes, one simply needs to update the default path and > >>>> rebuild > >>>> just the test, not whole QEMU. > >>>> > >>>> [1] https://mesonbuild.com/Reference-manual_functions.html#find_program > >>>> > >>>> CC: [email protected] > >>>> CC: [email protected] > >>>> Signed-off-by: Ani Sinha <[email protected]> > >>> > >>> I don't much like environment variables since they are > >>> not discoverable. > >> > >> I do have this: > >> > >> + " Set IASL_PATH environment variable to the path of iasl > >> binary\n" > >> + " if iasl is installed somewhere other than %s.\n", > > > > You only see this if there's a diff. > > > > And then people stick this in their scripts and are scratching their > > heads trying to figure out why is a wrong iasl running. Or someone > > comes up with a different use for IASL_PATH and they conflict. > > OK in that case I think its ok to simply remove the environment > variable part. If people are going to be changing a header file,
Not people. configure script > they > might as well change the DEFAULT_IASL_PATH in the test itself where > its easier to find. What additional complication meson provides is > that it uses find_program() to find the IASL binary in a list of > predefined locations. I do not think this additional tie up with meson > is worth it for the niche iasl use case. Simple is beautiful. The just the below then? And we can let it be? Signed-off-by: Michael S. Tsirkin <[email protected]> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c index ed1c69cf01..d0e1655d2e 100644 --- a/tests/qtest/bios-tables-test.c +++ b/tests/qtest/bios-tables-test.c @@ -102,6 +102,7 @@ typedef struct { static char disk[] = "tests/acpi-test-disk-XXXXXX"; static const char *data_dir = "tests/data/acpi"; +/* If you want your own path, change the below to iasl = "/home/usr/bin/iasl" */ #ifdef CONFIG_IASL static const char *iasl = CONFIG_IASL; #else
