| Message ID | 20230206150416.4604-3-farosas@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qtests vs. default devices | expand |
On 6/2/23 16:04, Fabiano Rosas wrote: > The tests are built once for all the targets, so as long as one QEMU > binary is built with CONFIG_LSI_SCSI_PCI=y, this test will > run. However some binaries might not include the device. So check this > again in runtime. > > Signed-off-by: Fabiano Rosas <farosas@suse.de> > --- > tests/qtest/fuzz-lsi53c895a-test.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/tests/qtest/fuzz-lsi53c895a-test.c b/tests/qtest/fuzz-lsi53c895a-test.c > index 392a7ae7ed..a9254b455d 100644 > --- a/tests/qtest/fuzz-lsi53c895a-test.c > +++ b/tests/qtest/fuzz-lsi53c895a-test.c > @@ -112,6 +112,10 @@ static void test_lsi_do_dma_empty_queue(void) > > int main(int argc, char **argv) > { > + if (!qtest_has_device("lsi53c895a")) { > + return 0; > + } Shouldn't we update Kconfig to now add the test unconditionally? Squashing: -- >8 -- diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build index e97616d327..498e49a1b2 100644 --- a/tests/qtest/meson.build +++ b/tests/qtest/meson.build @@ -66,7 +66,6 @@ qtests_i386 = \ (config_all_devices.has_key('CONFIG_RTL8139_PCI') ? ['rtl8139-test'] : []) + \ (config_all_devices.has_key('CONFIG_E1000E_PCI_EXPRESS') ? ['fuzz-e1000e-test'] : []) + \ (config_all_devices.has_key('CONFIG_MEGASAS_SCSI_PCI') ? ['fuzz-megasas-test'] : []) + \ - (config_all_devices.has_key('CONFIG_LSI_SCSI_PCI') ? ['fuzz-lsi53c895a-test'] : []) + \ (config_all_devices.has_key('CONFIG_VIRTIO_SCSI') ? ['fuzz-virtio-scsi-test'] : []) + \ (config_all_devices.has_key('CONFIG_SB16') ? ['fuzz-sb16-test'] : []) + \ (config_all_devices.has_key('CONFIG_SDHCI_PCI') ? ['fuzz-sdcard-test'] : []) + \ @@ -87,6 +86,7 @@ qtests_i386 = \ 'rtc-test', 'i440fx-test', 'fw_cfg-test', + 'fuzz-lsi53c895a-test', 'device-plug-test', 'drive_del-test', 'tco-test', ---
Philippe Mathieu-Daudé <philmd@linaro.org> writes: > On 6/2/23 16:04, Fabiano Rosas wrote: >> The tests are built once for all the targets, so as long as one QEMU >> binary is built with CONFIG_LSI_SCSI_PCI=y, this test will >> run. However some binaries might not include the device. So check this >> again in runtime. >> >> Signed-off-by: Fabiano Rosas <farosas@suse.de> >> --- >> tests/qtest/fuzz-lsi53c895a-test.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/tests/qtest/fuzz-lsi53c895a-test.c b/tests/qtest/fuzz-lsi53c895a-test.c >> index 392a7ae7ed..a9254b455d 100644 >> --- a/tests/qtest/fuzz-lsi53c895a-test.c >> +++ b/tests/qtest/fuzz-lsi53c895a-test.c >> @@ -112,6 +112,10 @@ static void test_lsi_do_dma_empty_queue(void) >> >> int main(int argc, char **argv) >> { >> + if (!qtest_has_device("lsi53c895a")) { >> + return 0; >> + } > > Shouldn't we update Kconfig to now add the test unconditionally? > Squashing: I think we'd still want to not build this test if nothing selected CONFIG_LSI_SCSI_PCI.
On 6/2/23 18:46, Fabiano Rosas wrote: > Philippe Mathieu-Daudé <philmd@linaro.org> writes: > >> On 6/2/23 16:04, Fabiano Rosas wrote: >>> The tests are built once for all the targets, so as long as one QEMU >>> binary is built with CONFIG_LSI_SCSI_PCI=y, this test will >>> run. However some binaries might not include the device. So check this >>> again in runtime. >>> >>> Signed-off-by: Fabiano Rosas <farosas@suse.de> >>> --- >>> tests/qtest/fuzz-lsi53c895a-test.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/tests/qtest/fuzz-lsi53c895a-test.c b/tests/qtest/fuzz-lsi53c895a-test.c >>> index 392a7ae7ed..a9254b455d 100644 >>> --- a/tests/qtest/fuzz-lsi53c895a-test.c >>> +++ b/tests/qtest/fuzz-lsi53c895a-test.c >>> @@ -112,6 +112,10 @@ static void test_lsi_do_dma_empty_queue(void) >>> >>> int main(int argc, char **argv) >>> { >>> + if (!qtest_has_device("lsi53c895a")) { >>> + return 0; >>> + } >> >> Shouldn't we update Kconfig to now add the test unconditionally? (I meant meson.build, not Kconfig). >> Squashing: > > I think we'd still want to not build this test if nothing selected > CONFIG_LSI_SCSI_PCI. Yeah, no need to waste resources building/testing if not available. OTOH the qtest's meson.build is too complicated.
On 06/02/2023 16.04, Fabiano Rosas wrote: > The tests are built once for all the targets, so as long as one QEMU > binary is built with CONFIG_LSI_SCSI_PCI=y, this test will > run. However some binaries might not include the device. So check this > again in runtime. > > Signed-off-by: Fabiano Rosas <farosas@suse.de> > --- > tests/qtest/fuzz-lsi53c895a-test.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/tests/qtest/fuzz-lsi53c895a-test.c b/tests/qtest/fuzz-lsi53c895a-test.c > index 392a7ae7ed..a9254b455d 100644 > --- a/tests/qtest/fuzz-lsi53c895a-test.c > +++ b/tests/qtest/fuzz-lsi53c895a-test.c > @@ -112,6 +112,10 @@ static void test_lsi_do_dma_empty_queue(void) > > int main(int argc, char **argv) > { > + if (!qtest_has_device("lsi53c895a")) { > + return 0; > + } I'm a little bit confused right now ... We're already checking CONFIG_LSI_SCSI_PCI in meson.build, and we're only adding the test to qtests_i386 there ... so how did you end up in a situation where you needed this change? Thomas
Thomas Huth <thuth@redhat.com> writes: > On 06/02/2023 16.04, Fabiano Rosas wrote: >> The tests are built once for all the targets, so as long as one QEMU >> binary is built with CONFIG_LSI_SCSI_PCI=y, this test will >> run. However some binaries might not include the device. So check this >> again in runtime. >> >> Signed-off-by: Fabiano Rosas <farosas@suse.de> >> --- >> tests/qtest/fuzz-lsi53c895a-test.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/tests/qtest/fuzz-lsi53c895a-test.c b/tests/qtest/fuzz-lsi53c895a-test.c >> index 392a7ae7ed..a9254b455d 100644 >> --- a/tests/qtest/fuzz-lsi53c895a-test.c >> +++ b/tests/qtest/fuzz-lsi53c895a-test.c >> @@ -112,6 +112,10 @@ static void test_lsi_do_dma_empty_queue(void) >> >> int main(int argc, char **argv) >> { >> + if (!qtest_has_device("lsi53c895a")) { >> + return 0; >> + } > > I'm a little bit confused right now ... We're already checking > CONFIG_LSI_SCSI_PCI in meson.build, and we're only adding the test to > qtests_i386 there ... so how did you end up in a situation where you needed > this change? I building with --target-list=i386-softmmu,x86_64-softmmu,aarch64-softmmu,arm-softmmu and the arm machines are selecting the CONFIG. That goes into config_all_devices and meson can't tell the difference. In theory that could be try for any CONFIG out there. Here's the output: $ (...) qemu/build/tests/qtest/fuzz-lsi53c895a-test --tap -k # random seed: R02Sfd8259d54ecdb6bdd8187b52e1a921c4 1..2 # Start of i386 tests # Start of fuzz tests # Start of lsi53c895a tests # starting QEMU: exec ./qemu-system-i386 -qtest unix:/tmp/qtest-5629.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-5629.qmp,id=char0 -mon chardev=char0,mode=control -display none -M q35 -nographic -monitor none -serial none -drive if=none,id=drive0,file=null-co://,file.read-zeroes=on,format=raw -device lsi53c895a,id=scsi0 -device scsi-hd,drive=drive0,bus=scsi0.0,channel=0,scsi-id=0,lun=0 -accel qtest qemu-system-i386: -device lsi53c895a,id=scsi0: 'lsi53c895a' is not a valid device model name Broken pipe ../tests/qtest/libqtest.c:181: kill_qemu() tried to terminate QEMU process but encountered exit status 1 (expected 0) Aborted (core dumped) $ find build -name "*.mak" -exec grep -H LSI_SCSI_PCI {} \; ./aarch64-softmmu-config-devices.mak:CONFIG_LSI_SCSI_PCI=y ./arm-softmmu-config-devices.mak:CONFIG_LSI_SCSI_PCI=y hw/arm/Kconfig has: config REALVIEW ... select LSI_SCSI_PCI config VERSATILE ... select LSI_SCSI_PCI
On 07/02/2023 15.02, Fabiano Rosas wrote: > Thomas Huth <thuth@redhat.com> writes: > >> On 06/02/2023 16.04, Fabiano Rosas wrote: >>> The tests are built once for all the targets, so as long as one QEMU >>> binary is built with CONFIG_LSI_SCSI_PCI=y, this test will >>> run. However some binaries might not include the device. So check this >>> again in runtime. >>> >>> Signed-off-by: Fabiano Rosas <farosas@suse.de> >>> --- >>> tests/qtest/fuzz-lsi53c895a-test.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/tests/qtest/fuzz-lsi53c895a-test.c b/tests/qtest/fuzz-lsi53c895a-test.c >>> index 392a7ae7ed..a9254b455d 100644 >>> --- a/tests/qtest/fuzz-lsi53c895a-test.c >>> +++ b/tests/qtest/fuzz-lsi53c895a-test.c >>> @@ -112,6 +112,10 @@ static void test_lsi_do_dma_empty_queue(void) >>> >>> int main(int argc, char **argv) >>> { >>> + if (!qtest_has_device("lsi53c895a")) { >>> + return 0; >>> + } >> >> I'm a little bit confused right now ... We're already checking >> CONFIG_LSI_SCSI_PCI in meson.build, and we're only adding the test to >> qtests_i386 there ... so how did you end up in a situation where you needed >> this change? > > I building with > --target-list=i386-softmmu,x86_64-softmmu,aarch64-softmmu,arm-softmmu > and the arm machines are selecting the CONFIG ... > hw/arm/Kconfig has: > > config REALVIEW > ... > select LSI_SCSI_PCI Ah, right, thanks for the explanation, I missed that some machines have this device built in this way. Ok, patch makes perfectly sense now: Reviewed-by: Thomas Huth <thuth@redhat.com>
diff --git a/tests/qtest/fuzz-lsi53c895a-test.c b/tests/qtest/fuzz-lsi53c895a-test.c index 392a7ae7ed..a9254b455d 100644 --- a/tests/qtest/fuzz-lsi53c895a-test.c +++ b/tests/qtest/fuzz-lsi53c895a-test.c @@ -112,6 +112,10 @@ static void test_lsi_do_dma_empty_queue(void) int main(int argc, char **argv) { + if (!qtest_has_device("lsi53c895a")) { + return 0; + } + g_test_init(&argc, &argv, NULL); qtest_add_func("fuzz/lsi53c895a/lsi_do_dma_empty_queue",
The tests are built once for all the targets, so as long as one QEMU binary is built with CONFIG_LSI_SCSI_PCI=y, this test will run. However some binaries might not include the device. So check this again in runtime. Signed-off-by: Fabiano Rosas <farosas@suse.de> --- tests/qtest/fuzz-lsi53c895a-test.c | 4 ++++ 1 file changed, 4 insertions(+)