Re: [PULL 03/31] iotests: Make _filter_img_create more active

[John Snow]

On 7/6/20 6:04 AM, Max Reitz wrote:
> Right now, _filter_img_create just filters out everything that looks
> format-dependent, and applies some filename filters.  That means that we
> have to add another filter line every time some format gets a new
> creation option.  This can be avoided by instead discarding everything
> and just keeping what we know is format-independent (format, size,
> backing file, encryption information[1], preallocation) or just
> interesting to have in the reference output (external data file path).
> 
> Furthermore, we probably want to sort these options.  Format drivers are
> not required to define them in any specific order, so the output is
> effectively random (although this has never bothered us until now).  We
> need a specific order for our reference outputs, though.  Unfortunately,
> just using a plain "sort" would change a lot of existing reference
> outputs, so we have to pre-filter the option keys to keep our existing
> order (fmt, size, backing*, data, encryption info, preallocation).
> 
> Finally, this makes it difficult for _filter_img_create to automagically
> work for QMP output.  Thus, this patch adds a separate
> _filter_img_create_for_qmp function that echos every line verbatim that
> does not start with "Formatting", and pipes those "Formatting" lines to
> _filter_img_create.
> 
> [1] Actually, the only thing that is really important is whether
>     encryption is enabled or not.  A patch by Maxim thus removes all
>     other "encrypt.*" options from the output:
>     https://lists.nongnu.org/archive/html/qemu-block/2020-06/msg00339.html
>     But that patch needs to come later so we can get away with changing
>     as few reference outputs in this patch here as possible.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> Message-Id: <20200625125548.870061-2-mreitz@redhat.com>
> Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>


Hi, I am seeing breakage on travis-ci;

https://travis-ci.org/github/jnsnow/qemu/jobs/706915953#L2236

(Disclaimer: Has my python patches applied)

There's some ambiguity perhaps over the order required by data_file /
data_file_raw.

Also in the IRC channel, pm215 notes that there is a breakage on OSX as
'readarray' is a bash 4.0 feature, and OSX has only bash 3.2:

11:34 < stsquad> MacOSX iotest failures
11:34 < stsquad> Not run: 005 013 018 019 024 034 038 039 041 048 060
061 074 079 080 086 097 099 108 137 138 140 141 150 154 161 172 176 179 181
                 184 203 220 226 229 244 249 251 252 259 265 267 287 290
292z
11:34 < stsquad> Failures: 001 002 003 004 007 008 009 010 011 012 017
020 021 022 025 027 029 031 032 033 035 036 037 042 043 046 047 050 052
                 053 054 062 063 066 069 071 072 073 089 090 098 103 104
105 107 110 111 114 117 120 126 127 133 134 156 158 159 170 174 177 186
                 187 190 191 192 195 214 217 268
11:34 < stsquad> Failed 69 of 75 iotests
11:34 < stsquad> that sounds like a bash-ism has snuck in?
11:36 < pm215> stsquad: readarray is in bash 4, osx system bash is 3.2
11:36 < pm215> dunno why that didn't fail for my own osx box
11:38 < th_huth> pm215: does "make check" run the iotests on your osx
box at all? Maybe it is missing gnu-sed ?
11:39 < pm215> th_huth: ah, it doesn't: "GNU sed not available ==> Not
running the qemu-iotests"
11:39 < th_huth> too bad :-(


> ---
>  tests/qemu-iotests/112.out       |   2 +-
>  tests/qemu-iotests/141           |   2 +-
>  tests/qemu-iotests/153           |   9 ++-
>  tests/qemu-iotests/common.filter | 109 ++++++++++++++++++++++++-------
>  4 files changed, 91 insertions(+), 31 deletions(-)
> 
> diff --git a/tests/qemu-iotests/112.out b/tests/qemu-iotests/112.out
> index ae0318cabe..182655dbf6 100644
> --- a/tests/qemu-iotests/112.out
> +++ b/tests/qemu-iotests/112.out
> @@ -5,7 +5,7 @@ QA output created by 112
>  qemu-img: TEST_DIR/t.IMGFMT: Refcount width must be a power of two and may 
> not exceed 64 bits
>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
>  qemu-img: TEST_DIR/t.IMGFMT: Refcount width must be a power of two and may 
> not exceed 64 bits
> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 refcount_bits=-1
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
>  qemu-img: TEST_DIR/t.IMGFMT: Refcount width must be a power of two and may 
> not exceed 64 bits
>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
>  qemu-img: TEST_DIR/t.IMGFMT: Refcount width must be a power of two and may 
> not exceed 64 bits
> diff --git a/tests/qemu-iotests/141 b/tests/qemu-iotests/141
> index 5192d256e3..6d1b7b0d4c 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 | _filter_qmp_empty_return
> +        | _filter_img_create_in_qmp | _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/153 b/tests/qemu-iotests/153
> index cf961d3609..11e3d28841 100755
> --- a/tests/qemu-iotests/153
> +++ b/tests/qemu-iotests/153
> @@ -167,11 +167,10 @@ done
>  
>  echo
>  echo "== Creating ${TEST_IMG}.[abc] ==" | _filter_testdir
> -(
> -    $QEMU_IMG create -f qcow2 "${TEST_IMG}.a" -b "${TEST_IMG}"
> -    $QEMU_IMG create -f qcow2 "${TEST_IMG}.b" -b "${TEST_IMG}"
> -    $QEMU_IMG create -f qcow2 "${TEST_IMG}.c" -b "${TEST_IMG}.b"
> -) | _filter_img_create
> +$QEMU_IMG create -f qcow2 "${TEST_IMG}.a" -b "${TEST_IMG}" | 
> _filter_img_create
> +$QEMU_IMG create -f qcow2 "${TEST_IMG}.b" -b "${TEST_IMG}" | 
> _filter_img_create
> +$QEMU_IMG create -f qcow2 "${TEST_IMG}.c" -b "${TEST_IMG}.b" \
> +    | _filter_img_create
>  
>  echo
>  echo "== Two devices sharing the same file in backing chain =="
> diff --git a/tests/qemu-iotests/common.filter 
> b/tests/qemu-iotests/common.filter
> index 03e4f71808..f8cd80ff1f 100644
> --- a/tests/qemu-iotests/common.filter
> +++ b/tests/qemu-iotests/common.filter
> @@ -122,38 +122,99 @@ _filter_actual_image_size()
>  # replace driver-specific options in the "Formatting..." line
>  _filter_img_create()
>  {
> -    data_file_filter=()
> -    if data_file=$(_get_data_file "$TEST_IMG"); then
> -        data_file_filter=(-e "s# data_file=$data_file##")
> +    # Split the line into the pre-options part ($filename_part, which
> +    # precedes ", fmt=") and the options part ($options, which starts
> +    # with "fmt=")
> +    # (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"
> +
> +        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
> +
> +    # Set grep_data_file to '\|data_file' to keep it; make it empty
> +    # to drop it.
> +    # We want to drop it if it is part of the global $IMGOPTS, and we
> +    # want to keep it otherwise (if the test specifically wants to
> +    # test data files).
> +    grep_data_file=(-e data_file)
> +    if _get_data_file "$TEST_IMG" > /dev/null; then
> +        grep_data_file=()
>      fi
>  
> -    $SED "${data_file_filter[@]}" \
> +    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' \
> -        -e "s# encryption=off##g" \
> -        -e "s# cluster_size=[0-9]\\+##g" \
> -        -e "s# table_size=[0-9]\\+##g" \
> -        -e "s# compat=[^ ]*##g" \
> -        -e "s# compat6=\\(on\\|off\\)##g" \
> -        -e "s# static=\\(on\\|off\\)##g" \
> -        -e "s# zeroed_grain=\\(on\\|off\\)##g" \
> -        -e "s# subformat=[^ ]*##g" \
> -        -e "s# adapter_type=[^ ]*##g" \
> -        -e "s# hwversion=[^ ]*##g" \
> -        -e "s# lazy_refcounts=\\(on\\|off\\)##g" \
> -        -e "s# block_size=[0-9]\\+##g" \
> -        -e "s# block_state_zero=\\(on\\|off\\)##g" \
> -        -e "s# log_size=[0-9]\\+##g" \
> -        -e "s# refcount_bits=[0-9]\\+##g" \
> -        -e "s# key-secret=[a-zA-Z0-9]\\+##g" \
> -        -e "s# iter-time=[0-9]\\+##g" \
> -        -e "s# force_size=\\(on\\|off\\)##g" \
> -        -e "s# compression_type=[a-zA-Z0-9]\\+##g"
> +        -e 's#nbd+unix:///\??socket=SOCK_DIR/nbd#TEST_DIR/t.IMGFMT#g'
> +    )
> +
> +    filename_part=$(echo "$filename_part" | $SED "${filename_filters[@]}")
> +
> +    # Break the option line before each option (preserving pre-existing
> +    # line breaks by replacing them by \0 and restoring them at the end),
> +    # then filter out the options we want to keep and sort them according
> +    # to some order that all block drivers used at the time of writing
> +    # this function.
> +    options=$(
> +        echo "$options" \
> +        | tr '\n' '\0' \
> +        | $SED -e 's/ \([a-z0-9_.-]*\)=/\n\1=/g' \
> +        | grep -a -e '^fmt' -e '^size' -e '^backing' -e '^preallocation' \
> +                  -e '^encrypt' "${grep_data_file[@]}" \
> +        | $SED "${filename_filters[@]}" \
> +            -e 's/^\(fmt\)/0-\1/' \
> +            -e 's/^\(size\)/1-\1/' \
> +            -e 's/^\(backing\)/2-\1/' \
> +            -e 's/^\(data_file\)/3-\1/' \
> +            -e 's/^\(encryption\)/4-\1/' \
> +            -e 's/^\(encrypt\.format\)/5-\1/' \
> +            -e 's/^\(encrypt\.key-secret\)/6-\1/' \
> +            -e 's/^\(encrypt\.iter-time\)/7-\1/' \
> +            -e 's/^\(preallocation\)/8-\1/' \
> +        | sort \
> +        | $SED -e 's/^[0-9]-//' \
> +        | tr '\n\0' ' \n' \
> +        | $SED -e 's/^ *$//' -e 's/ *$//'
> +    )
> +
> +    if [ -n "$options" ]; then
> +        echo "$filename_part, $options"
> +    elif [ -n "$filename_part" ]; then
> +        echo "$filename_part"
> +    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()
> +{
> +    while read -r line; do
> +        if echo "$line" | grep -q '^Formatting'; then
> +            echo "$line" | _filter_img_create
> +        else
> +            echo "$line"
> +        fi
> +    done
>  }
>  
>  _filter_img_create_size()
> 

-- 
—js