| Message ID | 20220705153708.186418-1-vsementsov@yandex-team.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iotests: fix copy-before-write for macOS and FreeBSD | expand |
On 7/5/22 21:07, Vladimir Sementsov-Ogievskiy wrote: > strerror() represents ETIMEDOUT a bit different in Linux and macOS / > FreeBSD. Let's support the latter too. > > Fixes: 9d05a87b77 ("iotests: copy-before-write: add cases for cbw-timeout option") > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> > --- > > As John and Thomas noted, the new iotests fails for FreeBSD and maxOS. > Here is a fix. Would be great if someone can test it. > > I tried to push it by > > git push --force -o ci.variable="QEMU_CI=1" > > to my block branch, I get a blocked pipeline > https://gitlab.com/vsementsov/qemu/-/pipelines/580573238 > but it doesn't have neither freebsd nor macos jobs.. How to get them? You'd have to have an attached cirrus token. Better to just use 'make vm-build-freebsd'. > def test_timeout_break_guest(self): > log = self.do_cbw_timeout('break-guest-write') > + # macOS and FreeBSD tend to represent ETIMEDOUT as > + # "Operation timed out", when Linux prefer > + # "Connection timed out" > + log = log.replace('Operation timed out', > + 'Connection timed out') Um, really? Matching strerror text? This is ultra-fragile. Dare I say broken already. r~
On 7/5/22 20:22, Richard Henderson wrote: > On 7/5/22 21:07, Vladimir Sementsov-Ogievskiy wrote: >> strerror() represents ETIMEDOUT a bit different in Linux and macOS / >> FreeBSD. Let's support the latter too. >> >> Fixes: 9d05a87b77 ("iotests: copy-before-write: add cases for cbw-timeout option") >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> >> --- >> >> As John and Thomas noted, the new iotests fails for FreeBSD and maxOS. >> Here is a fix. Would be great if someone can test it. >> >> I tried to push it by >> >> git push --force -o ci.variable="QEMU_CI=1" >> >> to my block branch, I get a blocked pipeline >> https://gitlab.com/vsementsov/qemu/-/pipelines/580573238 >> but it doesn't have neither freebsd nor macos jobs.. How to get them? > > You'd have to have an attached cirrus token. > Better to just use 'make vm-build-freebsd'. > >> def test_timeout_break_guest(self): >> log = self.do_cbw_timeout('break-guest-write') >> + # macOS and FreeBSD tend to represent ETIMEDOUT as >> + # "Operation timed out", when Linux prefer >> + # "Connection timed out" >> + log = log.replace('Operation timed out', >> + 'Connection timed out') > > Um, really? Matching strerror text? This is ultra-fragile. > Dare I say broken already. > Unfortunately we lack a good interface to test simple reads and writes in iotests. qemu-io command just print the result to stdout. So, I think, in short-term, this is the best thing to do. And if just `git grep '\(write\|read\) failed:' tests/qemu-iotests/` we'll see that this is the common practice.
On 05/07/2022 17.37, Vladimir Sementsov-Ogievskiy wrote: > strerror() represents ETIMEDOUT a bit different in Linux and macOS / > FreeBSD. Let's support the latter too. > > Fixes: 9d05a87b77 ("iotests: copy-before-write: add cases for cbw-timeout option") > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> > --- > > As John and Thomas noted, the new iotests fails for FreeBSD and maxOS. > Here is a fix. Would be great if someone can test it. Thanks, seems to work now: https://gitlab.com/thuth/qemu/-/jobs/2683487160#L3256 https://gitlab.com/thuth/qemu/-/jobs/2683487162#L2897 Tested-by: Thomas Huth <thuth@redhat.com> > I tried to push it by > > git push --force -o ci.variable="QEMU_CI=1" > > to my block branch, I get a blocked pipeline > https://gitlab.com/vsementsov/qemu/-/pipelines/580573238 > but it doesn't have neither freebsd nor macos jobs.. How to get them? The instructions are a little bit hidden - you can find them in the .gitlab-ci.d/cirrus/README.rst file in your git checkout. HTH, Thomas
On 05/07/2022 19.22, Richard Henderson wrote: > On 7/5/22 21:07, Vladimir Sementsov-Ogievskiy wrote: >> strerror() represents ETIMEDOUT a bit different in Linux and macOS / >> FreeBSD. Let's support the latter too. >> >> Fixes: 9d05a87b77 ("iotests: copy-before-write: add cases for cbw-timeout >> option") >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> >> --- >> >> As John and Thomas noted, the new iotests fails for FreeBSD and maxOS. >> Here is a fix. Would be great if someone can test it. >> >> I tried to push it by >> >> git push --force -o ci.variable="QEMU_CI=1" >> >> to my block branch, I get a blocked pipeline >> https://gitlab.com/vsementsov/qemu/-/pipelines/580573238 >> but it doesn't have neither freebsd nor macos jobs.. How to get them? > > You'd have to have an attached cirrus token. > Better to just use 'make vm-build-freebsd'. > >> def test_timeout_break_guest(self): >> log = self.do_cbw_timeout('break-guest-write') >> + # macOS and FreeBSD tend to represent ETIMEDOUT as >> + # "Operation timed out", when Linux prefer >> + # "Connection timed out" >> + log = log.replace('Operation timed out', >> + 'Connection timed out') > > Um, really? Matching strerror text? This is ultra-fragile. > Dare I say broken already. Many of the iotests rely on output text matching. It's very fragile, always has been and always will be (unless we rewrite the whole test suite to not use output text matching anymore). For example, the iotests also do not work with the libc from Alpine Linux (where one of the error messages is "I/O error" instead of "Input/Output error"), so we only run check-unit and check-qtest in the CI there. It's a pity, but that's the way it is currently. I think it's still better to tweak some of the text strings here instead of not running the tests at all. Thomas
On Wed, 6 Jul 2022 at 08:39, Thomas Huth <thuth@redhat.com> wrote: > Many of the iotests rely on output text matching. It's very fragile, always > has been and always will be (unless we rewrite the whole test suite to not > use output text matching anymore). Maybe you could have a pre-pass over the "expected results" files that substituted in the strerror text for the host it was running on. That's probably a reasonably doable amount of work compared to a complete testsuite rewrite. -- PMM
On 05.07.22 17:37, Vladimir Sementsov-Ogievskiy wrote: > strerror() represents ETIMEDOUT a bit different in Linux and macOS / > FreeBSD. Let's support the latter too. > > Fixes: 9d05a87b77 ("iotests: copy-before-write: add cases for cbw-timeout option") > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> > --- > > As John and Thomas noted, the new iotests fails for FreeBSD and maxOS. > Here is a fix. Would be great if someone can test it. > > I tried to push it by > > git push --force -o ci.variable="QEMU_CI=1" > > to my block branch, I get a blocked pipeline > https://gitlab.com/vsementsov/qemu/-/pipelines/580573238 > but it doesn't have neither freebsd nor macos jobs.. How to get them? > > tests/qemu-iotests/tests/copy-before-write | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/tests/qemu-iotests/tests/copy-before-write b/tests/qemu-iotests/tests/copy-before-write > index 16efebbf8f..56937b9dff 100755 > --- a/tests/qemu-iotests/tests/copy-before-write > +++ b/tests/qemu-iotests/tests/copy-before-write > @@ -192,6 +192,11 @@ read 1048576/1048576 bytes at offset 0 > > def test_timeout_break_guest(self): > log = self.do_cbw_timeout('break-guest-write') > + # macOS and FreeBSD tend to represent ETIMEDOUT as > + # "Operation timed out", when Linux prefer > + # "Connection timed out" > + log = log.replace('Operation timed out', > + 'Connection timed out') If we know for sure that it’s ETIMEDOUT, how about os.strerror(errno.ETIMEDOUT)? Hanna > self.assertEqual(log, """\ > wrote 524288/524288 bytes at offset 0 > 512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
On 7/6/22 13:26, Hanna Reitz wrote: > On 05.07.22 17:37, Vladimir Sementsov-Ogievskiy wrote: >> strerror() represents ETIMEDOUT a bit different in Linux and macOS / >> FreeBSD. Let's support the latter too. >> >> Fixes: 9d05a87b77 ("iotests: copy-before-write: add cases for cbw-timeout option") >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> >> --- >> >> As John and Thomas noted, the new iotests fails for FreeBSD and maxOS. >> Here is a fix. Would be great if someone can test it. >> >> I tried to push it by >> >> git push --force -o ci.variable="QEMU_CI=1" >> >> to my block branch, I get a blocked pipeline >> https://gitlab.com/vsementsov/qemu/-/pipelines/580573238 >> but it doesn't have neither freebsd nor macos jobs.. How to get them? >> >> tests/qemu-iotests/tests/copy-before-write | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/tests/qemu-iotests/tests/copy-before-write b/tests/qemu-iotests/tests/copy-before-write >> index 16efebbf8f..56937b9dff 100755 >> --- a/tests/qemu-iotests/tests/copy-before-write >> +++ b/tests/qemu-iotests/tests/copy-before-write >> @@ -192,6 +192,11 @@ read 1048576/1048576 bytes at offset 0 >> def test_timeout_break_guest(self): >> log = self.do_cbw_timeout('break-guest-write') >> + # macOS and FreeBSD tend to represent ETIMEDOUT as >> + # "Operation timed out", when Linux prefer >> + # "Connection timed out" >> + log = log.replace('Operation timed out', >> + 'Connection timed out') > > If we know for sure that it’s ETIMEDOUT, how about os.strerror(errno.ETIMEDOUT)? > Good idea, will try.
On 7/6/22 13:26, Hanna Reitz wrote: > On 05.07.22 17:37, Vladimir Sementsov-Ogievskiy wrote: >> strerror() represents ETIMEDOUT a bit different in Linux and macOS / >> FreeBSD. Let's support the latter too. >> >> Fixes: 9d05a87b77 ("iotests: copy-before-write: add cases for cbw-timeout option") >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> >> --- >> >> As John and Thomas noted, the new iotests fails for FreeBSD and maxOS. >> Here is a fix. Would be great if someone can test it. >> >> I tried to push it by >> >> git push --force -o ci.variable="QEMU_CI=1" >> >> to my block branch, I get a blocked pipeline >> https://gitlab.com/vsementsov/qemu/-/pipelines/580573238 >> but it doesn't have neither freebsd nor macos jobs.. How to get them? >> >> tests/qemu-iotests/tests/copy-before-write | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/tests/qemu-iotests/tests/copy-before-write b/tests/qemu-iotests/tests/copy-before-write >> index 16efebbf8f..56937b9dff 100755 >> --- a/tests/qemu-iotests/tests/copy-before-write >> +++ b/tests/qemu-iotests/tests/copy-before-write >> @@ -192,6 +192,11 @@ read 1048576/1048576 bytes at offset 0 >> def test_timeout_break_guest(self): >> log = self.do_cbw_timeout('break-guest-write') >> + # macOS and FreeBSD tend to represent ETIMEDOUT as >> + # "Operation timed out", when Linux prefer >> + # "Connection timed out" >> + log = log.replace('Operation timed out', >> + 'Connection timed out') > > If we know for sure that it’s ETIMEDOUT, how about os.strerror(errno.ETIMEDOUT)? I've checked this with make vm-build-freebsd, but it doesn't work: --- /home/qemu/qemu-test.fxwm16/src/tests/qemu-iotests/tests/copy-before-write.out +++ /usr/home/qemu/qemu-test.fxwm16/build/tests/qemu-iotests/scratch/copy-before-write.out.bad @@ -1,5 +1,22 @@ -.... +..F. +====================================================================== +FAIL: test_timeout_break_guest (__main__.TestCbwError) +---------------------------------------------------------------------- +Traceback (most recent call last): + File "/usr/home/qemu/qemu-test.fxwm16/src/tests/qemu-iotests/tests/copy-before-write", line 207, in test_timeout_break_guest + """) +AssertionError: 'wrot[102 chars]led: Connection timed out\nread 1048576/104857[73 chars]c)\n' != 'wrot[102 chars]led: Operation timed out\nread 1048576/1048576[72 chars]c)\n' + wrote 524288/524288 bytes at offset 0 + 512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +- write failed: Connection timed out +? ^^^^ ^ ++ write failed: Operation timed out +? ^^ ^^ + read 1048576/1048576 bytes at offset 0 + 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + + ---------------------------------------------------------------------- Ran 4 tests Probably pythonic os.strerror doesn't correspond to what we have in C.. So, let's just go with my fix.
On 06.07.22 16:46, Vladimir Sementsov-Ogievskiy wrote: > On 7/6/22 13:26, Hanna Reitz wrote: >> On 05.07.22 17:37, Vladimir Sementsov-Ogievskiy wrote: >>> strerror() represents ETIMEDOUT a bit different in Linux and macOS / >>> FreeBSD. Let's support the latter too. >>> >>> Fixes: 9d05a87b77 ("iotests: copy-before-write: add cases for >>> cbw-timeout option") >>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> >>> --- >>> >>> As John and Thomas noted, the new iotests fails for FreeBSD and maxOS. >>> Here is a fix. Would be great if someone can test it. >>> >>> I tried to push it by >>> >>> git push --force -o ci.variable="QEMU_CI=1" >>> >>> to my block branch, I get a blocked pipeline >>> https://gitlab.com/vsementsov/qemu/-/pipelines/580573238 >>> but it doesn't have neither freebsd nor macos jobs.. How to get them? >>> >>> tests/qemu-iotests/tests/copy-before-write | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/tests/qemu-iotests/tests/copy-before-write >>> b/tests/qemu-iotests/tests/copy-before-write >>> index 16efebbf8f..56937b9dff 100755 >>> --- a/tests/qemu-iotests/tests/copy-before-write >>> +++ b/tests/qemu-iotests/tests/copy-before-write >>> @@ -192,6 +192,11 @@ read 1048576/1048576 bytes at offset 0 >>> def test_timeout_break_guest(self): >>> log = self.do_cbw_timeout('break-guest-write') >>> + # macOS and FreeBSD tend to represent ETIMEDOUT as >>> + # "Operation timed out", when Linux prefer >>> + # "Connection timed out" >>> + log = log.replace('Operation timed out', >>> + 'Connection timed out') >> >> If we know for sure that it’s ETIMEDOUT, how about >> os.strerror(errno.ETIMEDOUT)? > > I've checked this with make vm-build-freebsd, but it doesn't work: > > --- > /home/qemu/qemu-test.fxwm16/src/tests/qemu-iotests/tests/copy-before-write.out > +++ > /usr/home/qemu/qemu-test.fxwm16/build/tests/qemu-iotests/scratch/copy-before-write.out.bad > @@ -1,5 +1,22 @@ > -.... > +..F. > +====================================================================== > +FAIL: test_timeout_break_guest (__main__.TestCbwError) > +---------------------------------------------------------------------- > +Traceback (most recent call last): > + File > "/usr/home/qemu/qemu-test.fxwm16/src/tests/qemu-iotests/tests/copy-before-write", > line 207, in test_timeout_break_guest > + """) > +AssertionError: 'wrot[102 chars]led: Connection timed out\nread > 1048576/104857[73 chars]c)\n' != 'wrot[102 chars]led: Operation timed > out\nread 1048576/1048576[72 chars]c)\n' > + wrote 524288/524288 bytes at offset 0 > + 512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) > +- write failed: Connection timed out > +? ^^^^ ^ > ++ write failed: Operation timed out > +? ^^ ^^ > + read 1048576/1048576 bytes at offset 0 > + 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) > + > + > ---------------------------------------------------------------------- > Ran 4 tests > :( > Probably pythonic os.strerror doesn't correspond to what we have in C.. Great... > So, let's just go with my fix. Sounds good, then. Reviewed-by: Hanna Reitz <hreitz@redhat.com>
On 7/5/22 21:07, Vladimir Sementsov-Ogievskiy wrote: > strerror() represents ETIMEDOUT a bit different in Linux and macOS / > FreeBSD. Let's support the latter too. > > Fixes: 9d05a87b77 ("iotests: copy-before-write: add cases for cbw-timeout option") > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> > --- > > As John and Thomas noted, the new iotests fails for FreeBSD and maxOS. > Here is a fix. Would be great if someone can test it. > > I tried to push it by > > git push --force -o ci.variable="QEMU_CI=1" > > to my block branch, I get a blocked pipeline > https://gitlab.com/vsementsov/qemu/-/pipelines/580573238 > but it doesn't have neither freebsd nor macos jobs.. How to get them? > > tests/qemu-iotests/tests/copy-before-write | 5 +++++ > 1 file changed, 5 insertions(+) I am going to apply this as a hot fix, trying to re-green the CI. r~
diff --git a/tests/qemu-iotests/tests/copy-before-write b/tests/qemu-iotests/tests/copy-before-write index 16efebbf8f..56937b9dff 100755 --- a/tests/qemu-iotests/tests/copy-before-write +++ b/tests/qemu-iotests/tests/copy-before-write @@ -192,6 +192,11 @@ read 1048576/1048576 bytes at offset 0 def test_timeout_break_guest(self): log = self.do_cbw_timeout('break-guest-write') + # macOS and FreeBSD tend to represent ETIMEDOUT as + # "Operation timed out", when Linux prefer + # "Connection timed out" + log = log.replace('Operation timed out', + 'Connection timed out') self.assertEqual(log, """\ wrote 524288/524288 bytes at offset 0 512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
strerror() represents ETIMEDOUT a bit different in Linux and macOS / FreeBSD. Let's support the latter too. Fixes: 9d05a87b77 ("iotests: copy-before-write: add cases for cbw-timeout option") Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> --- As John and Thomas noted, the new iotests fails for FreeBSD and maxOS. Here is a fix. Would be great if someone can test it. I tried to push it by git push --force -o ci.variable="QEMU_CI=1" to my block branch, I get a blocked pipeline https://gitlab.com/vsementsov/qemu/-/pipelines/580573238 but it doesn't have neither freebsd nor macos jobs.. How to get them? tests/qemu-iotests/tests/copy-before-write | 5 +++++ 1 file changed, 5 insertions(+)