[Qemu-devel] [PATCH] discard rbd error output when not relevant in qemu-iotests

[Qemu-devel] [PATCH] discard rbd error output when not relevant in qemu-iotests

[Loic Dachary]

Stash the rbd stderr and stdout because it also contains human readable
progress messages : Removing image: 3% complete... in addition to
potential error messages.

Display the stashed output if rbd exits on error.

Signed-off-by: Loic Dachary <loic@dachary.org>
---
 tests/qemu-iotests/common.rc | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 28ba0d9..af66bbd 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -189,7 +189,11 @@ _cleanup_test_img()
             ;;
 
         rbd)
-            rbd rm "$TEST_DIR/t.$IMGFMT" > /dev/null
+            if ! rbd rm "$TEST_DIR/t.$IMGFMT" > $TEST_DIR/rbd.out 2>&1
+            then
+                cat $TEST_DIR/rbd.out
+            fi
+            rm $TEST_DIR/rbd.out
             ;;
 
         sheepdog)
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH] discard rbd error output when not relevant in qemu-iotests

[Benoît Canet]

Le Monday 30 Dec 2013 à 01:33:34 (+0100), Loic Dachary a écrit :
> Stash the rbd stderr and stdout because it also contains human readable
> progress messages : Removing image: 3% complete... in addition to
> potential error messages.
> 
> Display the stashed output if rbd exits on error.
> 
> Signed-off-by: Loic Dachary <loic@dachary.org>
> ---
>  tests/qemu-iotests/common.rc | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
> index 28ba0d9..af66bbd 100644
> --- a/tests/qemu-iotests/common.rc
> +++ b/tests/qemu-iotests/common.rc
> @@ -189,7 +189,11 @@ _cleanup_test_img()
>              ;;
>  
>          rbd)
> -            rbd rm "$TEST_DIR/t.$IMGFMT" > /dev/null
> +            if ! rbd rm "$TEST_DIR/t.$IMGFMT" > $TEST_DIR/rbd.out 2>&1
> +            then
> +                cat $TEST_DIR/rbd.out
> +            fi
> +            rm $TEST_DIR/rbd.out
>              ;;
>  
>          sheepdog)

Hi Loïc,

Maybe you could filter by modifying _filter_qemu_io.

Best regards

Benoît
> -- 
> 1.8.3.2
> 
> 

Re: [Qemu-devel] [PATCH] discard rbd error output when not relevant in qemu-iotests

[Loic Dachary]

[-- Attachment #1: Type: text/plain, Size: 1625 bytes --]

Hi Benoît,

If I understand correctly common.filter is designed to transform the output and remove variance. In the case of rbd displaying the progress of removal, it is something we probably want to get rid of entirely. Please let me know if I'm missing something and I'll update the patch according to your suggestion.

Cheers

On 30/12/2013 01:39, Benoît Canet wrote:
> Le Monday 30 Dec 2013 à 01:33:34 (+0100), Loic Dachary a écrit :
>> Stash the rbd stderr and stdout because it also contains human readable
>> progress messages : Removing image: 3% complete... in addition to
>> potential error messages.
>>
>> Display the stashed output if rbd exits on error.
>>
>> Signed-off-by: Loic Dachary <loic@dachary.org>
>> ---
>>  tests/qemu-iotests/common.rc | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
>> index 28ba0d9..af66bbd 100644
>> --- a/tests/qemu-iotests/common.rc
>> +++ b/tests/qemu-iotests/common.rc
>> @@ -189,7 +189,11 @@ _cleanup_test_img()
>>              ;;
>>  
>>          rbd)
>> -            rbd rm "$TEST_DIR/t.$IMGFMT" > /dev/null
>> +            if ! rbd rm "$TEST_DIR/t.$IMGFMT" > $TEST_DIR/rbd.out 2>&1
>> +            then
>> +                cat $TEST_DIR/rbd.out
>> +            fi
>> +            rm $TEST_DIR/rbd.out
>>              ;;
>>  
>>          sheepdog)
> 
> Hi Loïc,
> 
> Maybe you could filter by modifying _filter_qemu_io.
> 
> Best regards
> 
> Benoît
>> -- 
>> 1.8.3.2
>>
>>
> 
> 
> 

-- 
Loïc Dachary, Artisan Logiciel Libre


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [Qemu-devel] [PATCH] discard rbd error output when not relevant in qemu-iotests

[Stefan Hajnoczi]

On Mon, Dec 30, 2013 at 01:33:34AM +0100, Loic Dachary wrote:
> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
> index 28ba0d9..af66bbd 100644
> --- a/tests/qemu-iotests/common.rc
> +++ b/tests/qemu-iotests/common.rc
> @@ -189,7 +189,11 @@ _cleanup_test_img()
>              ;;
>  
>          rbd)
> -            rbd rm "$TEST_DIR/t.$IMGFMT" > /dev/null

stderr will be displayed.  Why isn't this enough?

Re: [Qemu-devel] [PATCH] discard rbd error output when not relevant in qemu-iotests

[Loic Dachary]

[-- Attachment #1: Type: text/plain, Size: 707 bytes --]



On 06/01/2014 03:23, Stefan Hajnoczi wrote:
> On Mon, Dec 30, 2013 at 01:33:34AM +0100, Loic Dachary wrote:
>> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
>> index 28ba0d9..af66bbd 100644
>> --- a/tests/qemu-iotests/common.rc
>> +++ b/tests/qemu-iotests/common.rc
>> @@ -189,7 +189,11 @@ _cleanup_test_img()
>>              ;;
>>  
>>          rbd)
>> -            rbd rm "$TEST_DIR/t.$IMGFMT" > /dev/null
> 
> stderr will be displayed.  Why isn't this enough?
> 

Because the progress of the removal operation will be displayed on stderr. rbd outputs on stderr even when there is no error.

Happy new year !

-- 
Loïc Dachary, Artisan Logiciel Libre


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [Qemu-devel] [PATCH] discard rbd error output when not relevant in qemu-iotests

[Stefan Hajnoczi]

On Mon, Jan 06, 2014 at 04:50:41PM +0100, Loic Dachary wrote:
> 
> 
> On 06/01/2014 03:23, Stefan Hajnoczi wrote:
> > On Mon, Dec 30, 2013 at 01:33:34AM +0100, Loic Dachary wrote:
> >> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
> >> index 28ba0d9..af66bbd 100644
> >> --- a/tests/qemu-iotests/common.rc
> >> +++ b/tests/qemu-iotests/common.rc
> >> @@ -189,7 +189,11 @@ _cleanup_test_img()
> >>              ;;
> >>  
> >>          rbd)
> >> -            rbd rm "$TEST_DIR/t.$IMGFMT" > /dev/null
> > 
> > stderr will be displayed.  Why isn't this enough?
> > 
> 
> Because the progress of the removal operation will be displayed on stderr. rbd outputs on stderr even when there is no error.

Is that a bug in the rbd tool which should be fixed?  Either by printing
progress to stdout or by adding a --quiet option?

Re: [Qemu-devel] [PATCH] discard rbd error output when not relevant in qemu-iotests

[Loic Dachary]

[-- Attachment #1: Type: text/plain, Size: 1019 bytes --]



On 08/01/2014 05:33, Stefan Hajnoczi wrote:
> On Mon, Jan 06, 2014 at 04:50:41PM +0100, Loic Dachary wrote:
>>
>>
>> On 06/01/2014 03:23, Stefan Hajnoczi wrote:
>>> On Mon, Dec 30, 2013 at 01:33:34AM +0100, Loic Dachary wrote:
>>>> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
>>>> index 28ba0d9..af66bbd 100644
>>>> --- a/tests/qemu-iotests/common.rc
>>>> +++ b/tests/qemu-iotests/common.rc
>>>> @@ -189,7 +189,11 @@ _cleanup_test_img()
>>>>              ;;
>>>>  
>>>>          rbd)
>>>> -            rbd rm "$TEST_DIR/t.$IMGFMT" > /dev/null
>>>
>>> stderr will be displayed.  Why isn't this enough?
>>>
>>
>> Because the progress of the removal operation will be displayed on stderr. rbd outputs on stderr even when there is no error.
> 
> Is that a bug in the rbd tool which should be fixed?  Either by printing
> progress to stdout or by adding a --quiet option?

In my opinion, yes. But Josh may disagree ;-)

-- 
Loïc Dachary, Artisan Logiciel Libre


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

[Qemu-devel] [PATCH v2] discard rbd error output when not relevant in qemu-iotests

[Loic Dachary]

Suppress rbd progress messages with --no-progress so they are not
confused with an error output when comparing test results ( progress is
displayed on stderr ).

Signed-off-by: Loic Dachary <loic@dachary.org>
---
 tests/qemu-iotests/common.rc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 28ba0d9..7442bad 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -189,7 +189,7 @@ _cleanup_test_img()
             ;;
 
         rbd)
-            rbd rm "$TEST_DIR/t.$IMGFMT" > /dev/null
+            rbd --no-progress rm "$TEST_DIR/t.$IMGFMT" > /dev/null
             ;;
 
         sheepdog)
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v2] discard rbd error output when not relevant in qemu-iotests

[Josh Durgin]

On 01/08/2014 12:05 PM, Loic Dachary wrote:
> Suppress rbd progress messages with --no-progress so they are not
> confused with an error output when comparing test results ( progress is
> displayed on stderr ).
>
> Signed-off-by: Loic Dachary <loic@dachary.org>
> ---
>   tests/qemu-iotests/common.rc | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
> index 28ba0d9..7442bad 100644
> --- a/tests/qemu-iotests/common.rc
> +++ b/tests/qemu-iotests/common.rc
> @@ -189,7 +189,7 @@ _cleanup_test_img()
>               ;;
>
>           rbd)
> -            rbd rm "$TEST_DIR/t.$IMGFMT" > /dev/null
> +            rbd --no-progress rm "$TEST_DIR/t.$IMGFMT" > /dev/null
>               ;;
>
>           sheepdog)
>

This flag was added in ceph 0.61. I doubt anyone is interested in
running these tests against earlier versions of ceph (the current stable
releases are 0.67 and 0.72).

Reviewed-by: Josh Durgin <josh.durgin@inktank.com>

Re: [Qemu-devel] [PATCH v2] discard rbd error output when not relevant in qemu-iotests

[Stefan Hajnoczi]

On Wed, Jan 08, 2014 at 09:05:38PM +0100, Loic Dachary wrote:
> Suppress rbd progress messages with --no-progress so they are not
> confused with an error output when comparing test results ( progress is
> displayed on stderr ).
> 
> Signed-off-by: Loic Dachary <loic@dachary.org>
> ---
>  tests/qemu-iotests/common.rc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks!

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Re: [Qemu-devel] [PATCH v2] discard rbd error output when not relevant in qemu-iotests

[Benoît Canet]

The Thursday 09 Jan 2014 à 09:45:14 (+0800), Stefan Hajnoczi wrote :
> On Wed, Jan 08, 2014 at 09:05:38PM +0100, Loic Dachary wrote:
> > Suppress rbd progress messages with --no-progress so they are not
> > confused with an error output when comparing test results ( progress is
> > displayed on stderr ).
> > 
> > Signed-off-by: Loic Dachary <loic@dachary.org>
> > ---
> >  tests/qemu-iotests/common.rc | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Thanks!
> 
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> 

Should this be merged before 2.0 ?

Best regards

Benoît

Re: [Qemu-devel] [PATCH v2] discard rbd error output when not relevant in qemu-iotests

[Stefan Hajnoczi]

On Wed, Jan 08, 2014 at 09:05:38PM +0100, Loic Dachary wrote:
> Suppress rbd progress messages with --no-progress so they are not
> confused with an error output when comparing test results ( progress is
> displayed on stderr ).
> 
> Signed-off-by: Loic Dachary <loic@dachary.org>
> ---
>  tests/qemu-iotests/common.rc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan