1. Patchew
  2. QEMU
  3. [PATCH v2 0/2] Increase amount of data for monitor to read
  4. View patch

[PATCH v2 1/2] iotests: add another bash sleep command to 247

Andrey Shinkevich via posted 2 patches 4 years, 11 months ago
Maintainers: Kevin Wolf <kwolf@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Max Reitz <mreitz@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Markus Armbruster <armbru@redhat.com>
There is a newer version of this series
[PATCH v2 1/2] iotests: add another bash sleep command to 247
Posted by Andrey Shinkevich via 4 years, 11 months ago 
This patch paves the way for the one that follows. The following patch
makes the QMP monitor to read up to 4K from stdin at once. That results
in running the bash 'sleep' command before the _qemu_proc_exec() starts
in subshell. Another 'sleep' command with an unobtrusive 'query-status'
plays as a workaround.

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 tests/qemu-iotests/247     | 2 ++
 tests/qemu-iotests/247.out | 1 +
 2 files changed, 3 insertions(+)

diff --git a/tests/qemu-iotests/247 b/tests/qemu-iotests/247
index 87e37b3..7d316ec 100755
--- a/tests/qemu-iotests/247
+++ b/tests/qemu-iotests/247
@@ -59,6 +59,8 @@ TEST_IMG="$TEST_IMG.4" _make_test_img $size
 {"execute":"block-commit",
  "arguments":{"device":"format-4", "top-node": "format-2", "base-node":"format-0", "job-id":"job0"}}
 EOF
+sleep 1
+echo '{"execute":"query-status"}'
 if [ "${VALGRIND_QEMU}" == "y" ]; then
     sleep 10
 else
diff --git a/tests/qemu-iotests/247.out b/tests/qemu-iotests/247.out
index e909e83..13d9547 100644
--- a/tests/qemu-iotests/247.out
+++ b/tests/qemu-iotests/247.out
@@ -17,6 +17,7 @@ QMP_VERSION
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "job0", "len": 134217728, "offset": 134217728, "speed": 0, "type": "commit"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "concluded", "id": "job0"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "null", "id": "job0"}}
+{"return": {"status": "running", "singlestep": false, "running": true}}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}}
 *** done
-- 
1.8.3.1
Re: [PATCH v2 1/2] iotests: add another bash sleep command to 247
Posted by Vladimir Sementsov-Ogievskiy 4 years, 11 months ago 
23.11.2020 18:44, Andrey Shinkevich wrote:
> This patch paves the way for the one that follows. The following patch
> makes the QMP monitor to read up to 4K from stdin at once. That results
> in running the bash 'sleep' command before the _qemu_proc_exec() starts

But how? Before _qemu_proc_exec() starts, qemu monitor is not runnning,
and its new behavior can't influence..

If bash subshell work in unpredictable way, may be better is refactor test
to send commands one by one with help of _send_qemu_cmd. Then sleep will
be natively executed between sending commands.

> in subshell. Another 'sleep' command with an unobtrusive 'query-status'
> plays as a workaround.
> 
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>   tests/qemu-iotests/247     | 2 ++
>   tests/qemu-iotests/247.out | 1 +
>   2 files changed, 3 insertions(+)
> 
> diff --git a/tests/qemu-iotests/247 b/tests/qemu-iotests/247
> index 87e37b3..7d316ec 100755
> --- a/tests/qemu-iotests/247
> +++ b/tests/qemu-iotests/247
> @@ -59,6 +59,8 @@ TEST_IMG="$TEST_IMG.4" _make_test_img $size
>   {"execute":"block-commit",
>    "arguments":{"device":"format-4", "top-node": "format-2", "base-node":"format-0", "job-id":"job0"}}
>   EOF
> +sleep 1
> +echo '{"execute":"query-status"}'
>   if [ "${VALGRIND_QEMU}" == "y" ]; then
>       sleep 10
>   else
> diff --git a/tests/qemu-iotests/247.out b/tests/qemu-iotests/247.out
> index e909e83..13d9547 100644
> --- a/tests/qemu-iotests/247.out
> +++ b/tests/qemu-iotests/247.out
> @@ -17,6 +17,7 @@ QMP_VERSION
>   {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "job0", "len": 134217728, "offset": 134217728, "speed": 0, "type": "commit"}}
>   {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "concluded", "id": "job0"}}
>   {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "null", "id": "job0"}}
> +{"return": {"status": "running", "singlestep": false, "running": true}}
>   {"return": {}}
>   {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}}
>   *** done
> 


-- 
Best regards,
Vladimir
Re: [PATCH v2 1/2] iotests: add another bash sleep command to 247
Posted by Andrey Shinkevich 4 years, 11 months ago 
On 24.11.2020 13:04, Vladimir Sementsov-Ogievskiy wrote:
> 23.11.2020 18:44, Andrey Shinkevich wrote:
>> This patch paves the way for the one that follows. The following patch
>> makes the QMP monitor to read up to 4K from stdin at once. That results
>> in running the bash 'sleep' command before the _qemu_proc_exec() starts
> 
> But how? Before _qemu_proc_exec() starts, qemu monitor is not runnning,
> and its new behavior can't influence..
> 

I am not a bash expert to explain 'how' but this workaround works. It's 
just a test. Maybe other colleagues can say.

> If bash subshell work in unpredictable way, may be better is refactor test
> to send commands one by one with help of _send_qemu_cmd. Then sleep will
> be natively executed between sending commands.
> 

Or maybe write a similar test case in Python if Kevin agrees.

>> in subshell. Another 'sleep' command with an unobtrusive 'query-status'
>> plays as a workaround.
>>
>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>> ---
>>   tests/qemu-iotests/247     | 2 ++
>>   tests/qemu-iotests/247.out | 1 +
>>   2 files changed, 3 insertions(+)
>>
>> diff --git a/tests/qemu-iotests/247 b/tests/qemu-iotests/247
>> index 87e37b3..7d316ec 100755
>> --- a/tests/qemu-iotests/247
>> +++ b/tests/qemu-iotests/247
>> @@ -59,6 +59,8 @@ TEST_IMG="$TEST_IMG.4" _make_test_img $size
>>   {"execute":"block-commit",
>>    "arguments":{"device":"format-4", "top-node": "format-2", 
>> "base-node":"format-0", "job-id":"job0"}}
>>   EOF
>> +sleep 1
>> +echo '{"execute":"query-status"}'
>>   if [ "${VALGRIND_QEMU}" == "y" ]; then
>>       sleep 10
>>   else
>> diff --git a/tests/qemu-iotests/247.out b/tests/qemu-iotests/247.out
>> index e909e83..13d9547 100644
>> --- a/tests/qemu-iotests/247.out
>> +++ b/tests/qemu-iotests/247.out
>> @@ -17,6 +17,7 @@ QMP_VERSION
>>   {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, 
>> "event": "BLOCK_JOB_COMPLETED", "data": {"device": "job0", "len": 
>> 134217728, "offset": 134217728, "speed": 0, "type": "commit"}}
>>   {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, 
>> "event": "JOB_STATUS_CHANGE", "data": {"status": "concluded", "id": 
>> "job0"}}
>>   {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, 
>> "event": "JOB_STATUS_CHANGE", "data": {"status": "null", "id": "job0"}}
>> +{"return": {"status": "running", "singlestep": false, "running": true}}
>>   {"return": {}}
>>   {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, 
>> "event": "SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}}
>>   *** done
>>
> 
>
Re: [PATCH v2 1/2] iotests: add another bash sleep command to 247
Posted by Dr. David Alan Gilbert 4 years, 11 months ago 
* Andrey Shinkevich (andrey.shinkevich@virtuozzo.com) wrote:
> On 24.11.2020 13:04, Vladimir Sementsov-Ogievskiy wrote:
> > 23.11.2020 18:44, Andrey Shinkevich wrote:
> > > This patch paves the way for the one that follows. The following patch
> > > makes the QMP monitor to read up to 4K from stdin at once. That results
> > > in running the bash 'sleep' command before the _qemu_proc_exec() starts
> > 
> > But how? Before _qemu_proc_exec() starts, qemu monitor is not runnning,
> > and its new behavior can't influence..
> > 
> 
> I am not a bash expert to explain 'how' but this workaround works. It's just
> a test. Maybe other colleagues can say.

> > If bash subshell work in unpredictable way, may be better is refactor test
> > to send commands one by one with help of _send_qemu_cmd. Then sleep will
> > be natively executed between sending commands.
> > 
> 
> Or maybe write a similar test case in Python if Kevin agrees.

 Yeh I don't believe the 'before the _qemu_proc_exec' starts- it's all
happening horribly in parallel with the subshell - the timing is
probably fragile as hell.

THe test needs to wait for output and then quit.

Dave

> > > in subshell. Another 'sleep' command with an unobtrusive 'query-status'
> > > plays as a workaround.
> > > 
> > > Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> > > ---
> > >   tests/qemu-iotests/247     | 2 ++
> > >   tests/qemu-iotests/247.out | 1 +
> > >   2 files changed, 3 insertions(+)
> > > 
> > > diff --git a/tests/qemu-iotests/247 b/tests/qemu-iotests/247
> > > index 87e37b3..7d316ec 100755
> > > --- a/tests/qemu-iotests/247
> > > +++ b/tests/qemu-iotests/247
> > > @@ -59,6 +59,8 @@ TEST_IMG="$TEST_IMG.4" _make_test_img $size
> > >   {"execute":"block-commit",
> > >    "arguments":{"device":"format-4", "top-node": "format-2",
> > > "base-node":"format-0", "job-id":"job0"}}
> > >   EOF
> > > +sleep 1
> > > +echo '{"execute":"query-status"}'
> > >   if [ "${VALGRIND_QEMU}" == "y" ]; then
> > >       sleep 10
> > >   else
> > > diff --git a/tests/qemu-iotests/247.out b/tests/qemu-iotests/247.out
> > > index e909e83..13d9547 100644
> > > --- a/tests/qemu-iotests/247.out
> > > +++ b/tests/qemu-iotests/247.out
> > > @@ -17,6 +17,7 @@ QMP_VERSION
> > >   {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP},
> > > "event": "BLOCK_JOB_COMPLETED", "data": {"device": "job0", "len":
> > > 134217728, "offset": 134217728, "speed": 0, "type": "commit"}}
> > >   {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP},
> > > "event": "JOB_STATUS_CHANGE", "data": {"status": "concluded", "id":
> > > "job0"}}
> > >   {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP},
> > > "event": "JOB_STATUS_CHANGE", "data": {"status": "null", "id":
> > > "job0"}}
> > > +{"return": {"status": "running", "singlestep": false, "running": true}}
> > >   {"return": {}}
> > >   {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP},
> > > "event": "SHUTDOWN", "data": {"guest": false, "reason":
> > > "host-qmp-quit"}}
> > >   *** done
> > > 
> > 
> > 
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Re: [PATCH v2 1/2] iotests: add another bash sleep command to 247
Posted by Andrey Shinkevich 4 years, 11 months ago 
On 23.11.2020 18:44, Andrey Shinkevich wrote:
> This patch paves the way for the one that follows. The following patch
> makes the QMP monitor to read up to 4K from stdin at once. That results
> in running the bash 'sleep' command before the _qemu_proc_exec() starts
> in subshell. Another 'sleep' command with an unobtrusive 'query-status'
> plays as a workaround.
> 
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>   tests/qemu-iotests/247     | 2 ++
>   tests/qemu-iotests/247.out | 1 +
>   2 files changed, 3 insertions(+)
> 

[...]

With the patch 2/2 of the current version 2, the test case #247 passes 
without this patch 1/2. So, it may be excluded from the series.
Thanks to Vladimir for the idea to check.

Andrey

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.