1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH 0/5] qemu-iotests: test savevm/loadvm iothread (and make it work!)
  4. View patch

[Qemu-devel] [PATCH 3/5] qemu-iotests: 068: extract _qemu() function

Stefan Hajnoczi posted 5 patches 8 years, 4 months ago
[Qemu-devel] [PATCH 3/5] qemu-iotests: 068: extract _qemu() function
Posted by Stefan Hajnoczi 8 years, 4 months ago 
Avoid duplicating the QEMU command-line.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qemu-iotests/068 | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/068 b/tests/qemu-iotests/068
index 9c1687d..653e23c 100755
--- a/tests/qemu-iotests/068
+++ b/tests/qemu-iotests/068
@@ -59,14 +59,17 @@ case "$QEMU_DEFAULT_MACHINE" in
       ;;
 esac
 
-# Give qemu some time to boot before saving the VM state
-bash -c 'sleep 1; echo -e "savevm 0\nquit"' |\
+_qemu()
+{
     $QEMU $platform_parm -nographic -monitor stdio -serial none -hda "$TEST_IMG" |\
+          "$@" |\
     _filter_qemu | _filter_hmp
+}
+
+# Give qemu some time to boot before saving the VM state
+bash -c 'sleep 1; echo -e "savevm 0\nquit"' | _qemu
 # Now try to continue from that VM state (this should just work)
-echo quit |\
-    $QEMU $platform_parm -nographic -monitor stdio -serial none -hda "$TEST_IMG" -loadvm 0 |\
-    _filter_qemu | _filter_hmp
+echo quit | _qemu -loadvm 0
 
 # success, all done
 echo "*** done"
-- 
2.9.4
Re: [Qemu-devel] [PATCH 3/5] qemu-iotests: 068: extract _qemu() function
Posted by Kevin Wolf 8 years, 4 months ago 
Am 15.06.2017 um 18:38 hat Stefan Hajnoczi geschrieben:
> Avoid duplicating the QEMU command-line.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  tests/qemu-iotests/068 | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/qemu-iotests/068 b/tests/qemu-iotests/068
> index 9c1687d..653e23c 100755
> --- a/tests/qemu-iotests/068
> +++ b/tests/qemu-iotests/068
> @@ -59,14 +59,17 @@ case "$QEMU_DEFAULT_MACHINE" in
>        ;;
>  esac
>  
> -# Give qemu some time to boot before saving the VM state
> -bash -c 'sleep 1; echo -e "savevm 0\nquit"' |\
> +_qemu()
> +{
>      $QEMU $platform_parm -nographic -monitor stdio -serial none -hda "$TEST_IMG" |\

This line shouldn't end with a pipe now. The bug goes away with the next
patch anyway, so I can just remove that one character while applying the
series.

> +          "$@" |\
>      _filter_qemu | _filter_hmp
> +}

Kevin
Re: [Qemu-devel] [PATCH 3/5] qemu-iotests: 068: extract _qemu() function
Posted by Eric Blake 8 years, 4 months ago 
On 06/15/2017 11:38 AM, Stefan Hajnoczi wrote:
> Avoid duplicating the QEMU command-line.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  tests/qemu-iotests/068 | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 

> +# Give qemu some time to boot before saving the VM state
> +bash -c 'sleep 1; echo -e "savevm 0\nquit"' | _qemu

Are we sure that 'bash' on PATH is the same as the /bin/bash running the
script?

Also, while bash has more deterministic behavior for 'echo -e' (it is
non-portable if you are porting a script to other shells), it is still
possible to set bash to a mode where it does not work (see xhopt
xpg_echo).  I'd much prefer you use 'printf' instead of 'set -e'.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH 3/5] qemu-iotests: 068: extract _qemu() function
Posted by Stefan Hajnoczi 8 years, 4 months ago 
On Tue, Jun 27, 2017 at 06:40:04AM -0500, Eric Blake wrote:
> On 06/15/2017 11:38 AM, Stefan Hajnoczi wrote:
> > Avoid duplicating the QEMU command-line.
> > 
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >  tests/qemu-iotests/068 | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> > 
> 
> > +# Give qemu some time to boot before saving the VM state
> > +bash -c 'sleep 1; echo -e "savevm 0\nquit"' | _qemu
> 
> Are we sure that 'bash' on PATH is the same as the /bin/bash running the
> script?
> 
> Also, while bash has more deterministic behavior for 'echo -e' (it is
> non-portable if you are porting a script to other shells), it is still
> possible to set bash to a mode where it does not work (see xhopt
> xpg_echo).  I'd much prefer you use 'printf' instead of 'set -e'.

Do you want to send a separate patch?

The purpose of this patch was just to move this code.

Stefan
Re: [Qemu-devel] [PATCH 3/5] qemu-iotests: 068: extract _qemu() function
Posted by Eric Blake 8 years, 4 months ago 
On 06/28/2017 07:13 AM, Stefan Hajnoczi wrote:
> On Tue, Jun 27, 2017 at 06:40:04AM -0500, Eric Blake wrote:
>> On 06/15/2017 11:38 AM, Stefan Hajnoczi wrote:
>>> Avoid duplicating the QEMU command-line.
>>>
>>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>>> ---
>>>  tests/qemu-iotests/068 | 13 ++++++++-----
>>>  1 file changed, 8 insertions(+), 5 deletions(-)
>>>
>>
>>> +# Give qemu some time to boot before saving the VM state
>>> +bash -c 'sleep 1; echo -e "savevm 0\nquit"' | _qemu
>>
>> Are we sure that 'bash' on PATH is the same as the /bin/bash running the
>> script?
>>
>> Also, while bash has more deterministic behavior for 'echo -e' (it is
>> non-portable if you are porting a script to other shells), it is still
>> possible to set bash to a mode where it does not work (see xhopt
>> xpg_echo).  I'd much prefer you use 'printf' instead of 'set -e'.

instead of 'echo -e'

> 
> Do you want to send a separate patch?

Sure.

> 
> The purpose of this patch was just to move this code.

Fair enough. Besides, more than just 068 use 'echo -e' where 'printf'
would be better.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH 3/5] qemu-iotests: 068: extract _qemu() function
Posted by Kevin Wolf 8 years, 4 months ago 
Am 27.06.2017 um 13:40 hat Eric Blake geschrieben:
> On 06/15/2017 11:38 AM, Stefan Hajnoczi wrote:
> > Avoid duplicating the QEMU command-line.
> > 
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >  tests/qemu-iotests/068 | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> > 
> 
> > +# Give qemu some time to boot before saving the VM state
> > +bash -c 'sleep 1; echo -e "savevm 0\nquit"' | _qemu
> 
> Are we sure that 'bash' on PATH is the same as the /bin/bash running the
> script?

Do we even need to explicitly call bash here? Doesn't this do the same
as far as this test case is concerned?

    ( sleep 1; echo "..." ) | _qemu

Kevin
Re: [Qemu-devel] [PATCH 3/5] qemu-iotests: 068: extract _qemu() function
Posted by Eric Blake 8 years, 4 months ago 
On 06/28/2017 07:57 AM, Kevin Wolf wrote:
> Am 27.06.2017 um 13:40 hat Eric Blake geschrieben:
>> On 06/15/2017 11:38 AM, Stefan Hajnoczi wrote:
>>> Avoid duplicating the QEMU command-line.
>>>
>>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>>> ---
>>>  tests/qemu-iotests/068 | 13 ++++++++-----
>>>  1 file changed, 8 insertions(+), 5 deletions(-)
>>>
>>
>>> +# Give qemu some time to boot before saving the VM state
>>> +bash -c 'sleep 1; echo -e "savevm 0\nquit"' | _qemu
>>
>> Are we sure that 'bash' on PATH is the same as the /bin/bash running the
>> script?
> 
> Do we even need to explicitly call bash here? Doesn't this do the same
> as far as this test case is concerned?
> 
>     ( sleep 1; echo "..." ) | _qemu

Or save a process:

{ sleep 1; echo "..."; } | _qemu

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH 3/5] qemu-iotests: 068: extract _qemu() function
Posted by Kevin Wolf 8 years, 4 months ago 
Am 28.06.2017 um 16:02 hat Eric Blake geschrieben:
> On 06/28/2017 07:57 AM, Kevin Wolf wrote:
> > Am 27.06.2017 um 13:40 hat Eric Blake geschrieben:
> >> On 06/15/2017 11:38 AM, Stefan Hajnoczi wrote:
> >>> Avoid duplicating the QEMU command-line.
> >>>
> >>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> >>> ---
> >>>  tests/qemu-iotests/068 | 13 ++++++++-----
> >>>  1 file changed, 8 insertions(+), 5 deletions(-)
> >>>
> >>
> >>> +# Give qemu some time to boot before saving the VM state
> >>> +bash -c 'sleep 1; echo -e "savevm 0\nquit"' | _qemu
> >>
> >> Are we sure that 'bash' on PATH is the same as the /bin/bash running the
> >> script?
> > 
> > Do we even need to explicitly call bash here? Doesn't this do the same
> > as far as this test case is concerned?
> > 
> >     ( sleep 1; echo "..." ) | _qemu
> 
> Or save a process:
> 
> { sleep 1; echo "..."; } | _qemu

Ah, I knew that something like this must work. I missed the semicolon...

Kevin
Re: [Qemu-devel] [PATCH 3/5] qemu-iotests: 068: extract _qemu() function
Posted by Eric Blake 8 years, 4 months ago 
On 06/27/2017 06:40 AM, Eric Blake wrote:
> On 06/15/2017 11:38 AM, Stefan Hajnoczi wrote:
>> Avoid duplicating the QEMU command-line.
>>
>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>> ---
>>  tests/qemu-iotests/068 | 13 ++++++++-----
>>  1 file changed, 8 insertions(+), 5 deletions(-)
>>
> 
>> +# Give qemu some time to boot before saving the VM state
>> +bash -c 'sleep 1; echo -e "savevm 0\nquit"' | _qemu
> 
> Are we sure that 'bash' on PATH is the same as the /bin/bash running the
> script?
> 
> Also, while bash has more deterministic behavior for 'echo -e' (it is
> non-portable if you are porting a script to other shells), it is still
> possible to set bash to a mode where it does not work (see xhopt

shopt (I can't type first thing in the morning)

> xpg_echo).  I'd much prefer you use 'printf' instead of 'set -e'.
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.