"Michael S. Tsirkin" <m...@redhat.com> writes:
> On Wed, May 17, 2023 at 04:58:06PM +0100, Alex Bennée wrote: >> >> "Michael S. Tsirkin" <m...@redhat.com> writes: >> >> > On Wed, May 17, 2023 at 04:16:47PM +0100, Alex Bennée wrote: >> >> >> >> Ani Sinha <anisi...@redhat.com> writes: >> >> >> >> >> On 17-May-2023, at 8:06 PM, Michael S. Tsirkin <m...@redhat.com> wrote: >> >> >> >> >> >> On Wed, May 17, 2023 at 07:57:53PM +0530, Ani Sinha wrote: >> >> >>> >> >> >>> >> >> >>>> On 17-May-2023, at 7:47 PM, Michael S. Tsirkin <m...@redhat.com> >> >> >>>> wrote: >> >> >>>> >> >> >>>> On Wed, May 17, 2023 at 05:37:51PM +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 which is >> >> >>>>> then used by the test. >> >> >>>>> >> >> >>>>> This has two disadvantages: >> >> >>>>> - If iasl was not previously installed in the PATH, one has to >> >> >>>>> install iasl >> >> >>>>> and rebuild QEMU in order to pick up the 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. >> >> >> >> Usually we work the other way by checking at configure time and skipping >> >> the feature if the prerequisites are not in place. We do this with gdb: >> >> >> >> ../../configure >> >> --gdb=/home/alex/src/tools/binutils-gdb.git/builds/all/install/bin/gdb >> >> >> >> which checks gdb is at least new enough to support the features we need: >> >> >> >> if test -n "$gdb_bin"; then >> >> gdb_version=$($gdb_bin --version | head -n 1) >> >> if version_ge ${gdb_version##* } 9.1; then >> >> echo "HAVE_GDB_BIN=$gdb_bin" >> $config_host_mak >> >> gdb_arches=$("$source_path/scripts/probe-gdb-support.py" >> >> $gdb_bin) >> >> else >> >> gdb_bin="" >> >> fi >> >> fi >> >> >> >> >>>>> - 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. >> >> >> >> I think configure should be checking if iasl is new enough and reporting >> >> to the user at configure time they need to do something different. We >> >> don't want to attempt to run tests that will fail unless the user has >> >> added the right magic to their environment. >> > >> > iasl is a disassembler we trigger for user convenience in case tests >> > fail. It will never cause tests to fail. >> >> Fair enough. But I still think the place to report it is in configure. >> Maybe something like: >> >> iasl : /usr/bin/iasl (version 20200925, might >> not handle all ACPI) >> >> in the Host Binaries section. Re-configuring shouldn't cause too much of >> the build to be regenerated although we could certainly do better in >> this regard. > > won't all of it be regenerated? a header everyone includes changes. Ahh I see meson is doing: if iasl.found() config_host_data.set_quoted('CONFIG_IASL', iasl.full_path()) endif which causes the inclusion in config-host.h - this seems excessive. It would seem simpler to get meson to apply CONFIG_IASL to the invocation of bios-tables-test rather than embedding it in the binary, e.g.: ./tests/bios-tables-test --iasl-path ${CONFIG_IASL} and then you have the best of both worlds. You can run manually with a different path and you don't need to pollute config-host.h Paolo, I see we expand all the qtests with: test('qtest-@0@/@1@'.format(target_base, test), qtest_executables[test], depends: [test_deps, qtest_emulator, emulator_modules], env: qtest_env, args: ['--tap', '-k'], protocol: 'tap', timeout: slow_qtests.get(test, 30), priority: slow_qtests.get(test, 30), suite: ['qtest', 'qtest-' + target_base]) is there any easy way to add arguments to individual tests or do we need an explicit test expansion for bios-tables-test? -- Alex Bennée Virtualisation Tech Lead @ Linaro