qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 0/4] Enable iotests during "make check"
@ 2019-07-16 12:28 Thomas Huth
  2019-07-16 12:28 ` [Qemu-devel] [PATCH v2 1/4] tests/qemu-iotests/check: Allow tests without groups Thomas Huth
                   ` (3 more replies)
  0 siblings, 4 replies; 21+ messages in thread
From: Thomas Huth @ 2019-07-16 12:28 UTC (permalink / raw)
  To: qemu-devel, Max Reitz; +Cc: Kevin Wolf, Alex Bennée, qemu-block

Let's enable the block iotests during "make check" again, to avoid that
they get broken so easily by accident (like we've seen many times in the
past).

v2:
 - Added new patch to allow tests without groups, removed the "o_direct"
   group again.
 - Removed some more tests from the "auto" group, like 197 and 215 which
   were sometimes causing the CI pipelines to fail on gitlab
 - Added a patch to remove the "duplicated" tests from the gitlab-ci.yml
   file.

Thomas Huth (4):
  tests/qemu-iotests/check: Allow tests without groups
  tests/qemu-iotests/group: Remove some more tests from the "auto" group
  tests: Run the iotests during "make check" again
  gitlab-ci: Remove qcow2 tests that are handled by "make check" already

 .gitlab-ci.yml              |  12 +---
 tests/Makefile.include      |   8 +--
 tests/check-block.sh        |  44 ++++++++++----
 tests/qemu-iotests-quick.sh |   8 ---
 tests/qemu-iotests/check    |   4 +-
 tests/qemu-iotests/group    | 114 ++++++++++++++++++------------------
 6 files changed, 101 insertions(+), 89 deletions(-)
 delete mode 100755 tests/qemu-iotests-quick.sh

-- 
2.21.0



^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH v2 1/4] tests/qemu-iotests/check: Allow tests without groups
  2019-07-16 12:28 [Qemu-devel] [PATCH v2 0/4] Enable iotests during "make check" Thomas Huth
@ 2019-07-16 12:28 ` Thomas Huth
  2019-07-16 15:27   ` Max Reitz
  2019-07-16 15:52   ` Alex Bennée
  2019-07-16 12:28 ` [Qemu-devel] [PATCH v2 2/4] tests/qemu-iotests/group: Remove some more tests from the "auto" group Thomas Huth
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 21+ messages in thread
From: Thomas Huth @ 2019-07-16 12:28 UTC (permalink / raw)
  To: qemu-devel, Max Reitz; +Cc: Kevin Wolf, Alex Bennée, qemu-block

The regular expressions in the "check" script currently expect that there
is always a space after the test number in the group file, so you can't
have a test in there without a group unless the line still ends with a
space - which is quite error prone since some editors might remove spaces
at the end of lines automatically.
Thus let's fix the regular expressions so that it is also possible to
have lines with one test number only in the group file.

Suggested-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qemu-iotests/check | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index f925606cc5..c24874ff4a 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -488,7 +488,7 @@ testlist options
 BEGIN        { for (t='$start'; t<='$end'; t++) printf "%03d\n",t }' \
         | while read id
         do
-            if grep -s "^$id " "$source_iotests/group" >/dev/null
+            if grep -s "^$id\( \|\$\)" "$source_iotests/group" >/dev/null
             then
                 # in group file ... OK
                 echo $id >>$tmp.list
@@ -547,7 +547,7 @@ else
         touch $tmp.list
     else
         # no test numbers, do everything from group file
-        sed -n -e '/^[0-9][0-9][0-9]*/s/[         ].*//p' <"$source_iotests/group" >$tmp.list
+        sed -n -e '/^[0-9][0-9][0-9]*/s/^\([0-9]*\).*/\1/p' <"$source_iotests/group" >$tmp.list
     fi
 fi
 
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH v2 2/4] tests/qemu-iotests/group: Remove some more tests from the "auto" group
  2019-07-16 12:28 [Qemu-devel] [PATCH v2 0/4] Enable iotests during "make check" Thomas Huth
  2019-07-16 12:28 ` [Qemu-devel] [PATCH v2 1/4] tests/qemu-iotests/check: Allow tests without groups Thomas Huth
@ 2019-07-16 12:28 ` Thomas Huth
  2019-07-16 15:26   ` Max Reitz
  2019-07-16 12:28 ` [Qemu-devel] [PATCH v2 3/4] tests: Run the iotests during "make check" again Thomas Huth
  2019-07-16 12:28 ` [Qemu-devel] [PATCH v2 4/4] gitlab-ci: Remove qcow2 tests that are handled by "make check" already Thomas Huth
  3 siblings, 1 reply; 21+ messages in thread
From: Thomas Huth @ 2019-07-16 12:28 UTC (permalink / raw)
  To: qemu-devel, Max Reitz; +Cc: Kevin Wolf, Alex Bennée, qemu-block

Remove some more tests from the "auto" group that either have issues
in certain environments (like macOS or FreeBSD, or on certain file systems
like ZFS or tmpfs), do not work with the qcow2 format, or that are simply
taking too much time.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qemu-iotests/group | 114 ++++++++++++++++++++-------------------
 1 file changed, 58 insertions(+), 56 deletions(-)

diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index b34c8e3c0c..0ae555b610 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -14,7 +14,9 @@
 #   runnable in any case. That means they should run with every QEMU binary
 #   (also non-x86), with every QEMU configuration (i.e. must not fail if
 #   an optional feature is not compiled in - but reporting a "skip" is ok),
-#   and work all kind of host filesystems and users (e.g. "nobody" or "root").
+#   work at least with the qcow2 file format, work with all kind of host
+#   filesystems and users (e.g. "nobody" or "root") and must not take too
+#   much memory and disk space (since CI pipelines tend to fail otherwise).
 #
 
 #
@@ -33,8 +35,8 @@
 011 rw auto quick
 012 auto quick
 013 rw auto
-014 rw auto
-015 rw snapshot auto
+014 rw
+015 rw snapshot
 # 016 was removed, do not reuse
 017 rw backing auto quick
 018 rw backing auto quick
@@ -42,7 +44,7 @@
 020 rw backing auto quick
 021 io auto quick
 022 rw snapshot auto
-023 rw auto
+023 rw
 024 rw backing auto quick
 025 rw auto quick
 026 rw blkdbg
@@ -78,94 +80,94 @@
 056 rw backing
 057 rw
 058 rw quick
-059 rw auto quick
+059 rw quick
 060 rw auto quick
 061 rw auto
 062 rw auto quick
 063 rw auto quick
-064 rw auto quick
+064 rw quick
 065 rw quick
 066 rw auto quick
 067 rw quick
 068 rw quick
 069 rw auto quick
-070 rw auto quick
+070 rw quick
 071 rw auto quick
 072 rw auto quick
 073 rw auto quick
 074 rw auto quick
-075 rw auto quick
-076 auto
-077 rw auto quick
-078 rw auto quick
+075 rw quick
+076 io
+077 rw quick
+078 rw quick
 079 rw auto
 080 rw auto
-081 rw auto quick
-082 rw auto quick
-083 rw auto
-084 img auto quick
+081 rw quick
+082 rw quick
+083 rw
+084 img quick
 085 rw
 086 rw auto quick
 087 rw quick
-088 rw auto quick
+088 rw quick
 089 rw auto quick
 090 rw auto quick
-091 rw auto migration
-092 rw auto quick
+091 rw migration
+092 rw quick
 093 throttle
-094 rw auto quick
+094 rw quick
 095 rw quick
 096 rw quick
 097 rw auto backing
 098 rw auto backing quick
 099 rw auto quick
 # 100 was removed, do not reuse
-101 rw auto quick
-102 rw auto quick
+101 rw quick
+102 rw quick
 103 rw auto quick
 104 rw auto
 105 rw auto quick
-106 rw auto quick
+106 rw quick
 107 rw auto quick
 108 rw auto quick
-109 rw auto
+109 rw
 110 rw auto backing quick
 111 rw auto quick
 112 rw
-113 rw auto quick
+113 rw quick
 114 rw auto quick
 115 rw
-116 rw auto quick
+116 rw quick
 117 rw auto
 118 rw
-119 rw auto quick
+119 rw quick
 120 rw auto quick
 121 rw
 122 rw auto
-123 rw auto quick
+123 rw quick
 124 rw backing
 125 rw
 126 rw auto backing
 127 rw backing quick
-128 rw auto quick
+128 rw quick
 129 rw quick
 130 rw auto quick
-131 rw auto quick
+131 rw quick
 132 rw quick
 133 auto quick
 134 rw auto quick
-135 rw auto
+135 rw
 136 rw
 137 rw auto
 138 rw auto quick
 139 rw quick
 140 rw auto quick
 141 rw auto quick
-142 auto
+142
 143 auto quick
 144 rw quick
 145 quick
-146 auto quick
+146 quick
 147 img
 148 rw quick
 149 rw sudo
@@ -179,18 +181,18 @@
 157 quick
 158 rw auto quick
 159 rw auto quick
-160 rw auto quick
+160 rw quick
 161 rw auto quick
 162 quick
 163 rw
 165 rw quick
 169 rw quick migration
 170 rw auto quick
-171 rw auto quick
+171 rw quick
 172 auto
-173 rw auto
+173 rw
 174 auto
-175 auto quick
+175 quick
 176 rw auto backing
 177 rw auto quick
 178 img
@@ -210,7 +212,7 @@
 194 rw migration quick
 195 rw auto quick
 196 rw quick migration
-197 rw auto quick
+197 rw quick
 198 rw
 199 rw migration
 200 rw
@@ -220,52 +222,52 @@
 204 rw quick
 205 rw quick
 206 rw
-207 rw auto
+207 rw
 208 rw quick
 209 rw quick
-210 rw auto
-211 rw auto quick
-212 rw auto quick
-213 rw auto quick
+210 rw
+211 rw quick
+212 rw quick
+213 rw quick
 214 rw auto
-215 rw auto quick
+215 rw quick
 216 rw quick
 217 rw auto quick
 218 rw quick
 219 rw
 220 rw auto
-221 rw auto quick
+221 rw quick
 222 rw quick
 223 rw quick
 224 rw quick
-225 rw auto quick
+225 rw quick
 226 auto quick
 227 quick
 228 rw quick
 229 auto quick
-231 auto quick
+231 quick
 232 quick
-233 auto quick
+233 quick
 234 quick migration
 235 quick
 236 quick
-237 rw auto quick
+237 rw quick
 238 quick
-239 rw auto quick
+239 rw quick
 240 quick
-241 rw auto quick
+241 rw quick
 242 rw quick
-243 rw auto quick
+243 rw quick
 244 rw auto quick
 245 rw
 246 rw quick
 247 rw quick
 248 rw quick
 249 rw auto quick
-250 rw auto quick
+250 rw quick
 251 rw auto quick
 252 rw auto backing quick
-253 rw auto quick
-254 rw auto backing quick
-255 rw auto quick
-256 rw auto quick
+253 rw quick
+254 rw backing quick
+255 rw quick
+256 rw quick
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH v2 3/4] tests: Run the iotests during "make check" again
  2019-07-16 12:28 [Qemu-devel] [PATCH v2 0/4] Enable iotests during "make check" Thomas Huth
  2019-07-16 12:28 ` [Qemu-devel] [PATCH v2 1/4] tests/qemu-iotests/check: Allow tests without groups Thomas Huth
  2019-07-16 12:28 ` [Qemu-devel] [PATCH v2 2/4] tests/qemu-iotests/group: Remove some more tests from the "auto" group Thomas Huth
@ 2019-07-16 12:28 ` Thomas Huth
  2019-07-16 15:37   ` Max Reitz
  2019-07-16 12:28 ` [Qemu-devel] [PATCH v2 4/4] gitlab-ci: Remove qcow2 tests that are handled by "make check" already Thomas Huth
  3 siblings, 1 reply; 21+ messages in thread
From: Thomas Huth @ 2019-07-16 12:28 UTC (permalink / raw)
  To: qemu-devel, Max Reitz; +Cc: Kevin Wolf, Alex Bennée, qemu-block

People often forget to run the iotests before submitting patches or pull
requests - this is likely due to the fact that we do not run the tests
during our mandatory "make check" tests yet. Now that we've got a proper
"auto" group of iotests that should be fine to run in every environment,
we can enable the iotests during "make check" again by running the "auto"
tests by default from the check-block.sh script.

Some cases still need to be checked first, though: iotests need bash and
GNU sed (otherwise they fail), and if gprof is enabled, it spoils the
output of some test cases causing them to fail. So if we detect that one
of the required programs is missing or that gprof is enabled, we still
have to skip the iotests to avoid failures.

And finally, since we are using check-block.sh now again, this patch also
removes the qemu-iotests-quick.sh script since we do not need that anymore
(and having two shell wrapper scripts around the block tests seems rather
confusing than helpful).

Signed-off-by: Thomas Huth <thuth@redhat.com>
[AJB: -makecheck to check-block.sh, move check-block to start and gate it]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/Makefile.include      |  8 +++----
 tests/check-block.sh        | 44 ++++++++++++++++++++++++++++---------
 tests/qemu-iotests-quick.sh |  8 -------
 3 files changed, 38 insertions(+), 22 deletions(-)
 delete mode 100755 tests/qemu-iotests-quick.sh

diff --git a/tests/Makefile.include b/tests/Makefile.include
index fd7fdb8658..e868dc1b9c 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -142,7 +142,7 @@ check-unit-y += tests/test-uuid$(EXESUF)
 check-unit-y += tests/ptimer-test$(EXESUF)
 check-unit-y += tests/test-qapi-util$(EXESUF)
 
-check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
+check-block-$(call land,$(CONFIG_POSIX),$(CONFIG_SOFTMMU)) += tests/check-block.sh
 
 # All QTests for now are POSIX-only, but the dependencies are
 # really in libqtest, not in the testcases themselves.
@@ -1092,8 +1092,8 @@ clean-tcg: $(CLEAN_TCG_TARGET_RULES)
 
 QEMU_IOTESTS_HELPERS-$(call land,$(CONFIG_SOFTMMU),$(CONFIG_LINUX)) = tests/qemu-iotests/socket_scm_helper$(EXESUF)
 
-.PHONY: check-tests/qemu-iotests-quick.sh
-check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF) qemu-io$(EXESUF) qemu-nbd$(EXESUF) $(QEMU_IOTESTS_HELPERS-y)
+.PHONY: check-tests/check-block.sh
+check-tests/check-block.sh: tests/check-block.sh qemu-img$(EXESUF) qemu-io$(EXESUF) qemu-nbd$(EXESUF) $(QEMU_IOTESTS_HELPERS-y)
 	$<
 
 .PHONY: $(patsubst %, check-%, $(check-qapi-schema-y))
@@ -1167,7 +1167,7 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR)
 check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y)) check-tests/qapi-schema/doc-good.texi
 check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS))
 check-block: $(patsubst %,check-%, $(check-block-y))
-check: check-qapi-schema check-unit check-softfloat check-qtest check-decodetree
+check: check-block check-qapi-schema check-unit check-softfloat check-qtest check-decodetree
 check-clean:
 	rm -rf $(check-unit-y) tests/*.o $(QEMU_IOTESTS_HELPERS-y)
 	rm -rf $(sort $(foreach target,$(SYSEMU_TARGET_LIST), $(check-qtest-$(target)-y)) $(check-qtest-generic-y))
diff --git a/tests/check-block.sh b/tests/check-block.sh
index f3d12fd602..c8b6cec3f6 100755
--- a/tests/check-block.sh
+++ b/tests/check-block.sh
@@ -1,24 +1,48 @@
 #!/bin/sh
 
-FORMAT_LIST="raw qcow2 qed vmdk vpc"
+# Honor the SPEED environment variable, just like we do it for the qtests.
+if [ "$SPEED" = "slow" ]; then
+    format_list="raw qcow2"
+    group=
+elif [ "$SPEED" = "thorough" ]; then
+    format_list="raw qcow2 qed vmdk vpc"
+    group=
+else
+    format_list=qcow2
+    group="-g auto"
+fi
+
 if [ "$#" -ne 0 ]; then
-    FORMAT_LIST="$@"
+    format_list="$@"
+fi
+
+if grep -q "TARGET_GPROF=y" *-softmmu/config-target.mak 2>/dev/null ; then
+    echo "GPROF is enabled ==> Not running the qemu-iotests."
+    exit 0
 fi
 
-export QEMU_PROG="$PWD/x86_64-softmmu/qemu-system-x86_64"
-export QEMU_IMG_PROG="$PWD/qemu-img"
-export QEMU_IO_PROG="$PWD/qemu-io"
+if [ -z "$(find . -name 'qemu-system-*' -print)" ]; then
+    echo "No qemu-system binary available ==> Not running the qemu-iotests."
+    exit 0
+fi
+
+if ! command -v bash >/dev/null 2>&1 ; then
+    echo "bash not available ==> Not running the qemu-iotests."
+    exit 0
+fi
 
-if [ ! -x $QEMU_PROG ]; then
-    echo "'make check-block' requires qemu-system-x86_64"
-    exit 1
+if ! (sed --version | grep 'GNU sed') > /dev/null 2>&1 ; then
+    if ! command -v gsed >/dev/null 2>&1; then
+        echo "GNU sed not available ==> Not running the qemu-iotests."
+        exit 0
+    fi
 fi
 
 cd tests/qemu-iotests
 
 ret=0
-for FMT in $FORMAT_LIST ; do
-    ./check -T -nocache -$FMT || ret=1
+for fmt in $format_list ; do
+    ./check -makecheck -$fmt $group || ret=1
 done
 
 exit $ret
diff --git a/tests/qemu-iotests-quick.sh b/tests/qemu-iotests-quick.sh
deleted file mode 100755
index 0e554bb972..0000000000
--- a/tests/qemu-iotests-quick.sh
+++ /dev/null
@@ -1,8 +0,0 @@
-#!/bin/sh
-
-cd tests/qemu-iotests
-
-ret=0
-TEST_DIR=${TEST_DIR:-/tmp/qemu-iotests-quick-$$} ./check -T -qcow2 -g quick || ret=1
-
-exit $ret
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH v2 4/4] gitlab-ci: Remove qcow2 tests that are handled by "make check" already
  2019-07-16 12:28 [Qemu-devel] [PATCH v2 0/4] Enable iotests during "make check" Thomas Huth
                   ` (2 preceding siblings ...)
  2019-07-16 12:28 ` [Qemu-devel] [PATCH v2 3/4] tests: Run the iotests during "make check" again Thomas Huth
@ 2019-07-16 12:28 ` Thomas Huth
  2019-07-16 15:41   ` Max Reitz
  3 siblings, 1 reply; 21+ messages in thread
From: Thomas Huth @ 2019-07-16 12:28 UTC (permalink / raw)
  To: qemu-devel, Max Reitz; +Cc: Kevin Wolf, Alex Bennée, qemu-block

Since most iotests are now run during "make check" already, we do not
need to test them explicitly from the gitlab-ci.yml script anymore.
And while we're at it, add some of the new non-auto tests >= 248 instead.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 .gitlab-ci.yml | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index c63bf2f822..fa5d094453 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -45,15 +45,9 @@ build-tcg-disabled:
  - ./check -raw 001 002 003 004 005 008 009 010 011 012 021 025 032 033 048
             052 063 077 086 101 104 106 113 147 148 150 151 152 157 159 160
             163 170 171 183 184 192 194 197 205 208 215 221 222 226 227 236
- - ./check -qcow2 001 002 003 004 005 007 008 009 010 011 012 013 017 018 019
-            020 021 022 024 025 027 028 029 031 032 033 034 035 036 037 038
-            039 040 042 043 046 047 048 049 050 051 052 053 054 056 057 058
-            060 061 062 063 065 066 067 068 069 071 072 073 074 079 080 082
-            085 086 089 090 091 095 096 097 098 099 102 103 104 105 107 108
-            110 111 114 117 120 122 124 126 127 129 130 132 133 134 137 138
-            139 140 141 142 143 144 145 147 150 151 152 154 155 156 157 158
-            161 165 170 172 174 176 177 179 184 186 187 190 192 194 195 196
-            197 200 202 203 205 208 209 214 215 216 217 218 222 226 227 229 234
+ - ./check -qcow2 028 040 051 056 057 058 065 067 068 082 085 091 095 096 102
+            124 127 129 132 139 142 144 145 147 151 152 155 157 165 194 196
+            200 202 203 205 208 209 216 218 222 227 234 248 250 254 255 256
 
 build-user:
  script:
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/4] tests/qemu-iotests/group: Remove some more tests from the "auto" group
  2019-07-16 12:28 ` [Qemu-devel] [PATCH v2 2/4] tests/qemu-iotests/group: Remove some more tests from the "auto" group Thomas Huth
@ 2019-07-16 15:26   ` Max Reitz
  2019-07-16 15:31     ` Thomas Huth
  0 siblings, 1 reply; 21+ messages in thread
From: Max Reitz @ 2019-07-16 15:26 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel; +Cc: Kevin Wolf, Alex Bennée, qemu-block


[-- Attachment #1.1: Type: text/plain, Size: 801 bytes --]

On 16.07.19 14:28, Thomas Huth wrote:
> Remove some more tests from the "auto" group that either have issues
> in certain environments (like macOS or FreeBSD, or on certain file systems
> like ZFS or tmpfs), do not work with the qcow2 format, or that are simply
> taking too much time.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/qemu-iotests/group | 114 ++++++++++++++++++++-------------------
>  1 file changed, 58 insertions(+), 56 deletions(-)

I just looked through the list to see whether any of the test seems like
we’d want to keep it even though it is a bit slow.  Mostly I was looking
for tests that cover complex cases.

255 is the only one that seemed to fit that bill to me.  So why do you
remove it?  Is it because it takes two seconds?

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 1/4] tests/qemu-iotests/check: Allow tests without groups
  2019-07-16 12:28 ` [Qemu-devel] [PATCH v2 1/4] tests/qemu-iotests/check: Allow tests without groups Thomas Huth
@ 2019-07-16 15:27   ` Max Reitz
  2019-07-16 15:52   ` Alex Bennée
  1 sibling, 0 replies; 21+ messages in thread
From: Max Reitz @ 2019-07-16 15:27 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel; +Cc: Kevin Wolf, Alex Bennée, qemu-block


[-- Attachment #1.1: Type: text/plain, Size: 765 bytes --]

On 16.07.19 14:28, Thomas Huth wrote:
> The regular expressions in the "check" script currently expect that there
> is always a space after the test number in the group file, so you can't
> have a test in there without a group unless the line still ends with a
> space - which is quite error prone since some editors might remove spaces
> at the end of lines automatically.
> Thus let's fix the regular expressions so that it is also possible to
> have lines with one test number only in the group file.
> 
> Suggested-by: Max Reitz <mreitz@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/qemu-iotests/check | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

FWIW

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/4] tests/qemu-iotests/group: Remove some more tests from the "auto" group
  2019-07-16 15:26   ` Max Reitz
@ 2019-07-16 15:31     ` Thomas Huth
  2019-07-16 15:44       ` Max Reitz
  0 siblings, 1 reply; 21+ messages in thread
From: Thomas Huth @ 2019-07-16 15:31 UTC (permalink / raw)
  To: Max Reitz, qemu-devel; +Cc: Kevin Wolf, Alex Bennée, qemu-block

[-- Attachment #1: Type: text/plain, Size: 1041 bytes --]

On 16/07/2019 17.26, Max Reitz wrote:
> On 16.07.19 14:28, Thomas Huth wrote:
>> Remove some more tests from the "auto" group that either have issues
>> in certain environments (like macOS or FreeBSD, or on certain file systems
>> like ZFS or tmpfs), do not work with the qcow2 format, or that are simply
>> taking too much time.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  tests/qemu-iotests/group | 114 ++++++++++++++++++++-------------------
>>  1 file changed, 58 insertions(+), 56 deletions(-)
> 
> I just looked through the list to see whether any of the test seems like
> we’d want to keep it even though it is a bit slow.  Mostly I was looking
> for tests that cover complex cases.
> 
> 255 is the only one that seemed to fit that bill to me.  So why do you
> remove it?  Is it because it takes two seconds?

No, I removed it because it was failing on macOS:

 https://cirrus-ci.com/task/4860239294234624

("OSError: AF_UNIX path too long" is the error, if I got that right)

 Thomas


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 3/4] tests: Run the iotests during "make check" again
  2019-07-16 12:28 ` [Qemu-devel] [PATCH v2 3/4] tests: Run the iotests during "make check" again Thomas Huth
@ 2019-07-16 15:37   ` Max Reitz
  2019-07-17  9:41     ` Thomas Huth
  0 siblings, 1 reply; 21+ messages in thread
From: Max Reitz @ 2019-07-16 15:37 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel; +Cc: Kevin Wolf, Alex Bennée, qemu-block


[-- Attachment #1.1: Type: text/plain, Size: 2425 bytes --]

On 16.07.19 14:28, Thomas Huth wrote:
> People often forget to run the iotests before submitting patches or pull
> requests - this is likely due to the fact that we do not run the tests
> during our mandatory "make check" tests yet. Now that we've got a proper
> "auto" group of iotests that should be fine to run in every environment,
> we can enable the iotests during "make check" again by running the "auto"
> tests by default from the check-block.sh script.
> 
> Some cases still need to be checked first, though: iotests need bash and
> GNU sed (otherwise they fail), and if gprof is enabled, it spoils the
> output of some test cases causing them to fail. So if we detect that one
> of the required programs is missing or that gprof is enabled, we still
> have to skip the iotests to avoid failures.
> 
> And finally, since we are using check-block.sh now again, this patch also
> removes the qemu-iotests-quick.sh script since we do not need that anymore
> (and having two shell wrapper scripts around the block tests seems rather
> confusing than helpful).
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> [AJB: -makecheck to check-block.sh, move check-block to start and gate it]
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  tests/Makefile.include      |  8 +++----
>  tests/check-block.sh        | 44 ++++++++++++++++++++++++++++---------
>  tests/qemu-iotests-quick.sh |  8 -------
>  3 files changed, 38 insertions(+), 22 deletions(-)
>  delete mode 100755 tests/qemu-iotests-quick.sh
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index fd7fdb8658..e868dc1b9c 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include

[...]

> @@ -1092,8 +1092,8 @@ clean-tcg: $(CLEAN_TCG_TARGET_RULES)
>  
>  QEMU_IOTESTS_HELPERS-$(call land,$(CONFIG_SOFTMMU),$(CONFIG_LINUX)) = tests/qemu-iotests/socket_scm_helper$(EXESUF)
>  
> -.PHONY: check-tests/qemu-iotests-quick.sh
> -check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF) qemu-io$(EXESUF) qemu-nbd$(EXESUF) $(QEMU_IOTESTS_HELPERS-y)
> +.PHONY: check-tests/check-block.sh
> +check-tests/check-block.sh: tests/check-block.sh qemu-img$(EXESUF) qemu-io$(EXESUF) qemu-nbd$(EXESUF) $(QEMU_IOTESTS_HELPERS-y)

Does this need to depend on a full qemu binary, too?

Max

>  	$<
>  
>  .PHONY: $(patsubst %, check-%, $(check-qapi-schema-y))


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 4/4] gitlab-ci: Remove qcow2 tests that are handled by "make check" already
  2019-07-16 12:28 ` [Qemu-devel] [PATCH v2 4/4] gitlab-ci: Remove qcow2 tests that are handled by "make check" already Thomas Huth
@ 2019-07-16 15:41   ` Max Reitz
  2019-07-16 15:51     ` Thomas Huth
  0 siblings, 1 reply; 21+ messages in thread
From: Max Reitz @ 2019-07-16 15:41 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel; +Cc: Kevin Wolf, Alex Bennée, qemu-block


[-- Attachment #1.1: Type: text/plain, Size: 1842 bytes --]

On 16.07.19 14:28, Thomas Huth wrote:
> Since most iotests are now run during "make check" already, we do not
> need to test them explicitly from the gitlab-ci.yml script anymore.
> And while we're at it, add some of the new non-auto tests >= 248 instead.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  .gitlab-ci.yml | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index c63bf2f822..fa5d094453 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -45,15 +45,9 @@ build-tcg-disabled:
>   - ./check -raw 001 002 003 004 005 008 009 010 011 012 021 025 032 033 048
>              052 063 077 086 101 104 106 113 147 148 150 151 152 157 159 160
>              163 170 171 183 184 192 194 197 205 208 215 221 222 226 227 236
> - - ./check -qcow2 001 002 003 004 005 007 008 009 010 011 012 013 017 018 019
> -            020 021 022 024 025 027 028 029 031 032 033 034 035 036 037 038
> -            039 040 042 043 046 047 048 049 050 051 052 053 054 056 057 058
> -            060 061 062 063 065 066 067 068 069 071 072 073 074 079 080 082
> -            085 086 089 090 091 095 096 097 098 099 102 103 104 105 107 108
> -            110 111 114 117 120 122 124 126 127 129 130 132 133 134 137 138
> -            139 140 141 142 143 144 145 147 150 151 152 154 155 156 157 158
> -            161 165 170 172 174 176 177 179 184 186 187 190 192 194 195 196
> -            197 200 202 203 205 208 209 214 215 216 217 218 222 226 227 229 234
> + - ./check -qcow2 028 040 051 056 057 058 065 067 068 082 085 091 095 096 102
> +            124 127 129 132 139 142 144 145 147 151 152 155 157 165 194 196
> +            200 202 203 205 208 209 216 218 222 227 234 248 250 254 255 256

This removes 197 and 215.  Why?

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/4] tests/qemu-iotests/group: Remove some more tests from the "auto" group
  2019-07-16 15:31     ` Thomas Huth
@ 2019-07-16 15:44       ` Max Reitz
  2019-07-16 15:45         ` Max Reitz
  2019-07-16 15:56         ` Eric Blake
  0 siblings, 2 replies; 21+ messages in thread
From: Max Reitz @ 2019-07-16 15:44 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel; +Cc: Kevin Wolf, Alex Bennée, qemu-block


[-- Attachment #1.1: Type: text/plain, Size: 1179 bytes --]

On 16.07.19 17:31, Thomas Huth wrote:
> On 16/07/2019 17.26, Max Reitz wrote:
>> On 16.07.19 14:28, Thomas Huth wrote:
>>> Remove some more tests from the "auto" group that either have issues
>>> in certain environments (like macOS or FreeBSD, or on certain file systems
>>> like ZFS or tmpfs), do not work with the qcow2 format, or that are simply
>>> taking too much time.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>  tests/qemu-iotests/group | 114 ++++++++++++++++++++-------------------
>>>  1 file changed, 58 insertions(+), 56 deletions(-)
>>
>> I just looked through the list to see whether any of the test seems like
>> we’d want to keep it even though it is a bit slow.  Mostly I was looking
>> for tests that cover complex cases.
>>
>> 255 is the only one that seemed to fit that bill to me.  So why do you
>> remove it?  Is it because it takes two seconds?
> 
> No, I removed it because it was failing on macOS:
> 
>  https://cirrus-ci.com/task/4860239294234624
> 
> ("OSError: AF_UNIX path too long" is the error, if I got that right)

Ah, OK.  So, uh, we effectively can’t run any Python tests on macOS?

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/4] tests/qemu-iotests/group: Remove some more tests from the "auto" group
  2019-07-16 15:44       ` Max Reitz
@ 2019-07-16 15:45         ` Max Reitz
  2019-07-16 15:56         ` Eric Blake
  1 sibling, 0 replies; 21+ messages in thread
From: Max Reitz @ 2019-07-16 15:45 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel; +Cc: Kevin Wolf, Alex Bennée, qemu-block


[-- Attachment #1.1: Type: text/plain, Size: 1348 bytes --]

On 16.07.19 17:44, Max Reitz wrote:
> On 16.07.19 17:31, Thomas Huth wrote:
>> On 16/07/2019 17.26, Max Reitz wrote:
>>> On 16.07.19 14:28, Thomas Huth wrote:
>>>> Remove some more tests from the "auto" group that either have issues
>>>> in certain environments (like macOS or FreeBSD, or on certain file systems
>>>> like ZFS or tmpfs), do not work with the qcow2 format, or that are simply
>>>> taking too much time.
>>>>
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>> ---
>>>>  tests/qemu-iotests/group | 114 ++++++++++++++++++++-------------------
>>>>  1 file changed, 58 insertions(+), 56 deletions(-)
>>>
>>> I just looked through the list to see whether any of the test seems like
>>> we’d want to keep it even though it is a bit slow.  Mostly I was looking
>>> for tests that cover complex cases.
>>>
>>> 255 is the only one that seemed to fit that bill to me.  So why do you
>>> remove it?  Is it because it takes two seconds?
>>
>> No, I removed it because it was failing on macOS:
>>
>>  https://cirrus-ci.com/task/4860239294234624
>>
>> ("OSError: AF_UNIX path too long" is the error, if I got that right)
> 
> Ah, OK.  So, uh, we effectively can’t run any Python tests on macOS?

(Forgot the

Reviewed-by: Max Reitz <mreitz@redhat.com>

because I’m so... amazed by that possibility)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 4/4] gitlab-ci: Remove qcow2 tests that are handled by "make check" already
  2019-07-16 15:41   ` Max Reitz
@ 2019-07-16 15:51     ` Thomas Huth
  2019-07-16 15:57       ` Max Reitz
  0 siblings, 1 reply; 21+ messages in thread
From: Thomas Huth @ 2019-07-16 15:51 UTC (permalink / raw)
  To: Max Reitz, qemu-devel; +Cc: Kevin Wolf, Alex Bennée, qemu-block

[-- Attachment #1: Type: text/plain, Size: 2263 bytes --]

On 16/07/2019 17.41, Max Reitz wrote:
> On 16.07.19 14:28, Thomas Huth wrote:
>> Since most iotests are now run during "make check" already, we do not
>> need to test them explicitly from the gitlab-ci.yml script anymore.
>> And while we're at it, add some of the new non-auto tests >= 248 instead.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  .gitlab-ci.yml | 12 +++---------
>>  1 file changed, 3 insertions(+), 9 deletions(-)
>>
>> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
>> index c63bf2f822..fa5d094453 100644
>> --- a/.gitlab-ci.yml
>> +++ b/.gitlab-ci.yml
>> @@ -45,15 +45,9 @@ build-tcg-disabled:
>>   - ./check -raw 001 002 003 004 005 008 009 010 011 012 021 025 032 033 048
>>              052 063 077 086 101 104 106 113 147 148 150 151 152 157 159 160
>>              163 170 171 183 184 192 194 197 205 208 215 221 222 226 227 236
>> - - ./check -qcow2 001 002 003 004 005 007 008 009 010 011 012 013 017 018 019
>> -            020 021 022 024 025 027 028 029 031 032 033 034 035 036 037 038
>> -            039 040 042 043 046 047 048 049 050 051 052 053 054 056 057 058
>> -            060 061 062 063 065 066 067 068 069 071 072 073 074 079 080 082
>> -            085 086 089 090 091 095 096 097 098 099 102 103 104 105 107 108
>> -            110 111 114 117 120 122 124 126 127 129 130 132 133 134 137 138
>> -            139 140 141 142 143 144 145 147 150 151 152 154 155 156 157 158
>> -            161 165 170 172 174 176 177 179 184 186 187 190 192 194 195 196
>> -            197 200 202 203 205 208 209 214 215 216 217 218 222 226 227 229 234
>> + - ./check -qcow2 028 040 051 056 057 058 065 067 068 082 085 091 095 096 102
>> +            124 127 129 132 139 142 144 145 147 151 152 155 157 165 194 196
>> +            200 202 203 205 208 209 216 218 222 227 234 248 250 254 255 256
> 
> This removes 197 and 215.  Why?

As mentioned in the cover letter, I've seen problems with 197 and 215 in
the gitlab CI pipelines (since the two tests apparently cause a lot of
memory pressure). At least they were causing trouble when "make check"
was running with other tests in parallel. Maybe they still work fine
when they run alone here. I'll give it another try...

 Thomas


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 1/4] tests/qemu-iotests/check: Allow tests without groups
  2019-07-16 12:28 ` [Qemu-devel] [PATCH v2 1/4] tests/qemu-iotests/check: Allow tests without groups Thomas Huth
  2019-07-16 15:27   ` Max Reitz
@ 2019-07-16 15:52   ` Alex Bennée
  1 sibling, 0 replies; 21+ messages in thread
From: Alex Bennée @ 2019-07-16 15:52 UTC (permalink / raw)
  To: Thomas Huth; +Cc: Kevin Wolf, qemu-devel, qemu-block, Max Reitz


Thomas Huth <thuth@redhat.com> writes:

> The regular expressions in the "check" script currently expect that there
> is always a space after the test number in the group file, so you can't
> have a test in there without a group unless the line still ends with a
> space - which is quite error prone since some editors might remove spaces
> at the end of lines automatically.
> Thus let's fix the regular expressions so that it is also possible to
> have lines with one test number only in the group file.
>
> Suggested-by: Max Reitz <mreitz@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  tests/qemu-iotests/check | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index f925606cc5..c24874ff4a 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -488,7 +488,7 @@ testlist options
>  BEGIN        { for (t='$start'; t<='$end'; t++) printf "%03d\n",t }' \
>          | while read id
>          do
> -            if grep -s "^$id " "$source_iotests/group" >/dev/null
> +            if grep -s "^$id\( \|\$\)" "$source_iotests/group" >/dev/null
>              then
>                  # in group file ... OK
>                  echo $id >>$tmp.list
> @@ -547,7 +547,7 @@ else
>          touch $tmp.list
>      else
>          # no test numbers, do everything from group file
> -        sed -n -e '/^[0-9][0-9][0-9]*/s/[         ].*//p' <"$source_iotests/group" >$tmp.list
> +        sed -n -e '/^[0-9][0-9][0-9]*/s/^\([0-9]*\).*/\1/p' <"$source_iotests/group" >$tmp.list
>      fi
>  fi


--
Alex Bennée


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/4] tests/qemu-iotests/group: Remove some more tests from the "auto" group
  2019-07-16 15:44       ` Max Reitz
  2019-07-16 15:45         ` Max Reitz
@ 2019-07-16 15:56         ` Eric Blake
  2019-07-16 15:58           ` Max Reitz
  1 sibling, 1 reply; 21+ messages in thread
From: Eric Blake @ 2019-07-16 15:56 UTC (permalink / raw)
  To: Max Reitz, Thomas Huth, qemu-devel
  Cc: Kevin Wolf, Alex Bennée, qemu-block


[-- Attachment #1.1: Type: text/plain, Size: 1939 bytes --]

On 7/16/19 10:44 AM, Max Reitz wrote:
> On 16.07.19 17:31, Thomas Huth wrote:
>> On 16/07/2019 17.26, Max Reitz wrote:
>>> On 16.07.19 14:28, Thomas Huth wrote:
>>>> Remove some more tests from the "auto" group that either have issues
>>>> in certain environments (like macOS or FreeBSD, or on certain file systems
>>>> like ZFS or tmpfs), do not work with the qcow2 format, or that are simply
>>>> taking too much time.
>>>>
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>> ---
>>>>  tests/qemu-iotests/group | 114 ++++++++++++++++++++-------------------
>>>>  1 file changed, 58 insertions(+), 56 deletions(-)
>>>
>>> I just looked through the list to see whether any of the test seems like
>>> we’d want to keep it even though it is a bit slow.  Mostly I was looking
>>> for tests that cover complex cases.
>>>
>>> 255 is the only one that seemed to fit that bill to me.  So why do you
>>> remove it?  Is it because it takes two seconds?
>>
>> No, I removed it because it was failing on macOS:
>>
>>  https://cirrus-ci.com/task/4860239294234624
>>
>> ("OSError: AF_UNIX path too long" is the error, if I got that right)
> 
> Ah, OK.  So, uh, we effectively can’t run any Python tests on macOS?

Not when our CI is set up to use super-long file names:

+  File
"/private/var/folders/sy/2x5qvs0n4lg18fry9jz4y21m0000gn/T/cirrus-ci-build/tests/qemu-iotests/../../python/qemu/machine.py",
line 294, in launch

Is there any way to create our sockets somewhere under /tmp instead of
inside tests/qemu-iotests, so that we have a shorter filename for
sockets no matter how deep in the file hierarchy the tests themselves live?

Also, at one point, we tossed around the idea of
s/qemu-iotests/iotests/, to shave off 5 characters that don't really add
anything.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 4/4] gitlab-ci: Remove qcow2 tests that are handled by "make check" already
  2019-07-16 15:51     ` Thomas Huth
@ 2019-07-16 15:57       ` Max Reitz
  0 siblings, 0 replies; 21+ messages in thread
From: Max Reitz @ 2019-07-16 15:57 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel; +Cc: Kevin Wolf, Alex Bennée, qemu-block


[-- Attachment #1.1: Type: text/plain, Size: 2457 bytes --]

On 16.07.19 17:51, Thomas Huth wrote:
> On 16/07/2019 17.41, Max Reitz wrote:
>> On 16.07.19 14:28, Thomas Huth wrote:
>>> Since most iotests are now run during "make check" already, we do not
>>> need to test them explicitly from the gitlab-ci.yml script anymore.
>>> And while we're at it, add some of the new non-auto tests >= 248 instead.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>  .gitlab-ci.yml | 12 +++---------
>>>  1 file changed, 3 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
>>> index c63bf2f822..fa5d094453 100644
>>> --- a/.gitlab-ci.yml
>>> +++ b/.gitlab-ci.yml
>>> @@ -45,15 +45,9 @@ build-tcg-disabled:
>>>   - ./check -raw 001 002 003 004 005 008 009 010 011 012 021 025 032 033 048
>>>              052 063 077 086 101 104 106 113 147 148 150 151 152 157 159 160
>>>              163 170 171 183 184 192 194 197 205 208 215 221 222 226 227 236
>>> - - ./check -qcow2 001 002 003 004 005 007 008 009 010 011 012 013 017 018 019
>>> -            020 021 022 024 025 027 028 029 031 032 033 034 035 036 037 038
>>> -            039 040 042 043 046 047 048 049 050 051 052 053 054 056 057 058
>>> -            060 061 062 063 065 066 067 068 069 071 072 073 074 079 080 082
>>> -            085 086 089 090 091 095 096 097 098 099 102 103 104 105 107 108
>>> -            110 111 114 117 120 122 124 126 127 129 130 132 133 134 137 138
>>> -            139 140 141 142 143 144 145 147 150 151 152 154 155 156 157 158
>>> -            161 165 170 172 174 176 177 179 184 186 187 190 192 194 195 196
>>> -            197 200 202 203 205 208 209 214 215 216 217 218 222 226 227 229 234
>>> + - ./check -qcow2 028 040 051 056 057 058 065 067 068 082 085 091 095 096 102
>>> +            124 127 129 132 139 142 144 145 147 151 152 155 157 165 194 196
>>> +            200 202 203 205 208 209 216 218 222 227 234 248 250 254 255 256
>>
>> This removes 197 and 215.  Why?
> 
> As mentioned in the cover letter, I've seen problems with 197 and 215 in
> the gitlab CI pipelines (since the two tests apparently cause a lot of
> memory pressure). At least they were causing trouble when "make check"
> was running with other tests in parallel. Maybe they still work fine
> when they run alone here. I'll give it another try...

OK.  Shouldn’t this be mentioned in the commit message here, too? ;-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/4] tests/qemu-iotests/group: Remove some more tests from the "auto" group
  2019-07-16 15:56         ` Eric Blake
@ 2019-07-16 15:58           ` Max Reitz
  2019-07-16 16:09             ` Eric Blake
  0 siblings, 1 reply; 21+ messages in thread
From: Max Reitz @ 2019-07-16 15:58 UTC (permalink / raw)
  To: Eric Blake, Thomas Huth, qemu-devel
  Cc: Kevin Wolf, Alex Bennée, qemu-block


[-- Attachment #1.1: Type: text/plain, Size: 2045 bytes --]

On 16.07.19 17:56, Eric Blake wrote:
> On 7/16/19 10:44 AM, Max Reitz wrote:
>> On 16.07.19 17:31, Thomas Huth wrote:
>>> On 16/07/2019 17.26, Max Reitz wrote:
>>>> On 16.07.19 14:28, Thomas Huth wrote:
>>>>> Remove some more tests from the "auto" group that either have issues
>>>>> in certain environments (like macOS or FreeBSD, or on certain file systems
>>>>> like ZFS or tmpfs), do not work with the qcow2 format, or that are simply
>>>>> taking too much time.
>>>>>
>>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>>> ---
>>>>>  tests/qemu-iotests/group | 114 ++++++++++++++++++++-------------------
>>>>>  1 file changed, 58 insertions(+), 56 deletions(-)
>>>>
>>>> I just looked through the list to see whether any of the test seems like
>>>> we’d want to keep it even though it is a bit slow.  Mostly I was looking
>>>> for tests that cover complex cases.
>>>>
>>>> 255 is the only one that seemed to fit that bill to me.  So why do you
>>>> remove it?  Is it because it takes two seconds?
>>>
>>> No, I removed it because it was failing on macOS:
>>>
>>>  https://cirrus-ci.com/task/4860239294234624
>>>
>>> ("OSError: AF_UNIX path too long" is the error, if I got that right)
>>
>> Ah, OK.  So, uh, we effectively can’t run any Python tests on macOS?
> 
> Not when our CI is set up to use super-long file names:
> 
> +  File
> "/private/var/folders/sy/2x5qvs0n4lg18fry9jz4y21m0000gn/T/cirrus-ci-build/tests/qemu-iotests/../../python/qemu/machine.py",
> line 294, in launch

That isn’t really long.

> Is there any way to create our sockets somewhere under /tmp instead of
> inside tests/qemu-iotests, so that we have a shorter filename for
> sockets no matter how deep in the file hierarchy the tests themselves live?
> 
> Also, at one point, we tossed around the idea of
> s/qemu-iotests/iotests/, to shave off 5 characters that don't really add
> anything.

I’d personally rather just skip the iotests if we detect such a silly
OS, but maybe that’s just me.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/4] tests/qemu-iotests/group: Remove some more tests from the "auto" group
  2019-07-16 15:58           ` Max Reitz
@ 2019-07-16 16:09             ` Eric Blake
  2019-07-16 16:12               ` Max Reitz
  0 siblings, 1 reply; 21+ messages in thread
From: Eric Blake @ 2019-07-16 16:09 UTC (permalink / raw)
  To: Max Reitz, Thomas Huth, qemu-devel
  Cc: Kevin Wolf, Alex Bennée, qemu-block


[-- Attachment #1.1: Type: text/plain, Size: 1893 bytes --]

On 7/16/19 10:58 AM, Max Reitz wrote:

>>> Ah, OK.  So, uh, we effectively can’t run any Python tests on macOS?
>>
>> Not when our CI is set up to use super-long file names:
>>
>> +  File
>> "/private/var/folders/sy/2x5qvs0n4lg18fry9jz4y21m0000gn/T/cirrus-ci-build/tests/qemu-iotests/../../python/qemu/machine.py",
>> line 294, in launch
> 
> That isn’t really long.

The MacOS limit is 104 (or 103 if you insist on using the trailing NUL
yourself), compared to Linux at 108 (107).  And:

$ printf
"/private/var/folders/sy/2x5qvs0n4lg18fry9jz4y21m0000gn/T/cirrus-ci-build/tests/qemu-iotests/scratch/name"
| wc -c
104

shows that we are right on the verge of overflowing the space allotted,
with any socket name worth using when the socket lives inside
qemu-iotests/scratch.

> 
>> Is there any way to create our sockets somewhere under /tmp instead of
>> inside tests/qemu-iotests, so that we have a shorter filename for
>> sockets no matter how deep in the file hierarchy the tests themselves live?
>>
>> Also, at one point, we tossed around the idea of
>> s/qemu-iotests/iotests/, to shave off 5 characters that don't really add
>> anything.
> 
> I’d personally rather just skip the iotests if we detect such a silly
> OS, but maybe that’s just me.

It's a rather unfortunate limit, but it's not all that silly (the limit
is based on the fact that struct sockaddr has to fit inside a nice
power-of-2 structure somewhere in the kernel, and enough else is used
that you really are left with just 104/108 bytes, or even 92 bytes if
you use HP-UX 11).  POSIX does not place a minimum length on sun_path,
but I know of no system that does not allow at least 92 bytes, if you
are aiming for a portably-small name.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/4] tests/qemu-iotests/group: Remove some more tests from the "auto" group
  2019-07-16 16:09             ` Eric Blake
@ 2019-07-16 16:12               ` Max Reitz
  2019-07-18  7:42                 ` Thomas Huth
  0 siblings, 1 reply; 21+ messages in thread
From: Max Reitz @ 2019-07-16 16:12 UTC (permalink / raw)
  To: Eric Blake, Thomas Huth, qemu-devel
  Cc: Kevin Wolf, Alex Bennée, qemu-block


[-- Attachment #1.1: Type: text/plain, Size: 2048 bytes --]

On 16.07.19 18:09, Eric Blake wrote:
> On 7/16/19 10:58 AM, Max Reitz wrote:
> 
>>>> Ah, OK.  So, uh, we effectively can’t run any Python tests on macOS?
>>>
>>> Not when our CI is set up to use super-long file names:
>>>
>>> +  File
>>> "/private/var/folders/sy/2x5qvs0n4lg18fry9jz4y21m0000gn/T/cirrus-ci-build/tests/qemu-iotests/../../python/qemu/machine.py",
>>> line 294, in launch
>>
>> That isn’t really long.
> 
> The MacOS limit is 104 (or 103 if you insist on using the trailing NUL
> yourself), compared to Linux at 108 (107).  And:
> 
> $ printf
> "/private/var/folders/sy/2x5qvs0n4lg18fry9jz4y21m0000gn/T/cirrus-ci-build/tests/qemu-iotests/scratch/name"
> | wc -c
> 104
> 
> shows that we are right on the verge of overflowing the space allotted,
> with any socket name worth using when the socket lives inside
> qemu-iotests/scratch.
> 
>>
>>> Is there any way to create our sockets somewhere under /tmp instead of
>>> inside tests/qemu-iotests, so that we have a shorter filename for
>>> sockets no matter how deep in the file hierarchy the tests themselves live?
>>>
>>> Also, at one point, we tossed around the idea of
>>> s/qemu-iotests/iotests/, to shave off 5 characters that don't really add
>>> anything.
>>
>> I’d personally rather just skip the iotests if we detect such a silly
>> OS, but maybe that’s just me.
> 
> It's a rather unfortunate limit, but it's not all that silly (the limit
> is based on the fact that struct sockaddr has to fit inside a nice
> power-of-2 structure somewhere in the kernel, and enough else is used
> that you really are left with just 104/108 bytes, or even 92 bytes if
> you use HP-UX 11).  POSIX does not place a minimum length on sun_path,
> but I know of no system that does not allow at least 92 bytes, if you
> are aiming for a portably-small name.

It does come to me as a surprise that the sockaddr is a path instead of,
say, an inode number.

But shaving off the “qemu-” seems like the wrong approach to me still.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 3/4] tests: Run the iotests during "make check" again
  2019-07-16 15:37   ` Max Reitz
@ 2019-07-17  9:41     ` Thomas Huth
  0 siblings, 0 replies; 21+ messages in thread
From: Thomas Huth @ 2019-07-17  9:41 UTC (permalink / raw)
  To: Max Reitz, qemu-devel; +Cc: Kevin Wolf, Alex Bennée, qemu-block

[-- Attachment #1: Type: text/plain, Size: 2638 bytes --]

On 16/07/2019 17.37, Max Reitz wrote:
> On 16.07.19 14:28, Thomas Huth wrote:
>> People often forget to run the iotests before submitting patches or pull
>> requests - this is likely due to the fact that we do not run the tests
>> during our mandatory "make check" tests yet. Now that we've got a proper
>> "auto" group of iotests that should be fine to run in every environment,
>> we can enable the iotests during "make check" again by running the "auto"
>> tests by default from the check-block.sh script.
>>
>> Some cases still need to be checked first, though: iotests need bash and
>> GNU sed (otherwise they fail), and if gprof is enabled, it spoils the
>> output of some test cases causing them to fail. So if we detect that one
>> of the required programs is missing or that gprof is enabled, we still
>> have to skip the iotests to avoid failures.
>>
>> And finally, since we are using check-block.sh now again, this patch also
>> removes the qemu-iotests-quick.sh script since we do not need that anymore
>> (and having two shell wrapper scripts around the block tests seems rather
>> confusing than helpful).
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> [AJB: -makecheck to check-block.sh, move check-block to start and gate it]
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  tests/Makefile.include      |  8 +++----
>>  tests/check-block.sh        | 44 ++++++++++++++++++++++++++++---------
>>  tests/qemu-iotests-quick.sh |  8 -------
>>  3 files changed, 38 insertions(+), 22 deletions(-)
>>  delete mode 100755 tests/qemu-iotests-quick.sh
>>
>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index fd7fdb8658..e868dc1b9c 100644
>> --- a/tests/Makefile.include
>> +++ b/tests/Makefile.include
> 
> [...]
> 
>> @@ -1092,8 +1092,8 @@ clean-tcg: $(CLEAN_TCG_TARGET_RULES)
>>  
>>  QEMU_IOTESTS_HELPERS-$(call land,$(CONFIG_SOFTMMU),$(CONFIG_LINUX)) = tests/qemu-iotests/socket_scm_helper$(EXESUF)
>>  
>> -.PHONY: check-tests/qemu-iotests-quick.sh
>> -check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF) qemu-io$(EXESUF) qemu-nbd$(EXESUF) $(QEMU_IOTESTS_HELPERS-y)
>> +.PHONY: check-tests/check-block.sh
>> +check-tests/check-block.sh: tests/check-block.sh qemu-img$(EXESUF) qemu-io$(EXESUF) qemu-nbd$(EXESUF) $(QEMU_IOTESTS_HELPERS-y)
> 
> Does this need to depend on a full qemu binary, too?

Yes, I guess so ... otherwise you can not run "make check-block"
immediately after doing a "configure" in a fresh directory.

I'll try to figure out the necessary Makefile magic for this...

 Thomas


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/4] tests/qemu-iotests/group: Remove some more tests from the "auto" group
  2019-07-16 16:12               ` Max Reitz
@ 2019-07-18  7:42                 ` Thomas Huth
  0 siblings, 0 replies; 21+ messages in thread
From: Thomas Huth @ 2019-07-18  7:42 UTC (permalink / raw)
  To: Max Reitz, Eric Blake, qemu-devel
  Cc: Kevin Wolf, Alex Bennée, qemu-block

[-- Attachment #1: Type: text/plain, Size: 3985 bytes --]

On 16/07/2019 18.12, Max Reitz wrote:
> On 16.07.19 18:09, Eric Blake wrote:
>> On 7/16/19 10:58 AM, Max Reitz wrote:
[...]
>>>> Is there any way to create our sockets somewhere under /tmp instead of
>>>> inside tests/qemu-iotests, so that we have a shorter filename for
>>>> sockets no matter how deep in the file hierarchy the tests themselves live?
>>>>
>>>> Also, at one point, we tossed around the idea of
>>>> s/qemu-iotests/iotests/, to shave off 5 characters that don't really add
>>>> anything.
>>>
>>> I’d personally rather just skip the iotests if we detect such a silly
>>> OS, but maybe that’s just me.
>>
>> It's a rather unfortunate limit, but it's not all that silly (the limit
>> is based on the fact that struct sockaddr has to fit inside a nice
>> power-of-2 structure somewhere in the kernel, and enough else is used
>> that you really are left with just 104/108 bytes, or even 92 bytes if
>> you use HP-UX 11).  POSIX does not place a minimum length on sun_path,
>> but I know of no system that does not allow at least 92 bytes, if you
>> are aiming for a portably-small name.
> 
> It does come to me as a surprise that the sockaddr is a path instead of,
> say, an inode number.
> 
> But shaving off the “qemu-” seems like the wrong approach to me still.

Anyway, it would be great if someone who's familiar with the code could
find a proper fix for this problem. FWIW, it also happens on Linux:

mkdir /tmp/0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789
cd /tmp/0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789/
~/path/to/qemu/configure --target-list=x86_64-softmmu
make -j8
cd tests/qemu-iotests/
./check -g quick -qcow2
...
058      fail       [09:29:33] [09:29:34]                    output mismatch (see 058.out.bad)
--- /home/thuth/devel/qemu/tests/qemu-iotests/058.out	2019-04-23 16:43:12.000000000 +0200
+++ /tmp/0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789/tests/qemu-iotests/058.out.bad	2019-07-18 09:29:34.305070819 +0200
@@ -17,18 +17,22 @@
 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 4096/4096 bytes at offset 8192
 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-nbd: UNIX socket path '/tmp/0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789/tests/qemu-iotests/scratch/qemu-nbd.sock' is too long
+Path must be less than 108 bytes
...
065      fail       [09:29:38] [09:29:38]                    output mismatch (see 065.out.bad)
--- /home/thuth/devel/qemu/tests/qemu-iotests/065.out	2019-04-23 16:43:12.000000000 +0200
+++ /tmp/0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789/tests/qemu-iotests/065.out.bad	2019-07-18 09:29:38.568118520 +0200
@@ -1,5 +1,41 @@
-........
+....E..E
+======================================================================
+ERROR: test_qmp (__main__.TestQCow3LazyQMP)
+----------------------------------------------------------------------
+Traceback (most recent call last):
+  File "065", line 75, in setUp
+    self.vm.launch()
+  File "/home/thuth/devel/qemu/tests/qemu-iotests/../../python/qemu/machine.py", line 294, in launch
+    self._launch()
+  File "/home/thuth/devel/qemu/tests/qemu-iotests/../../python/qemu/machine.py", line 311, in _launch
+    self._pre_launch()
+  File "/home/thuth/devel/qemu/tests/qemu-iotests/../../python/qemu/qtest.py", line 103, in _pre_launch
+    super(QEMUQtestMachine, self)._pre_launch()
+  File "/home/thuth/devel/qemu/tests/qemu-iotests/../../python/qemu/machine.py", line 262, in _pre_launch
+    server=True)
+  File "/home/thuth/devel/qemu/tests/qemu-iotests/../../python/qemu/qmp.py", line 60, in __init__
+    self.__sock.bind(self.__address)
+OSError: AF_UNIX path too long
...

 Thomas


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2019-07-18  7:42 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-16 12:28 [Qemu-devel] [PATCH v2 0/4] Enable iotests during "make check" Thomas Huth
2019-07-16 12:28 ` [Qemu-devel] [PATCH v2 1/4] tests/qemu-iotests/check: Allow tests without groups Thomas Huth
2019-07-16 15:27   ` Max Reitz
2019-07-16 15:52   ` Alex Bennée
2019-07-16 12:28 ` [Qemu-devel] [PATCH v2 2/4] tests/qemu-iotests/group: Remove some more tests from the "auto" group Thomas Huth
2019-07-16 15:26   ` Max Reitz
2019-07-16 15:31     ` Thomas Huth
2019-07-16 15:44       ` Max Reitz
2019-07-16 15:45         ` Max Reitz
2019-07-16 15:56         ` Eric Blake
2019-07-16 15:58           ` Max Reitz
2019-07-16 16:09             ` Eric Blake
2019-07-16 16:12               ` Max Reitz
2019-07-18  7:42                 ` Thomas Huth
2019-07-16 12:28 ` [Qemu-devel] [PATCH v2 3/4] tests: Run the iotests during "make check" again Thomas Huth
2019-07-16 15:37   ` Max Reitz
2019-07-17  9:41     ` Thomas Huth
2019-07-16 12:28 ` [Qemu-devel] [PATCH v2 4/4] gitlab-ci: Remove qcow2 tests that are handled by "make check" already Thomas Huth
2019-07-16 15:41   ` Max Reitz
2019-07-16 15:51     ` Thomas Huth
2019-07-16 15:57       ` Max Reitz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).