[Qemu-devel] [PULL 0/3] Run iotests during "make check"

[Qemu-devel] [PULL 0/3] Run iotests during "make check"

[Thomas Huth]

 Hi Peter,

the following changes since commit afd760539308a5524accf964107cdb1d54a059e3:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20190816' into staging (2019-08-16 17:21:40 +0100)

are available in the Git repository at:

  https://gitlab.com/huth/qemu.git tags/pull-request-2019-08-17

for you to fetch changes up to 72e031f3b80a421b309ce0d1759b26e428f944db:

  gitlab-ci: Remove qcow2 tests that are handled by "make check" already (2019-08-17 09:06:17 +0200)

----------------------------------------------------------------
- Run the iotest during "make check"
----------------------------------------------------------------

Paolo Bonzini (1):
      block: fix NetBSD qemu-iotests failure

Thomas Huth (2):
      tests: Run the iotests during "make check" again
      gitlab-ci: Remove qcow2 tests that are handled by "make check" already

 .gitlab-ci.yml              | 13 ++++---------
 block/file-posix.c          |  4 ++--
 tests/Makefile.include      | 10 ++++++----
 tests/check-block.sh        | 44 ++++++++++++++++++++++++++++++++++----------
 tests/qemu-iotests-quick.sh |  8 --------
 5 files changed, 46 insertions(+), 33 deletions(-)
 delete mode 100755 tests/qemu-iotests-quick.sh

[Qemu-devel] [PULL 1/3] block: fix NetBSD qemu-iotests failure

[Thomas Huth]

From: Paolo Bonzini <pbonzini@redhat.com>

Opening a block device on NetBSD has an additional step compared to other OSes,
corresponding to raw_normalize_devicepath.  The error message in that function
is slightly different from that in raw_open_common and this was causing spurious
failures in qemu-iotests.  However, in general it is not important to know what
exact step was failing, for example in the qemu-iotests case the error message
contains the fairly unequivocal "No such file or directory" text from strerror.
We can thus fix the failures by standardizing on a single error message for
both raw_open_common and raw_normalize_devicepath; in fact, we can even
use error_setg_file_open to make sure the error message is the same as in
the rest of QEMU.

Message-Id: <20190725095920.28419-1-pbonzini@redhat.com>
Tested-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 block/file-posix.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index b8b4dad553..e41e91e075 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -217,7 +217,7 @@ static int raw_normalize_devicepath(const char **filename, Error **errp)
     fname = *filename;
     dp = strrchr(fname, '/');
     if (lstat(fname, &sb) < 0) {
-        error_setg_errno(errp, errno, "%s: stat failed", fname);
+        error_setg_file_open(errp, errno, fname);
         return -errno;
     }
 
@@ -561,7 +561,7 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
     ret = fd < 0 ? -errno : 0;
 
     if (ret < 0) {
-        error_setg_errno(errp, -ret, "Could not open '%s'", filename);
+        error_setg_file_open(errp, -ret, filename);
         if (ret == -EROFS) {
             ret = -EACCES;
         }
-- 
2.18.1

[Qemu-devel] [PULL 2/3] tests: Run the iotests during "make check" again

[Thomas Huth]

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).

Message-Id: <20190717111947.30356-4-thuth@redhat.com>
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      | 10 +++++----
 tests/check-block.sh        | 44 ++++++++++++++++++++++++++++---------
 tests/qemu-iotests-quick.sh |  8 -------
 3 files changed, 40 insertions(+), 22 deletions(-)
 delete mode 100755 tests/qemu-iotests-quick.sh

diff --git a/tests/Makefile.include b/tests/Makefile.include
index fd7fdb8658..6f02dfcc01 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,10 @@ 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) \
+		$(patsubst %,%/all,$(filter %-softmmu,$(TARGET_DIRS)))
 	$<
 
 .PHONY: $(patsubst %, check-%, $(check-qapi-schema-y))
@@ -1167,7 +1169,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.18.1

Re: [Qemu-devel] [PULL 2/3] tests: Run the iotests during "make check" again

[Paolo Bonzini]

On 17/08/19 10:54, 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).
> 
> Message-Id: <20190717111947.30356-4-thuth@redhat.com>
> 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>

This breaks when sanitizers are enabled.  There are leaks reported,
though I'm not sure if they are real, and in additions the warning lines
break qemu-iotests' output comparison.

Paolo

Re: [Qemu-devel] [PULL 2/3] tests: Run the iotests during "make check" again

[Thomas Huth]

On 8/23/19 12:34 AM, Paolo Bonzini wrote:
> On 17/08/19 10:54, 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).
>>
>> Message-Id: <20190717111947.30356-4-thuth@redhat.com>
>> 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>
> 
> This breaks when sanitizers are enabled.  There are leaks reported,
> though I'm not sure if they are real, and in additions the warning lines
> break qemu-iotests' output comparison.

D'oh, I already thought that it was too easy ;-) I'll have a look at it...

 Thomas

Re: [Qemu-devel] [PULL 2/3] tests: Run the iotests during "make check" again

[Thomas Huth]

On 8/23/19 9:12 AM, Thomas Huth wrote:
> On 8/23/19 12:34 AM, Paolo Bonzini wrote:
>> On 17/08/19 10:54, 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).
>>>
>>> Message-Id: <20190717111947.30356-4-thuth@redhat.com>
>>> 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>
>>
>> This breaks when sanitizers are enabled.  There are leaks reported,
>> though I'm not sure if they are real, and in additions the warning lines
>> break qemu-iotests' output comparison.
> 
> D'oh, I already thought that it was too easy ;-) I'll have a look at it...

If I get that right, the problem only exists when compiling with Clang
and sanitizers enabled - Clang outputs lots of warnings that some things
are not implemented yet and then prints out these leak reports which are
likely false positives...

I've sent a patch now to disable the iotests during "make check" when
sanitizers are enabled. But I wonder whether we should maybe also check
for these incomplete sanitizer implementations in the configure script
to avoid to enable them in this case? (e.g. there are also lots of
warnings during "make check-qtest" in this case, which is somewhat ugly)

 Thomas

[Qemu-devel] [PULL 3/3] gitlab-ci: Remove qcow2 tests that are handled by "make check" already

[Thomas Huth]

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 >= 246 instead.

Message-Id: <20190717111947.30356-5-thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 .gitlab-ci.yml | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index c63bf2f822..cd4c03372b 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -45,15 +45,10 @@ 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
+            122 124 127 129 132 139 142 144 145 147 151 152 155 157 165 194
+            196 197 200 202 203 205 208 209 215 216 218 222 227 234 246 247
+            248 250 254 255 256
 
 build-user:
  script:
-- 
2.18.1

Re: [Qemu-devel] [PULL 0/3] Run iotests during "make check"

[Peter Maydell]

On Sat, 17 Aug 2019 at 09:54, Thomas Huth <thuth@redhat.com> wrote:
>
>  Hi Peter,
>
> the following changes since commit afd760539308a5524accf964107cdb1d54a059e3:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20190816' into staging (2019-08-16 17:21:40 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/huth/qemu.git tags/pull-request-2019-08-17
>
> for you to fetch changes up to 72e031f3b80a421b309ce0d1759b26e428f944db:
>
>   gitlab-ci: Remove qcow2 tests that are handled by "make check" already (2019-08-17 09:06:17 +0200)
>
> ----------------------------------------------------------------
> - Run the iotest during "make check"
> ----------------------------------------------------------------
>
> Paolo Bonzini (1):
>       block: fix NetBSD qemu-iotests failure
>
> Thomas Huth (2):
>       tests: Run the iotests during "make check" again
>       gitlab-ci: Remove qcow2 tests that are handled by "make check" already



Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.2
for any user-visible changes.

-- PMM