[PATCH] iotests/026: Move v3-exclusive test to new file

[PATCH] iotests/026: Move v3-exclusive test to new file

[Max Reitz]

data_file does not work with v2, and we probably want 026 to keep
working for v2 images.  Thus, open a new file for v3-exclusive error
path test cases.

Fixes: 81311255f217859413c94f2cd9cebf2684bbda94
       (“iotests/026: Test EIO on allocation in a data-file”)
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/026             | 31 -----------
 tests/qemu-iotests/026.out         |  6 --
 tests/qemu-iotests/026.out.nocache |  6 --
 tests/qemu-iotests/289             | 89 ++++++++++++++++++++++++++++++
 tests/qemu-iotests/289.out         |  8 +++
 tests/qemu-iotests/group           |  1 +
 6 files changed, 98 insertions(+), 43 deletions(-)
 create mode 100755 tests/qemu-iotests/289
 create mode 100644 tests/qemu-iotests/289.out

diff --git a/tests/qemu-iotests/026 b/tests/qemu-iotests/026
index b05a4692cf..b9713eb591 100755
--- a/tests/qemu-iotests/026
+++ b/tests/qemu-iotests/026
@@ -240,37 +240,6 @@ $QEMU_IO -c "write 0 $CLUSTER_SIZE" "$BLKDBG_TEST_IMG" | _filter_qemu_io
 
 _check_test_img
 
-echo
-echo === Avoid freeing external data clusters on failure ===
-echo
-
-# Similar test as the last one, except we test what happens when there
-# is an error when writing to an external data file instead of when
-# writing to a preallocated zero cluster
-_make_test_img -o "data_file=$TEST_IMG.data_file" $CLUSTER_SIZE
-
-# Put blkdebug above the data-file, and a raw node on top of that so
-# that blkdebug will see a write_aio event and emit an error
-$QEMU_IO -c "write 0 $CLUSTER_SIZE" \
-    "json:{
-         'driver': 'qcow2',
-         'file': { 'driver': 'file', 'filename': '$TEST_IMG' },
-         'data-file': {
-             'driver': 'raw',
-             'file': {
-                 'driver': 'blkdebug',
-                 'config': '$TEST_DIR/blkdebug.conf',
-                 'image': {
-                     'driver': 'file',
-                     'filename': '$TEST_IMG.data_file'
-                 }
-             }
-         }
-     }" \
-    | _filter_qemu_io
-
-_check_test_img
-
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/026.out b/tests/qemu-iotests/026.out
index c1b3b58482..83989996ff 100644
--- a/tests/qemu-iotests/026.out
+++ b/tests/qemu-iotests/026.out
@@ -653,10 +653,4 @@ wrote 1024/1024 bytes at offset 0
 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 write failed: Input/output error
 No errors were found on the image.
-
-=== Avoid freeing external data clusters on failure ===
-
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1024 data_file=TEST_DIR/t.IMGFMT.data_file
-write failed: Input/output error
-No errors were found on the image.
 *** done
diff --git a/tests/qemu-iotests/026.out.nocache b/tests/qemu-iotests/026.out.nocache
index 8d5001648a..9359d26d7e 100644
--- a/tests/qemu-iotests/026.out.nocache
+++ b/tests/qemu-iotests/026.out.nocache
@@ -661,10 +661,4 @@ wrote 1024/1024 bytes at offset 0
 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 write failed: Input/output error
 No errors were found on the image.
-
-=== Avoid freeing external data clusters on failure ===
-
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1024 data_file=TEST_DIR/t.IMGFMT.data_file
-write failed: Input/output error
-No errors were found on the image.
 *** done
diff --git a/tests/qemu-iotests/289 b/tests/qemu-iotests/289
new file mode 100755
index 0000000000..1c11d4030e
--- /dev/null
+++ b/tests/qemu-iotests/289
@@ -0,0 +1,89 @@
+#!/usr/bin/env bash
+#
+# qcow2 v3-exclusive error path testing
+# (026 tests paths common to v2 and v3)
+#
+# Copyright (C) 2020 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+seq=$(basename $0)
+echo "QA output created by $seq"
+
+status=1	# failure is the default!
+
+_cleanup()
+{
+    _cleanup_test_img
+    rm "$TEST_DIR/blkdebug.conf"
+    rm -f "$TEST_IMG.data_file"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.pattern
+
+_supported_fmt qcow2
+_supported_proto file
+# This is a v3-exclusive test;
+# As for data_file, error paths often very much depend on whether
+# there is an external data file or not; so we create one exactly when
+# we want to test it
+_unsupported_imgopts 'compat=0.10' data_file
+
+echo
+echo === Avoid freeing external data clusters on failure ===
+echo
+
+cat > "$TEST_DIR/blkdebug.conf" <<EOF
+[inject-error]
+event = "write_aio"
+errno = "5"
+once = "on"
+EOF
+
+# Test what happens when there is an error when writing to an external
+# data file instead of when writing to a preallocated zero cluster
+_make_test_img -o "data_file=$TEST_IMG.data_file" 64k
+
+# Put blkdebug above the data-file, and a raw node on top of that so
+# that blkdebug will see a write_aio event and emit an error.  This
+# will then trigger the alloc abort code, which we want to test here.
+$QEMU_IO -c "write 0 64k" \
+    "json:{
+         'driver': 'qcow2',
+         'file': { 'driver': 'file', 'filename': '$TEST_IMG' },
+         'data-file': {
+             'driver': 'raw',
+             'file': {
+                 'driver': 'blkdebug',
+                 'config': '$TEST_DIR/blkdebug.conf',
+                 'image': {
+                     'driver': 'file',
+                     'filename': '$TEST_IMG.data_file'
+                 }
+             }
+         }
+     }" \
+    | _filter_qemu_io
+
+_check_test_img
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/289.out b/tests/qemu-iotests/289.out
new file mode 100644
index 0000000000..e54e2629d4
--- /dev/null
+++ b/tests/qemu-iotests/289.out
@@ -0,0 +1,8 @@
+QA output created by 289
+
+=== Avoid freeing external data clusters on failure ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=65536 data_file=TEST_DIR/t.IMGFMT.data_file
+write failed: Input/output error
+No errors were found on the image.
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 559edc139a..a898fe2c26 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -294,3 +294,4 @@
 284 rw
 286 rw quick
 288 quick
+289 rw quick
-- 
2.24.1

Re: [PATCH] iotests/026: Move v3-exclusive test to new file

[John Snow]

On 3/11/20 10:07 AM, Max Reitz wrote:
> data_file does not work with v2, and we probably want 026 to keep
> working for v2 images.  Thus, open a new file for v3-exclusive error
> path test cases.
> 
> Fixes: 81311255f217859413c94f2cd9cebf2684bbda94
>        (“iotests/026: Test EIO on allocation in a data-file”)
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Let me start this reply with something good, or at least something
that's not bad. It's value neutral at worst.

Reviewed-by: John Snow <jsnow@redhat.com>
Tested-by: John Snow <jsnow@redhat.com>


Now, let's get cracking on some prime nonsense.

I assume this patch is still 'pending'. Here's a complete tangent
unrelated to your patch in every single way:

What's the best way to use patchew to see series that are "pending" in
some way? I'd like to:

- Search only the block list (to:qemu-block@nongnu.org. I assume this
catches CCs too.)
- Exclude series that are merged (-is:merged)
- Exclude obsoleted series (-is:obsolete)

This gets a bit closer to things that are interesting in some way --
give or take some fuzziness with patchew's detection of "merged" or
"obsoleted" sometimes.

- Exclude pull requests. (-is:pull seems broken, actually.)
- Exclude reviewed series (-is:reviewed -- what does patchew consider
'reviewed'? does this mean fully reviewed, or any reviews?)

This gives me something a bit more useful.

- Exclude 'expired' series. I use 30 days as a mental model for this. It
might be nice to formalize this and mark patches that received no
replies and didn't detect any other state change as "expired" and send
an autoreply from the bot.

(I.e., patches that are complete, applied, passed CI, were not
obsoleted, did not appear to be merged, and received no replies from
anyone except the patch author)


("Hi, this patch received no replies from anyone except the author (you)
for 30 days. The series is being dropped from the pending queue and is
being marked expired. If the patches are still important, please rebase
them and re-send to the list.

Please use scripts/get_maintainers.pl to identify candidate maintainers
and reviewers and make sure they are CC'd.

This series appears to touch files owned by the following maintainers:
- Blah
- Etc
- And so on

For more information on the contribution process, please visit:
<wiki links to contribution guides, etc>")

We don't have anything like that, so age:<30d suffices. Alright, this
list is starting to look *pretty* decent.

project:QEMU to:qemu-block@nongnu.org not:obsolete not:merged
-is:reviewed age:<30d

Lastly, maybe we can exclude series that don't have replies yet. It's
not clear to patchew which replies are:

- Unrelated comments, like this one here
- Requests for a change
- A question for the submitter
- A softly-worded N-A-C-K

and without a concept of designated reviewer, perhaps lack of replies is
good evidence that the series is untouched and needs someone to 'pick it
up'; (-has:replies)

https://patchew.org/search?q=project%3AQEMU+to%3Aqemu-block%40nongnu.org+not%3Aobsolete+not%3Amerged+-is%3Areviewed+age%3A%3C30d+-has%3Areplies

Alright, that's pretty good, actually.

OK, yes, this patch still needs love as far as patchew understands.

> ---
>  tests/qemu-iotests/026             | 31 -----------
>  tests/qemu-iotests/026.out         |  6 --
>  tests/qemu-iotests/026.out.nocache |  6 --
>  tests/qemu-iotests/289             | 89 ++++++++++++++++++++++++++++++
>  tests/qemu-iotests/289.out         |  8 +++
>  tests/qemu-iotests/group           |  1 +
>  6 files changed, 98 insertions(+), 43 deletions(-)
>  create mode 100755 tests/qemu-iotests/289
>  create mode 100644 tests/qemu-iotests/289.out
> 
> diff --git a/tests/qemu-iotests/026 b/tests/qemu-iotests/026
> index b05a4692cf..b9713eb591 100755
> --- a/tests/qemu-iotests/026
> +++ b/tests/qemu-iotests/026
> @@ -240,37 +240,6 @@ $QEMU_IO -c "write 0 $CLUSTER_SIZE" "$BLKDBG_TEST_IMG" | _filter_qemu_io
>  
>  _check_test_img
>  
> -echo
> -echo === Avoid freeing external data clusters on failure ===
> -echo
> -
> -# Similar test as the last one, except we test what happens when there
> -# is an error when writing to an external data file instead of when
> -# writing to a preallocated zero cluster
> -_make_test_img -o "data_file=$TEST_IMG.data_file" $CLUSTER_SIZE
> -
> -# Put blkdebug above the data-file, and a raw node on top of that so
> -# that blkdebug will see a write_aio event and emit an error
> -$QEMU_IO -c "write 0 $CLUSTER_SIZE" \
> -    "json:{
> -         'driver': 'qcow2',
> -         'file': { 'driver': 'file', 'filename': '$TEST_IMG' },
> -         'data-file': {
> -             'driver': 'raw',
> -             'file': {
> -                 'driver': 'blkdebug',
> -                 'config': '$TEST_DIR/blkdebug.conf',
> -                 'image': {
> -                     'driver': 'file',
> -                     'filename': '$TEST_IMG.data_file'
> -                 }
> -             }
> -         }
> -     }" \
> -    | _filter_qemu_io
> -
> -_check_test_img
> -
>  # success, all done
>  echo "*** done"
>  rm -f $seq.full
> diff --git a/tests/qemu-iotests/026.out b/tests/qemu-iotests/026.out
> index c1b3b58482..83989996ff 100644
> --- a/tests/qemu-iotests/026.out
> +++ b/tests/qemu-iotests/026.out
> @@ -653,10 +653,4 @@ wrote 1024/1024 bytes at offset 0
>  1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>  write failed: Input/output error
>  No errors were found on the image.
> -
> -=== Avoid freeing external data clusters on failure ===
> -
> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1024 data_file=TEST_DIR/t.IMGFMT.data_file
> -write failed: Input/output error
> -No errors were found on the image.
>  *** done
> diff --git a/tests/qemu-iotests/026.out.nocache b/tests/qemu-iotests/026.out.nocache
> index 8d5001648a..9359d26d7e 100644
> --- a/tests/qemu-iotests/026.out.nocache
> +++ b/tests/qemu-iotests/026.out.nocache
> @@ -661,10 +661,4 @@ wrote 1024/1024 bytes at offset 0
>  1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>  write failed: Input/output error
>  No errors were found on the image.
> -
> -=== Avoid freeing external data clusters on failure ===
> -
> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1024 data_file=TEST_DIR/t.IMGFMT.data_file
> -write failed: Input/output error
> -No errors were found on the image.
>  *** done
> diff --git a/tests/qemu-iotests/289 b/tests/qemu-iotests/289
> new file mode 100755
> index 0000000000..1c11d4030e
> --- /dev/null
> +++ b/tests/qemu-iotests/289
> @@ -0,0 +1,89 @@
> +#!/usr/bin/env bash
> +#
> +# qcow2 v3-exclusive error path testing
> +# (026 tests paths common to v2 and v3)
> +#
> +# Copyright (C) 2020 Red Hat, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +seq=$(basename $0)
> +echo "QA output created by $seq"
> +
> +status=1	# failure is the default!
> +
> +_cleanup()
> +{
> +    _cleanup_test_img
> +    rm "$TEST_DIR/blkdebug.conf"
> +    rm -f "$TEST_IMG.data_file"
> +}
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common.rc
> +. ./common.filter
> +. ./common.pattern
> +
> +_supported_fmt qcow2
> +_supported_proto file
> +# This is a v3-exclusive test;
> +# As for data_file, error paths often very much depend on whether
> +# there is an external data file or not; so we create one exactly when
> +# we want to test it
> +_unsupported_imgopts 'compat=0.10' data_file
> +
> +echo
> +echo === Avoid freeing external data clusters on failure ===
> +echo
> +
> +cat > "$TEST_DIR/blkdebug.conf" <<EOF
> +[inject-error]
> +event = "write_aio"
> +errno = "5"
> +once = "on"
> +EOF
> +
> +# Test what happens when there is an error when writing to an external
> +# data file instead of when writing to a preallocated zero cluster
> +_make_test_img -o "data_file=$TEST_IMG.data_file" 64k
> +
> +# Put blkdebug above the data-file, and a raw node on top of that so
> +# that blkdebug will see a write_aio event and emit an error.  This
> +# will then trigger the alloc abort code, which we want to test here.
> +$QEMU_IO -c "write 0 64k" \
> +    "json:{
> +         'driver': 'qcow2',
> +         'file': { 'driver': 'file', 'filename': '$TEST_IMG' },
> +         'data-file': {
> +             'driver': 'raw',
> +             'file': {
> +                 'driver': 'blkdebug',
> +                 'config': '$TEST_DIR/blkdebug.conf',
> +                 'image': {
> +                     'driver': 'file',
> +                     'filename': '$TEST_IMG.data_file'
> +                 }
> +             }
> +         }
> +     }" \
> +    | _filter_qemu_io
> +
> +_check_test_img
> +
> +# success, all done
> +echo "*** done"
> +rm -f $seq.full
> +status=0
> diff --git a/tests/qemu-iotests/289.out b/tests/qemu-iotests/289.out
> new file mode 100644
> index 0000000000..e54e2629d4
> --- /dev/null
> +++ b/tests/qemu-iotests/289.out
> @@ -0,0 +1,8 @@
> +QA output created by 289
> +
> +=== Avoid freeing external data clusters on failure ===
> +
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=65536 data_file=TEST_DIR/t.IMGFMT.data_file
> +write failed: Input/output error
> +No errors were found on the image.
> +*** done
> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
> index 559edc139a..a898fe2c26 100644
> --- a/tests/qemu-iotests/group
> +++ b/tests/qemu-iotests/group
> @@ -294,3 +294,4 @@
>  284 rw
>  286 rw quick
>  288 quick
> +289 rw quick
> 

-- 
—js

Re: [PATCH] iotests/026: Move v3-exclusive test to new file

[Max Reitz]

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

On 12.03.20 23:19, John Snow wrote:
> 
> 
> On 3/11/20 10:07 AM, Max Reitz wrote:
>> data_file does not work with v2, and we probably want 026 to keep
>> working for v2 images.  Thus, open a new file for v3-exclusive error
>> path test cases.
>>
>> Fixes: 81311255f217859413c94f2cd9cebf2684bbda94
>>        (“iotests/026: Test EIO on allocation in a data-file”)
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
> 
> Let me start this reply with something good, or at least something
> that's not bad. It's value neutral at worst.
> 
> Reviewed-by: John Snow <jsnow@redhat.com>
> Tested-by: John Snow <jsnow@redhat.com>

Thanks. :)

> Now, let's get cracking on some prime nonsense.
> 
> I assume this patch is still 'pending'. Here's a complete tangent
> unrelated to your patch in every single way:

Reasonable, but it’s a bit of a shame you bury it here.  I feel like
this makes it unlikely to reach the people you want to reach.

> What's the best way to use patchew to see series that are "pending" in
> some way? I'd like to:
> 
> - Search only the block list (to:qemu-block@nongnu.org. I assume this
> catches CCs too.)
> - Exclude series that are merged (-is:merged)
> - Exclude obsoleted series (-is:obsolete)
> 
> This gets a bit closer to things that are interesting in some way --
> give or take some fuzziness with patchew's detection of "merged" or
> "obsoleted" sometimes.
> 
> - Exclude pull requests. (-is:pull seems broken, actually.)
> - Exclude reviewed series (-is:reviewed -- what does patchew consider
> 'reviewed'? does this mean fully reviewed, or any reviews?)
> 
> This gives me something a bit more useful.
> 
> - Exclude 'expired' series. I use 30 days as a mental model for this. It
> might be nice to formalize this and mark patches that received no
> replies and didn't detect any other state change as "expired" and send
> an autoreply from the bot.
> 
> (I.e., patches that are complete, applied, passed CI, were not
> obsoleted, did not appear to be merged, and received no replies from
> anyone except the patch author)
> 
> 
> ("Hi, this patch received no replies from anyone except the author (you)
> for 30 days. The series is being dropped from the pending queue and is
> being marked expired. If the patches are still important, please rebase
> them and re-send to the list.
> 
> Please use scripts/get_maintainers.pl to identify candidate maintainers
> and reviewers and make sure they are CC'd.
> 
> This series appears to touch files owned by the following maintainers:
> - Blah
> - Etc
> - And so on
> 
> For more information on the contribution process, please visit:
> <wiki links to contribution guides, etc>")
> 
> We don't have anything like that, so age:<30d suffices. Alright, this
> list is starting to look *pretty* decent.
> 
> project:QEMU to:qemu-block@nongnu.org not:obsolete not:merged
> -is:reviewed age:<30d
> 
> Lastly, maybe we can exclude series that don't have replies yet.

Maybe Patchew should ping these after 14 days or so.

That’s about the only thing I can contribute, because I don’t really use
Patchew myself...  I keep patches in a subfolder of my inbox on unread;
and I keep my own pending series in a separate folder.  (Before I did
that, I was much more prone to forgetting my own patches rather than
someone else’s.)

Max

> It's
> not clear to patchew which replies are:
> 
> - Unrelated comments, like this one here
> - Requests for a change
> - A question for the submitter
> - A softly-worded N-A-C-K
> 
> and without a concept of designated reviewer, perhaps lack of replies is
> good evidence that the series is untouched and needs someone to 'pick it
> up'; (-has:replies)
> 
> https://patchew.org/search?q=project%3AQEMU+to%3Aqemu-block%40nongnu.org+not%3Aobsolete+not%3Amerged+-is%3Areviewed+age%3A%3C30d+-has%3Areplies
> 
> Alright, that's pretty good, actually.
> 
> OK, yes, this patch still needs love as far as patchew understands.
> 
>> ---
>>  tests/qemu-iotests/026             | 31 -----------
>>  tests/qemu-iotests/026.out         |  6 --
>>  tests/qemu-iotests/026.out.nocache |  6 --
>>  tests/qemu-iotests/289             | 89 ++++++++++++++++++++++++++++++
>>  tests/qemu-iotests/289.out         |  8 +++
>>  tests/qemu-iotests/group           |  1 +
>>  6 files changed, 98 insertions(+), 43 deletions(-)
>>  create mode 100755 tests/qemu-iotests/289
>>  create mode 100644 tests/qemu-iotests/289.out
>>
>> diff --git a/tests/qemu-iotests/026 b/tests/qemu-iotests/026
>> index b05a4692cf..b9713eb591 100755
>> --- a/tests/qemu-iotests/026
>> +++ b/tests/qemu-iotests/026
>> @@ -240,37 +240,6 @@ $QEMU_IO -c "write 0 $CLUSTER_SIZE" "$BLKDBG_TEST_IMG" | _filter_qemu_io
>>  
>>  _check_test_img
>>  
>> -echo
>> -echo === Avoid freeing external data clusters on failure ===
>> -echo
>> -
>> -# Similar test as the last one, except we test what happens when there
>> -# is an error when writing to an external data file instead of when
>> -# writing to a preallocated zero cluster
>> -_make_test_img -o "data_file=$TEST_IMG.data_file" $CLUSTER_SIZE
>> -
>> -# Put blkdebug above the data-file, and a raw node on top of that so
>> -# that blkdebug will see a write_aio event and emit an error
>> -$QEMU_IO -c "write 0 $CLUSTER_SIZE" \
>> -    "json:{
>> -         'driver': 'qcow2',
>> -         'file': { 'driver': 'file', 'filename': '$TEST_IMG' },
>> -         'data-file': {
>> -             'driver': 'raw',
>> -             'file': {
>> -                 'driver': 'blkdebug',
>> -                 'config': '$TEST_DIR/blkdebug.conf',
>> -                 'image': {
>> -                     'driver': 'file',
>> -                     'filename': '$TEST_IMG.data_file'
>> -                 }
>> -             }
>> -         }
>> -     }" \
>> -    | _filter_qemu_io
>> -
>> -_check_test_img
>> -
>>  # success, all done
>>  echo "*** done"
>>  rm -f $seq.full
>> diff --git a/tests/qemu-iotests/026.out b/tests/qemu-iotests/026.out
>> index c1b3b58482..83989996ff 100644
>> --- a/tests/qemu-iotests/026.out
>> +++ b/tests/qemu-iotests/026.out
>> @@ -653,10 +653,4 @@ wrote 1024/1024 bytes at offset 0
>>  1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>>  write failed: Input/output error
>>  No errors were found on the image.
>> -
>> -=== Avoid freeing external data clusters on failure ===
>> -
>> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1024 data_file=TEST_DIR/t.IMGFMT.data_file
>> -write failed: Input/output error
>> -No errors were found on the image.
>>  *** done
>> diff --git a/tests/qemu-iotests/026.out.nocache b/tests/qemu-iotests/026.out.nocache
>> index 8d5001648a..9359d26d7e 100644
>> --- a/tests/qemu-iotests/026.out.nocache
>> +++ b/tests/qemu-iotests/026.out.nocache
>> @@ -661,10 +661,4 @@ wrote 1024/1024 bytes at offset 0
>>  1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>>  write failed: Input/output error
>>  No errors were found on the image.
>> -
>> -=== Avoid freeing external data clusters on failure ===
>> -
>> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1024 data_file=TEST_DIR/t.IMGFMT.data_file
>> -write failed: Input/output error
>> -No errors were found on the image.
>>  *** done
>> diff --git a/tests/qemu-iotests/289 b/tests/qemu-iotests/289
>> new file mode 100755
>> index 0000000000..1c11d4030e
>> --- /dev/null
>> +++ b/tests/qemu-iotests/289
>> @@ -0,0 +1,89 @@
>> +#!/usr/bin/env bash
>> +#
>> +# qcow2 v3-exclusive error path testing
>> +# (026 tests paths common to v2 and v3)
>> +#
>> +# Copyright (C) 2020 Red Hat, Inc.
>> +#
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 2 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> +#
>> +
>> +seq=$(basename $0)
>> +echo "QA output created by $seq"
>> +
>> +status=1	# failure is the default!
>> +
>> +_cleanup()
>> +{
>> +    _cleanup_test_img
>> +    rm "$TEST_DIR/blkdebug.conf"
>> +    rm -f "$TEST_IMG.data_file"
>> +}
>> +trap "_cleanup; exit \$status" 0 1 2 3 15
>> +
>> +# get standard environment, filters and checks
>> +. ./common.rc
>> +. ./common.filter
>> +. ./common.pattern
>> +
>> +_supported_fmt qcow2
>> +_supported_proto file
>> +# This is a v3-exclusive test;
>> +# As for data_file, error paths often very much depend on whether
>> +# there is an external data file or not; so we create one exactly when
>> +# we want to test it
>> +_unsupported_imgopts 'compat=0.10' data_file
>> +
>> +echo
>> +echo === Avoid freeing external data clusters on failure ===
>> +echo
>> +
>> +cat > "$TEST_DIR/blkdebug.conf" <<EOF
>> +[inject-error]
>> +event = "write_aio"
>> +errno = "5"
>> +once = "on"
>> +EOF
>> +
>> +# Test what happens when there is an error when writing to an external
>> +# data file instead of when writing to a preallocated zero cluster
>> +_make_test_img -o "data_file=$TEST_IMG.data_file" 64k
>> +
>> +# Put blkdebug above the data-file, and a raw node on top of that so
>> +# that blkdebug will see a write_aio event and emit an error.  This
>> +# will then trigger the alloc abort code, which we want to test here.
>> +$QEMU_IO -c "write 0 64k" \
>> +    "json:{
>> +         'driver': 'qcow2',
>> +         'file': { 'driver': 'file', 'filename': '$TEST_IMG' },
>> +         'data-file': {
>> +             'driver': 'raw',
>> +             'file': {
>> +                 'driver': 'blkdebug',
>> +                 'config': '$TEST_DIR/blkdebug.conf',
>> +                 'image': {
>> +                     'driver': 'file',
>> +                     'filename': '$TEST_IMG.data_file'
>> +                 }
>> +             }
>> +         }
>> +     }" \
>> +    | _filter_qemu_io
>> +
>> +_check_test_img
>> +
>> +# success, all done
>> +echo "*** done"
>> +rm -f $seq.full
>> +status=0
>> diff --git a/tests/qemu-iotests/289.out b/tests/qemu-iotests/289.out
>> new file mode 100644
>> index 0000000000..e54e2629d4
>> --- /dev/null
>> +++ b/tests/qemu-iotests/289.out
>> @@ -0,0 +1,8 @@
>> +QA output created by 289
>> +
>> +=== Avoid freeing external data clusters on failure ===
>> +
>> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=65536 data_file=TEST_DIR/t.IMGFMT.data_file
>> +write failed: Input/output error
>> +No errors were found on the image.
>> +*** done
>> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
>> index 559edc139a..a898fe2c26 100644
>> --- a/tests/qemu-iotests/group
>> +++ b/tests/qemu-iotests/group
>> @@ -294,3 +294,4 @@
>>  284 rw
>>  286 rw quick
>>  288 quick
>> +289 rw quick
>>
> 



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

How do I use patchew to be a good block-devel citizen? (was: Re: [PATCH] iotests/026: Move v3-exclusive test to new file)

[John Snow]

On 3/23/20 8:23 AM, Max Reitz wrote:
> On 12.03.20 23:19, John Snow wrote:
>>
>>
>> On 3/11/20 10:07 AM, Max Reitz wrote:
>>> data_file does not work with v2, and we probably want 026 to keep
>>> working for v2 images.  Thus, open a new file for v3-exclusive error
>>> path test cases.
>>>
>>> Fixes: 81311255f217859413c94f2cd9cebf2684bbda94
>>>        (“iotests/026: Test EIO on allocation in a data-file”)
>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>>
>> Let me start this reply with something good, or at least something
>> that's not bad. It's value neutral at worst.
>>
>> Reviewed-by: John Snow <jsnow@redhat.com>
>> Tested-by: John Snow <jsnow@redhat.com>
> 
> Thanks. :)
> 
>> Now, let's get cracking on some prime nonsense.
>>
>> I assume this patch is still 'pending'. Here's a complete tangent
>> unrelated to your patch in every single way:
> 
> Reasonable, but it’s a bit of a shame you bury it here.  I feel like
> this makes it unlikely to reach the people you want to reach.
> 

I'm not sure if I was trying to reach anyone, exactly, but maybe I
should have been. Let's recirculate this to a wider audience.

So, directly below, find the $subject question:

>> What's the best way to use patchew to see series that are "pending" in
>> some way? I'd like to:
>>
>> - Search only the block list (to:qemu-block@nongnu.org. I assume this
>> catches CCs too.)
>> - Exclude series that are merged (-is:merged)
>> - Exclude obsoleted series (-is:obsolete)
>>
>> This gets a bit closer to things that are interesting in some way --
>> give or take some fuzziness with patchew's detection of "merged" or
>> "obsoleted" sometimes.
>>
>> - Exclude pull requests. (-is:pull seems broken, actually.)
>> - Exclude reviewed series (-is:reviewed -- what does patchew consider
>> 'reviewed'? does this mean fully reviewed, or any reviews?)
>>
>> This gives me something a bit more useful.
>>
>> - Exclude 'expired' series. I use 30 days as a mental model for this. It
>> might be nice to formalize this and mark patches that received no
>> replies and didn't detect any other state change as "expired" and send
>> an autoreply from the bot.
>>
>> (I.e., patches that are complete, applied, passed CI, were not
>> obsoleted, did not appear to be merged, and received no replies from
>> anyone except the patch author)
>>
>>
>> ("Hi, this patch received no replies from anyone except the author (you)
>> for 30 days. The series is being dropped from the pending queue and is
>> being marked expired. If the patches are still important, please rebase
>> them and re-send to the list.
>>
>> Please use scripts/get_maintainers.pl to identify candidate maintainers
>> and reviewers and make sure they are CC'd.
>>
>> This series appears to touch files owned by the following maintainers:
>> - Blah
>> - Etc
>> - And so on
>>
>> For more information on the contribution process, please visit:
>> <wiki links to contribution guides, etc>")
>>
>> We don't have anything like that, so age:<30d suffices. Alright, this
>> list is starting to look *pretty* decent.
>>
>> project:QEMU to:qemu-block@nongnu.org not:obsolete not:merged
>> -is:reviewed age:<30d
>>
>> Lastly, maybe we can exclude series that don't have replies yet.
> 
> Maybe Patchew should ping these after 14 days or so.
> 
> That’s about the only thing I can contribute, because I don’t really use
> Patchew myself...  I keep patches in a subfolder of my inbox on unread;
> and I keep my own pending series in a separate folder.  (Before I did
> that, I was much more prone to forgetting my own patches rather than
> someone else’s.)
> 
> Max
> 
>> It's
>> not clear to patchew which replies are:
>>
>> - Unrelated comments, like this one here
>> - Requests for a change
>> - A question for the submitter
>> - A softly-worded N-A-C-K
>>
>> and without a concept of designated reviewer, perhaps lack of replies is
>> good evidence that the series is untouched and needs someone to 'pick it
>> up'; (-has:replies)
>>
>> https://patchew.org/search?q=project%3AQEMU+to%3Aqemu-block%40nongnu.org+not%3Aobsolete+not%3Amerged+-is%3Areviewed+age%3A%3C30d+-has%3Areplies
>>
>> Alright, that's pretty good, actually.
>>
>> OK, yes, this patch still needs love as far as patchew understands.
>>

[Snip: no longer relevant to the topic.]

Re: [PATCH] iotests/026: Move v3-exclusive test to new file

[Max Reitz]

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

On 11.03.20 15:07, Max Reitz wrote:
> data_file does not work with v2, and we probably want 026 to keep
> working for v2 images.  Thus, open a new file for v3-exclusive error
> path test cases.
> 
> Fixes: 81311255f217859413c94f2cd9cebf2684bbda94
>        (“iotests/026: Test EIO on allocation in a data-file”)
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  tests/qemu-iotests/026             | 31 -----------
>  tests/qemu-iotests/026.out         |  6 --
>  tests/qemu-iotests/026.out.nocache |  6 --
>  tests/qemu-iotests/289             | 89 ++++++++++++++++++++++++++++++
>  tests/qemu-iotests/289.out         |  8 +++
>  tests/qemu-iotests/group           |  1 +
>  6 files changed, 98 insertions(+), 43 deletions(-)
>  create mode 100755 tests/qemu-iotests/289
>  create mode 100644 tests/qemu-iotests/289.out

Thanks for the review and test, applied to my block branch.

Max


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