On 09/23/2013 06:09 AM, Max Reitz wrote: > In _img_info, filter out additional information specific to the image > format provided by qemu-img info, since tests designed for multiple > image formats would produce different outputs for every image format > else.
s/else/otherwise/ > > Signed-off-by: Max Reitz <mre...@redhat.com> > --- > tests/qemu-iotests/common.rc | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc > index 28b39e4..12d8882 100644 > --- a/tests/qemu-iotests/common.rc > +++ b/tests/qemu-iotests/common.rc > @@ -181,12 +181,29 @@ _check_test_img() > > _img_info() > { > + discard=0 > $QEMU_IMG info "$@" $TEST_IMG 2>&1 | \ > sed -e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \ > -e "s#$TEST_DIR#TEST_DIR#g" \ > -e "s#$IMGFMT#IMGFMT#g" \ > -e "/^disk size:/ D" \ > - -e "/actual-size/ D" > + -e "/actual-size/ D" | \ > + while IFS='' read line; do > + if [ "$line" == "Format specific information:" ]; then [ ... == ...] is a bashism (thank goodness this is already a bash script); but I generally prefer you either stick to portable syntax: if [ "$line" = "Format specific information:" ] or make it obvious that you know you are using bash: if [[ $line == "Format specific information:" ]] > + discard=1 > + elif [ "`echo "$line" | sed -e 's/^ *//'`" == > '"format-specific": {' ]; then Use $(), not ``. This script is already a bash script; why not exploit that and avoid a fork: elif [[ $line =~ '"format-specific": {' ]] > + discard=2 > + json_indent="`echo "$line" | sed -e 's/^\( *\).*$/\1/'`" Use $(), not ``. Exploit bash to avoid a fork: json_indent=${line%%[! ]*} > + fi > + if [ $discard == 0 ]; then Again, I don't like the bashism of [ == ]. > + echo "$line" > + elif [ $discard == 1 -a -z "$line" ]; then [ ... -a ... ] is flat out non-portable. Even when you are already requiring bash. For example: [ "$str1" -a "$str2" ] gives status 0 for most pairs of non-empty strings, but could give status 1 for str1="!" and str2=".". Using a bashism, on the other hand, is unambiguous: elif [[ $discard == 1 && ! $line ]] > + echo > + discard=0 > + elif [ $discard == 2 -a "`echo "$line" | sed -e 's/ *$//'`" == > "${json_indent}}," ]; then Huh? If we detected json output, then compare whether the current line with trailing whitespace stripped is now identical to $json_indent; but based on the above, you set $json_indent to contain JUST whitespace. A line with trailing whitespace removed will NEVER match a variable that contains just whitespace, so this condition will never trigger. Not to mention the deprecated `` and non-portable use of == and -a inside []. > + discard=0 > + fi > + done For the human output case, sed can already do everything your 'while read' loop did: sed ... -e "/^disk size:/ D" \ -e "/actual-size/ D" \ -e "/Format specific information/,/^$/ D" but for the JSON output case, while I'm sure that sed could probably be coerced into stripping lines until the first line that does not have at least as much indentation as the line containing "format-specific", the resulting script wouldn't be very pretty to read (I couldn't quickly produce one, at any rate - maybe Paolo has more expertise at writing arcane sed scripts). This patch might be easier to review if you provided a sample in the commit message of what input is being stripped by this patch. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature