iotests: Simplify _filter_img_create() a bit
diff mbox series

Message ID 20200709110205.310942-1-mreitz@redhat.com
State New
Headers show
Series
  • iotests: Simplify _filter_img_create() a bit
Related show

Commit Message

Max Reitz July 9, 2020, 11:02 a.m. UTC
Not only is it a bit stupid to try to filter multi-line "Formatting"
output (because we only need it for a single test, which can easily be
amended to no longer need it), it is also problematic when there can be
output after a "Formatting" line that we do not want to filter as if it
were part of it.

So rename _filter_img_create to _do_filter_img_create, let it filter
only a single line, and let _filter_img_create loop over all input
lines, calling _do_filter_img_create only on those that match
/^Formatting/ (basically, what _filter_img_create_in_qmp did already).
(And fix 020 to work with that.)

Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
Kevin noted that the changes to _filter_img_create broke Eric's patch to
flush the Formatting line out before a potential error message.  This
patch should fix it (and the diff stat is negative, so that's nice).
---
 tests/qemu-iotests/020           | 29 ++++++++-------
 tests/qemu-iotests/020.out       | 13 +------
 tests/qemu-iotests/141           |  2 +-
 tests/qemu-iotests/common.filter | 62 ++++++++++++++------------------
 4 files changed, 45 insertions(+), 61 deletions(-)

Comments

Kevin Wolf July 9, 2020, 12:16 p.m. UTC | #1
Am 09.07.2020 um 13:02 hat Max Reitz geschrieben:
> Not only is it a bit stupid to try to filter multi-line "Formatting"
> output (because we only need it for a single test, which can easily be
> amended to no longer need it), it is also problematic when there can be
> output after a "Formatting" line that we do not want to filter as if it
> were part of it.
> 
> So rename _filter_img_create to _do_filter_img_create, let it filter
> only a single line, and let _filter_img_create loop over all input
> lines, calling _do_filter_img_create only on those that match
> /^Formatting/ (basically, what _filter_img_create_in_qmp did already).
> (And fix 020 to work with that.)
> 
> Reported-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> Kevin noted that the changes to _filter_img_create broke Eric's patch to
> flush the Formatting line out before a potential error message.  This
> patch should fix it (and the diff stat is negative, so that's nice).

Thanks, this fixed the problem. Applied to the block branch.

Kevin

Patch
diff mbox series

diff --git a/tests/qemu-iotests/020 b/tests/qemu-iotests/020
index 20f8f185d0..b488000cb9 100755
--- a/tests/qemu-iotests/020
+++ b/tests/qemu-iotests/020
@@ -115,18 +115,23 @@  TEST_IMG="$TEST_IMG.base" _make_test_img 1M
 # Create an image with a null backing file to which committing will fail (with
 # ENOSPC so we can distinguish the result from some generic EIO which may be
 # generated anywhere in the block layer)
-_make_test_img -b "json:{'driver': '$IMGFMT',
-                         'file': {
-                             'driver': 'blkdebug',
-                             'inject-error': [{
-                                 'event': 'write_aio',
-                                 'errno': 28,
-                                 'once': true
-                             }],
-                             'image': {
-                                 'driver': 'file',
-                                 'filename': '$TEST_IMG.base'
-                             }}}"
+backing="json:{'driver': '$IMGFMT',
+               'file': {
+                   'driver': 'blkdebug',
+                   'inject-error': [{
+                       'event': 'write_aio',
+                       'errno': 28,
+                       'once': true
+                   }],
+                   'image': {
+                       'driver': 'file',
+                       'filename': '$TEST_IMG.base'
+                   }}}"
+
+# Filter out newlines and collapse spaces
+backing=$(echo "$backing" | tr -d '\n' | tr -s ' ')
+
+_make_test_img -b "$backing"
 
 # Just write anything so committing will not be a no-op
 $QEMU_IO -c 'writev 0 64k' "$TEST_IMG" | _filter_qemu_io
diff --git a/tests/qemu-iotests/020.out b/tests/qemu-iotests/020.out
index 4b722b2dd0..4668ac59df 100644
--- a/tests/qemu-iotests/020.out
+++ b/tests/qemu-iotests/020.out
@@ -1079,18 +1079,7 @@  No errors were found on the image.
 Testing failing commit
 
 Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=1048576
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 backing_file=json:{'driver': 'IMGFMT',,
-                         'file': {
-                             'driver': 'blkdebug',,
-                             'inject-error': [{
-                                 'event': 'write_aio',,
-                                 'errno': 28,,
-                                 'once': true
-                             }],,
-                             'image': {
-                                 'driver': 'file',,
-                                 'filename': 'TEST_DIR/t.IMGFMT.base'
-                             }}}
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 backing_file=json:{'driver': 'IMGFMT',, 'file': { 'driver': 'blkdebug',, 'inject-error': [{ 'event': 'write_aio',, 'errno': 28,, 'once': true }],, 'image': { 'driver': 'file',, 'filename': 'TEST_DIR/t.IMGFMT.base' }}}
 wrote 65536/65536 bytes at offset 0
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 qemu-img: Block job failed: No space left on device
diff --git a/tests/qemu-iotests/141 b/tests/qemu-iotests/141
index 6d1b7b0d4c..5192d256e3 100755
--- a/tests/qemu-iotests/141
+++ b/tests/qemu-iotests/141
@@ -68,7 +68,7 @@  test_blockjob()
     _send_qemu_cmd $QEMU_HANDLE \
         "$1" \
         "$2" \
-        | _filter_img_create_in_qmp | _filter_qmp_empty_return
+        | _filter_img_create | _filter_qmp_empty_return
 
     # We want this to return an error because the block job is still running
     _send_qemu_cmd $QEMU_HANDLE \
diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index d967adc59a..3833206327 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -119,8 +119,21 @@  _filter_actual_image_size()
     $SED -s 's/\("actual-size":\s*\)[0-9]\+/\1SIZE/g'
 }
 
+# Filename filters for qemu-img create
+_filter_img_create_filenames()
+{
+    $SED \
+        -e "s#$REMOTE_TEST_DIR#TEST_DIR#g" \
+        -e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \
+        -e "s#$TEST_DIR#TEST_DIR#g" \
+        -e "s#$SOCK_DIR#SOCK_DIR#g" \
+        -e "s#$IMGFMT#IMGFMT#g" \
+        -e 's#nbd:127.0.0.1:[0-9]\\+#TEST_DIR/t.IMGFMT#g' \
+        -e 's#nbd+unix:///\??socket=SOCK_DIR/nbd#TEST_DIR/t.IMGFMT#g'
+}
+
 # replace driver-specific options in the "Formatting..." line
-_filter_img_create()
+_do_filter_img_create()
 {
     # Split the line into the pre-options part ($filename_part, which
     # precedes ", fmt=") and the options part ($options, which starts
@@ -128,23 +141,10 @@  _filter_img_create()
     # (And just echo everything before the first "^Formatting")
     readarray formatting_line < <($SED -e 's/, fmt=/\n/')
 
-    filename_part=''
-    options=''
-    lines=${#formatting_line[@]}
-    for ((i = 0; i < $lines; i++)); do
-        line=${formatting_line[i]}
-        unset formatting_line[i]
-
-        filename_part="$filename_part$line"
+    filename_part=${formatting_line[0]}
+    unset formatting_line[0]
 
-        if echo "$line" | grep -q '^Formatting'; then
-            next_i=$((i + 1))
-            if [ -n "${formatting_line[next_i]}" ]; then
-                options="fmt=${formatting_line[@]}"
-            fi
-            break
-        fi
-    done
+    options="fmt=${formatting_line[@]}"
 
     # Set grep_data_file to '\|data_file' to keep it; make it empty
     # to drop it.
@@ -156,17 +156,7 @@  _filter_img_create()
         grep_data_file=()
     fi
 
-    filename_filters=(
-        -e "s#$REMOTE_TEST_DIR#TEST_DIR#g" \
-        -e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \
-        -e "s#$TEST_DIR#TEST_DIR#g" \
-        -e "s#$SOCK_DIR#SOCK_DIR#g" \
-        -e "s#$IMGFMT#IMGFMT#g" \
-        -e 's#nbd:127.0.0.1:[0-9]\\+#TEST_DIR/t.IMGFMT#g' \
-        -e 's#nbd+unix:///\??socket=SOCK_DIR/nbd#TEST_DIR/t.IMGFMT#g'
-    )
-
-    filename_part=$(echo "$filename_part" | $SED "${filename_filters[@]}")
+    filename_part=$(echo "$filename_part" | _filter_img_create_filenames)
 
     # Break the option line before each option (preserving pre-existing
     # line breaks by replacing them by \0 and restoring them at the end),
@@ -179,7 +169,8 @@  _filter_img_create()
         | $SED -e 's/ \([a-z0-9_.-]*\)=/\n\1=/g' \
         | grep -a -e '^fmt' -e '^size' -e '^backing' -e '^preallocation' \
                   -e '^encryption' "${grep_data_file[@]}" \
-        | $SED "${filename_filters[@]}" \
+        | _filter_img_create_filenames \
+        | $SED \
             -e 's/^\(fmt\)/0-\1/' \
             -e 's/^\(size\)/1-\1/' \
             -e 's/^\(backing\)/2-\1/' \
@@ -199,17 +190,16 @@  _filter_img_create()
     fi
 }
 
-# Filter the "Formatting..." line in QMP output (leaving the QMP output
-# untouched)
-# (In contrast to _filter_img_create(), this function does not support
-# multi-line Formatting output)
-_filter_img_create_in_qmp()
+# Filter qemu-img create output:
+# Pipe all ^Formatting lines through _do_filter_img_create, and all
+# other lines through _filter_img_create_filenames
+_filter_img_create()
 {
     while read -r line; do
         if echo "$line" | grep -q '^Formatting'; then
-            echo "$line" | _filter_img_create
+            echo "$line" | _do_filter_img_create
         else
-            echo "$line"
+            echo "$line" | _filter_img_create_filenames
         fi
     done
 }