Patchwork [11/12] qemu-iotests: Be more flexible with image creation options

login
register
mail settings
Submitter Kevin Wolf
Date Aug. 6, 2012, 8:44 p.m.
Message ID <1344285891-6578-12-git-send-email-kwolf@redhat.com>
Download mbox | patch
Permalink /patch/175456/
State New
Headers show

Comments

Kevin Wolf - Aug. 6, 2012, 8:44 p.m.
qemu-iotests already filters out image creation options that may be
present or not in order to get the same output in both cases. However,
often it only considers the default value of the option. Cover all valid
values instead so that ./check -o name=value can be used successfull for
all of them.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/039.out   |    6 +++---
 tests/qemu-iotests/common.rc |    8 ++++----
 2 files changed, 7 insertions(+), 7 deletions(-)
Eric Blake - Aug. 6, 2012, 9:54 p.m.
On 08/06/2012 02:44 PM, Kevin Wolf wrote:
> qemu-iotests already filters out image creation options that may be
> present or not in order to get the same output in both cases. However,
> often it only considers the default value of the option. Cover all valid
> values instead so that ./check -o name=value can be used successfull for
> all of them.
> 

> +++ b/tests/qemu-iotests/common.rc
> @@ -110,11 +110,11 @@ _make_test_img()
>      	sed -e "s#$IMGFMT#IMGFMT#g" | \
>  	sed -e "s# encryption=off##g" | \
>  	sed -e "s# cluster_size=[0-9]\\+##g" | \
> -	sed -e "s# table_size=0##g" | \
> +	sed -e "s# table_size=[0-9]\\+##g" | \

Technically, use of \+ in a sed expression is undefined by POSIX.

>  	sed -e "s# compat='[^']*'##g" | \
> -	sed -e "s# compat6=off##g" | \
> -	sed -e "s# static=off##g" | \
> -	sed -e "s# lazy_refcounts=off##g"
> +	sed -e "s# compat6=\\(on\\|off\\)##g" | \

Likewise for use of \(, \|, and \).

If you were worried about strict POSIX compliance, you could rewrite
these to use similar expressions that are limited to portable sed:

sed -e "s# table_size=[0-9][0-9]*##g"
sed -e "s# compat6=o[nf]*##g"

(and if the difference of wiping unlikely things like compat6=onfnfn
really matters to one of our tests, then we've got some UI to improve.)
 But since this is the testsuite, which will run primarily on systems
with GNU sed where all four extensions are commonly present for
selecting features of ERE on top of the POSIX-mandated BRE syntax used
by sed, and where encountering a non-GNU sed that doesn't have the same
extensions will probably result in a noisy testsuite failure rather than
silent disk corruption, I'm okay overlooking this non-portable aspect
for now.  (Not to mention that you're not the first offender, seeing
that cluster_size earlier in the context also used \+.)
Eric Blake - Aug. 6, 2012, 9:57 p.m.
On 08/06/2012 03:54 PM, Eric Blake wrote:
> On 08/06/2012 02:44 PM, Kevin Wolf wrote:
>> qemu-iotests already filters out image creation options that may be
>> present or not in order to get the same output in both cases. However,
>> often it only considers the default value of the option. Cover all valid
>> values instead so that ./check -o name=value can be used successfull for
>> all of them.
>>
> 
>> +++ b/tests/qemu-iotests/common.rc
>> @@ -110,11 +110,11 @@ _make_test_img()
>>      	sed -e "s#$IMGFMT#IMGFMT#g" | \
>>  	sed -e "s# encryption=off##g" | \
>>  	sed -e "s# cluster_size=[0-9]\\+##g" | \
>> -	sed -e "s# table_size=0##g" | \
>> +	sed -e "s# table_size=[0-9]\\+##g" | \
> 
> Technically, use of \+ in a sed expression is undefined by POSIX.

Oh, one other thing.  This wastes a lot of processes by making a huge
pipeline.  Why not just do it with one sed process instead?

sed -e "s# encryption=off##g" \
    -e "s# cluster_size=..." \
    -e ...
Kevin Wolf - Aug. 9, 2012, 11:13 a.m.
Am 06.08.2012 23:57, schrieb Eric Blake:
> On 08/06/2012 03:54 PM, Eric Blake wrote:
>> On 08/06/2012 02:44 PM, Kevin Wolf wrote:
>>> qemu-iotests already filters out image creation options that may be
>>> present or not in order to get the same output in both cases. However,
>>> often it only considers the default value of the option. Cover all valid
>>> values instead so that ./check -o name=value can be used successfull for
>>> all of them.
>>>
>>
>>> +++ b/tests/qemu-iotests/common.rc
>>> @@ -110,11 +110,11 @@ _make_test_img()
>>>      	sed -e "s#$IMGFMT#IMGFMT#g" | \
>>>  	sed -e "s# encryption=off##g" | \
>>>  	sed -e "s# cluster_size=[0-9]\\+##g" | \
>>> -	sed -e "s# table_size=0##g" | \
>>> +	sed -e "s# table_size=[0-9]\\+##g" | \
>>
>> Technically, use of \+ in a sed expression is undefined by POSIX.
> 
> Oh, one other thing.  This wastes a lot of processes by making a huge
> pipeline.  Why not just do it with one sed process instead?
> 
> sed -e "s# encryption=off##g" \
>     -e "s# cluster_size=..." \
>     -e ...

Makes sense, I'll send a patch.

Kevin

Patch

diff --git a/tests/qemu-iotests/039.out b/tests/qemu-iotests/039.out
index 8ad570d..155a05e 100644
--- a/tests/qemu-iotests/039.out
+++ b/tests/qemu-iotests/039.out
@@ -1,14 +1,14 @@ 
 QA output created by 039
 
 == Checking that image is clean on shutdown ==
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 lazy_refcounts=on 
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
 wrote 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 incompatible_features     0x0
 No errors were found on the image.
 
 == Creating a dirty image file ==
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 lazy_refcounts=on 
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
 wrote 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 incompatible_features     0x1
@@ -34,7 +34,7 @@  read 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 == Opening a dirty image read/write should repair it ==
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 lazy_refcounts=on 
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
 wrote 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 incompatible_features     0x1
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index cc4e39b..7782808 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -110,11 +110,11 @@  _make_test_img()
     	sed -e "s#$IMGFMT#IMGFMT#g" | \
 	sed -e "s# encryption=off##g" | \
 	sed -e "s# cluster_size=[0-9]\\+##g" | \
-	sed -e "s# table_size=0##g" | \
+	sed -e "s# table_size=[0-9]\\+##g" | \
 	sed -e "s# compat='[^']*'##g" | \
-	sed -e "s# compat6=off##g" | \
-	sed -e "s# static=off##g" | \
-	sed -e "s# lazy_refcounts=off##g"
+	sed -e "s# compat6=\\(on\\|off\\)##g" | \
+	sed -e "s# static=\\(on\\|off\\)##g" | \
+	sed -e "s# lazy_refcounts=\\(on\\|off\\)##g"
 }
 
 _cleanup_test_img()