Re: [PATCH for-9.0 0/3] qtest/virtio-9p-test.c: fix slow tests
On Tuesday, March 26, 2024 5:07:16 PM CET Daniel Henrique Barboza wrote:
>
> On 3/26/24 12:55, Greg Kurz wrote:
> > Bom dia Daniel !
>
> Bonne après-midi !
>
> >
> > On Tue, 26 Mar 2024 10:26:03 -0300
> > Daniel Henrique Barboza wrote:
> >
> >> Hi,
> >>
> >> Thomas reported in [1] a problem that happened with the RISC-V machine
> >> where some tests from virtio-9p-test.c were failing with '-m slow', i.e.
> >> enabling slow tests.
> >>
> >> In the end it wasn't a RISC-V specific problem. It just so happens that
> >> the recently added riscv machine nodes runs the tests from
> >> virtio-9p-test two times for each qos-test run: one with the
> >> virtio-9p-device device and another with the virtio-9p-pci. The temp dir
> >> for these tests is being created at the start of qos-test and removed
> >> only at the end of qos-test, and the tests are leaving dirs and files
> >> behind. virtio-9-device tests run first, creates stuff in the temp dir,
> >> then when virtio-9p-pci tests runs again it'll fail because the previous
> >> run left created dirs and files in the same temp dir. Here's a run that
> >> exemplifies the problem:
> >>
> >> $ MALLOC_PERTURB_=21 V=2 QTEST_QEMU_BINARY=./qemu-system-riscv64
> >> ./tests/qtest/qos-test -m slow
> >> (...)
> >> # starting QEMU: exec ./qemu-system-riscv64 -qtest
> >> unix:/tmp/qtest-621710.sock -qtest-log /dev/null -chardev
> >> socket,path=/tmp/qtest-621710.qmp,id=char0 -mon chardev=char0,mode=control
> >> -display none -audio none -M virt,aclint=on,aia=aplic-imsic -fsdev
> >> local,id=fsdev0,path='/home/danielhb/work/qemu/build/qtest-9p-local-7E16K2',security_model=mapped-xattr
> >> -device virtio-9p-device,fsdev=fsdev0,mount_tag=qtest -accel qtest
> >> ( goes ok ...)
> >> # starting QEMU: exec ./qemu-system-riscv64 -qtest
> >> unix:/tmp/qtest-621710.sock -qtest-log /dev/null -chardev
> >> socket,path=/tmp/qtest-621710.qmp,id=char0 -mon chardev=char0,mode=control
> >> -display none -audio none -M virt,aclint=on,aia=aplic-imsic -fsdev
> >> local,id=fsdev0,path='/home/danielhb/work/qemu/build/qtest-9p-local-7E16K2',security_model=mapped-xattr
> >> -device virtio-9p-pci,fsdev=fsdev0,addr=04.0,mount_tag=qtest -accel qtest
> >> ok 168
> >> /riscv64/virt/generic-pcihost/pci-bus-generic/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/local/config
> >> Received response 7 (RLERROR) instead of 73 (RMKDIR)
> >> Rlerror has errno 17 (File exists)
> >> **
> >> ERROR:../tests/qtest/libqos/virtio-9p-client.c:275:v9fs_req_recv:
> >> assertion failed (hdr.id == id): (7 == 73)
> >>
> >> As we can see we're running both 'virtio-9p-device' tests and
> >> 'virtio-9p-pci'
> >> tests using the same '/home/danielhb/work/qemu/build/qtest-9p-local-7E16K2'
> >> temp dir.
> >>
> >
> >
> > Good catch ! I'll try to find some time to review.
> >
> >> The quick fix I came up with was to make each test clean themselves up
> >> after each run. The tests were also consolidated, i.e. fewer tests with the
> >> same coverage, because the 'unlikat' tests were doing the same thing the
> >> 'create' tests were doing but removing stuff after. Might as well keep just
> >> the 'unlikat' tests.
> >>
> >
> > As long as coverage is preserved, I'm fine with consolidation of the
> > checks. In any case, last call goes to Christian.
> >
> >> I also went ahead and reverted 558f5c42efd ("tests/9pfs: Mark "local"
> >> tests as "slow"") after realizing that the problem I was fixing is also
> >> the same problem that this patch was trying to working around with the
> >> skip [2]. I validated this change in this Gitlab pipeline:
> >>
> >
> > Are you sure with that ? Issues look very similar indeed but not
> > exactly the same.
>
> We can skip this revert if we're not sure about it. Gitlab passed with it but
> perhaps this isn't evidence enough. I'll let you guys decide.
I am a bit surprised because errnos were different (file exists vs. not
supported), but indeed, it did pass in your Gitlab pipeline. So I am fine with
bringing those tests back in on Gitlab.
/Christian
Re: [PATCH for-9.0 0/3] qtest/virtio-9p-test.c: fix slow tests
On 26/03/2024 14.26, Daniel Henrique Barboza wrote:
Hi,
Thomas reported in [1] a problem that happened with the RISC-V machine
where some tests from virtio-9p-test.c were failing with '-m slow', i.e.
enabling slow tests.
In the end it wasn't a RISC-V specific problem. It just so happens that
the recently added riscv machine nodes runs the tests from
virtio-9p-test two times for each qos-test run: one with the
virtio-9p-device device and another with the virtio-9p-pci. The temp dir
for these tests is being created at the start of qos-test and removed
only at the end of qos-test, and the tests are leaving dirs and files
behind. virtio-9-device tests run first, creates stuff in the temp dir,
then when virtio-9p-pci tests runs again it'll fail because the previous
run left created dirs and files in the same temp dir. Here's a run that
exemplifies the problem:
$ MALLOC_PERTURB_=21 V=2 QTEST_QEMU_BINARY=./qemu-system-riscv64
./tests/qtest/qos-test -m slow
(...)
# starting QEMU: exec ./qemu-system-riscv64 -qtest unix:/tmp/qtest-621710.sock
-qtest-log /dev/null -chardev socket,path=/tmp/qtest-621710.qmp,id=char0 -mon
chardev=char0,mode=control -display none -audio none -M
virt,aclint=on,aia=aplic-imsic -fsdev
local,id=fsdev0,path='/home/danielhb/work/qemu/build/qtest-9p-local-7E16K2',security_model=mapped-xattr
-device virtio-9p-device,fsdev=fsdev0,mount_tag=qtest -accel qtest
( goes ok ...)
# starting QEMU: exec ./qemu-system-riscv64 -qtest unix:/tmp/qtest-621710.sock
-qtest-log /dev/null -chardev socket,path=/tmp/qtest-621710.qmp,id=char0 -mon
chardev=char0,mode=control -display none -audio none -M
virt,aclint=on,aia=aplic-imsic -fsdev
local,id=fsdev0,path='/home/danielhb/work/qemu/build/qtest-9p-local-7E16K2',security_model=mapped-xattr
-device virtio-9p-pci,fsdev=fsdev0,addr=04.0,mount_tag=qtest -accel qtest
ok 168
/riscv64/virt/generic-pcihost/pci-bus-generic/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/local/config
Received response 7 (RLERROR) instead of 73 (RMKDIR)
Rlerror has errno 17 (File exists)
**
ERROR:../tests/qtest/libqos/virtio-9p-client.c:275:v9fs_req_recv: assertion
failed (hdr.id == id): (7 == 73)
As we can see we're running both 'virtio-9p-device' tests and 'virtio-9p-pci'
tests using the same '/home/danielhb/work/qemu/build/qtest-9p-local-7E16K2'
temp dir.
The quick fix I came up with was to make each test clean themselves up
after each run. The tests were also consolidated, i.e. fewer tests with the
same coverage, because the 'unlikat' tests were doing the same thing the
'create' tests were doing but removing stuff after. Might as well keep just
the 'unlikat' tests.
I also went ahead and reverted 558f5c42efd ("tests/9pfs: Mark "local"
tests as "slow"") after realizing that the problem I was fixing is also
the same problem that this patch was trying to working around with the
skip [2]. I validated this change in this Gitlab pipeline:
https://gitlab.com/danielhb/qemu/-/pipelines/1227953967
[1] https://mail.gnu.org/archive/html/qemu-devel/2024-03/msg05807.html
[2] https://lists.nongnu.org/archive/html/qemu-devel/2020-11/msg05510.html
Daniel Henrique Barboza (3):
qtest/virtio-9p-test.c: consolidate create dir, file and symlink tests
qtest/virtio-9p-test.c: consolidate hardlink tests
Thanks, these fix the "make check SPEED=slow" problems for me!
Tested-by: Thomas Huth
Re: [PATCH for-9.0 0/3] qtest/virtio-9p-test.c: fix slow tests
On 3/26/24 12:55, Greg Kurz wrote:
Bom dia Daniel !
Bonne après-midi !
On Tue, 26 Mar 2024 10:26:03 -0300
Daniel Henrique Barboza wrote:
Hi,
Thomas reported in [1] a problem that happened with the RISC-V machine
where some tests from virtio-9p-test.c were failing with '-m slow', i.e.
enabling slow tests.
In the end it wasn't a RISC-V specific problem. It just so happens that
the recently added riscv machine nodes runs the tests from
virtio-9p-test two times for each qos-test run: one with the
virtio-9p-device device and another with the virtio-9p-pci. The temp dir
for these tests is being created at the start of qos-test and removed
only at the end of qos-test, and the tests are leaving dirs and files
behind. virtio-9-device tests run first, creates stuff in the temp dir,
then when virtio-9p-pci tests runs again it'll fail because the previous
run left created dirs and files in the same temp dir. Here's a run that
exemplifies the problem:
$ MALLOC_PERTURB_=21 V=2 QTEST_QEMU_BINARY=./qemu-system-riscv64
./tests/qtest/qos-test -m slow
(...)
# starting QEMU: exec ./qemu-system-riscv64 -qtest unix:/tmp/qtest-621710.sock
-qtest-log /dev/null -chardev socket,path=/tmp/qtest-621710.qmp,id=char0 -mon
chardev=char0,mode=control -display none -audio none -M
virt,aclint=on,aia=aplic-imsic -fsdev
local,id=fsdev0,path='/home/danielhb/work/qemu/build/qtest-9p-local-7E16K2',security_model=mapped-xattr
-device virtio-9p-device,fsdev=fsdev0,mount_tag=qtest -accel qtest
( goes ok ...)
# starting QEMU: exec ./qemu-system-riscv64 -qtest unix:/tmp/qtest-621710.sock
-qtest-log /dev/null -chardev socket,path=/tmp/qtest-621710.qmp,id=char0 -mon
chardev=char0,mode=control -display none -audio none -M
virt,aclint=on,aia=aplic-imsic -fsdev
local,id=fsdev0,path='/home/danielhb/work/qemu/build/qtest-9p-local-7E16K2',security_model=mapped-xattr
-device virtio-9p-pci,fsdev=fsdev0,addr=04.0,mount_tag=qtest -accel qtest
ok 168
/riscv64/virt/generic-pcihost/pci-bus-generic/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/local/config
Received response 7 (RLERROR) instead of 73 (RMKDIR)
Rlerror has errno 17 (File exists)
**
ERROR:../tests/qtest/libqos/virtio-9p-client.c:275:v9fs_req_recv: assertion
failed (hdr.id == id): (7 == 73)
As we can see we're running both 'virtio-9p-device' tests and 'virtio-9p-pci'
tests using the same '/home/danielhb/work/qemu/build/qtest-9p-local-7E16K2'
temp dir.
Good catch ! I'll try to find some time to review.
The quick fix I came up with was to make each test clean themselves up
after each run. The tests were also consolidated, i.e. fewer tests with the
same coverage, because the 'unlikat' tests were doing the same thing the
'create' tests were doing but removing stuff after. Might as well keep just
the 'unlikat' tests.
As long as coverage is preserved, I'm fine with consolidation of the
checks. In any case, last call goes to Christian.
I also went ahead and reverted 558f5c42efd ("tests/9pfs: Mark "local"
tests as "slow"") after realizing that the problem I was fixing is also
the same problem that this patch was trying to working around with the
skip [2]. I validated this change in this Gitlab pipeline:
Are you sure with that ? Issues look very similar indeed but not
exactly the same.
We can skip this revert if we're not sure about it. Gitlab passed with it but
perhaps this isn't evidence enough. I'll let you guys decide.
Thanks,
Daniel
Cheers,
--
Greg
https://gitlab.com/danielhb/qemu/-/pipelines/1227953967
[1] https://mail.gnu.org/archive/html/qemu-devel/2024-03/msg05807.html
[2] https://lists.nongnu.org/archive/html/qemu-devel/2020-11/msg05510.html
Daniel Henrique Barboza (3):
qtest/virtio-9p-test.c: consolidate create dir, file and symlink tests
qtest/virtio-9p-test.c: consolidate hardlink tests
qtest/virtio-9p-test.c: remove g_test_slow() gate
tests/qtest/virtio-9p-test.c | 155 +++
1 file changed, 48 insertions(+), 107 deletions(-)
Re: [PATCH for-9.0 0/3] qtest/virtio-9p-test.c: fix slow tests
Bom dia Daniel !
On Tue, 26 Mar 2024 10:26:03 -0300
Daniel Henrique Barboza wrote:
> Hi,
>
> Thomas reported in [1] a problem that happened with the RISC-V machine
> where some tests from virtio-9p-test.c were failing with '-m slow', i.e.
> enabling slow tests.
>
> In the end it wasn't a RISC-V specific problem. It just so happens that
> the recently added riscv machine nodes runs the tests from
> virtio-9p-test two times for each qos-test run: one with the
> virtio-9p-device device and another with the virtio-9p-pci. The temp dir
> for these tests is being created at the start of qos-test and removed
> only at the end of qos-test, and the tests are leaving dirs and files
> behind. virtio-9-device tests run first, creates stuff in the temp dir,
> then when virtio-9p-pci tests runs again it'll fail because the previous
> run left created dirs and files in the same temp dir. Here's a run that
> exemplifies the problem:
>
> $ MALLOC_PERTURB_=21 V=2 QTEST_QEMU_BINARY=./qemu-system-riscv64
> ./tests/qtest/qos-test -m slow
> (...)
> # starting QEMU: exec ./qemu-system-riscv64 -qtest
> unix:/tmp/qtest-621710.sock -qtest-log /dev/null -chardev
> socket,path=/tmp/qtest-621710.qmp,id=char0 -mon chardev=char0,mode=control
> -display none -audio none -M virt,aclint=on,aia=aplic-imsic -fsdev
> local,id=fsdev0,path='/home/danielhb/work/qemu/build/qtest-9p-local-7E16K2',security_model=mapped-xattr
> -device virtio-9p-device,fsdev=fsdev0,mount_tag=qtest -accel qtest
> ( goes ok ...)
> # starting QEMU: exec ./qemu-system-riscv64 -qtest
> unix:/tmp/qtest-621710.sock -qtest-log /dev/null -chardev
> socket,path=/tmp/qtest-621710.qmp,id=char0 -mon chardev=char0,mode=control
> -display none -audio none -M virt,aclint=on,aia=aplic-imsic -fsdev
> local,id=fsdev0,path='/home/danielhb/work/qemu/build/qtest-9p-local-7E16K2',security_model=mapped-xattr
> -device virtio-9p-pci,fsdev=fsdev0,addr=04.0,mount_tag=qtest -accel qtest
> ok 168
> /riscv64/virt/generic-pcihost/pci-bus-generic/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/local/config
> Received response 7 (RLERROR) instead of 73 (RMKDIR)
> Rlerror has errno 17 (File exists)
> **
> ERROR:../tests/qtest/libqos/virtio-9p-client.c:275:v9fs_req_recv: assertion
> failed (hdr.id == id): (7 == 73)
>
> As we can see we're running both 'virtio-9p-device' tests and 'virtio-9p-pci'
> tests using the same '/home/danielhb/work/qemu/build/qtest-9p-local-7E16K2'
> temp dir.
>
Good catch ! I'll try to find some time to review.
> The quick fix I came up with was to make each test clean themselves up
> after each run. The tests were also consolidated, i.e. fewer tests with the
> same coverage, because the 'unlikat' tests were doing the same thing the
> 'create' tests were doing but removing stuff after. Might as well keep just
> the 'unlikat' tests.
>
As long as coverage is preserved, I'm fine with consolidation of the
checks. In any case, last call goes to Christian.
> I also went ahead and reverted 558f5c42efd ("tests/9pfs: Mark "local"
> tests as "slow"") after realizing that the problem I was fixing is also
> the same problem that this patch was trying to working around with the
> skip [2]. I validated this change in this Gitlab pipeline:
>
Are you sure with that ? Issues look very similar indeed but not
exactly the same.
Cheers,
--
Greg
> https://gitlab.com/danielhb/qemu/-/pipelines/1227953967
>
> [1] https://mail.gnu.org/archive/html/qemu-devel/2024-03/msg05807.html
> [2] https://lists.nongnu.org/archive/html/qemu-devel/2020-11/msg05510.html
>
> Daniel Henrique Barboza (3):
> qtest/virtio-9p-test.c: consolidate create dir, file and symlink tests
> qtest/virtio-9p-test.c: consolidate hardlink tests
> qtest/virtio-9p-test.c: remove g_test_slow() gate
>
> tests/qtest/virtio-9p-test.c | 155 +++
> 1 file changed, 48 insertions(+), 107 deletions(-)
>
