| Message ID | 20200625125548.870061-2-mreitz@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: LUKS encryption slot management + iotest tweaks | expand |
On Thu, 2020-06-25 at 14:55 +0200, 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> I went over this patch again, and it looks OK, but I might have missed something. Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com> Best regards, Maxim Levitsky > --- > 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()
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()
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> --- 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(-)