[Qemu-devel] [PATCH v2 3/4] iotests.py: rewrite run_job to be pickier

John Snow posted 4 patches 6 years, 9 months ago
Maintainers: Markus Armbruster <armbru@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Max Reitz <mreitz@redhat.com>
There is a newer version of this series
[Qemu-devel] [PATCH v2 3/4] iotests.py: rewrite run_job to be pickier
Posted by John Snow 6 years, 9 months ago
Don't pull events out of the queue that don't belong to us;
be choosier so that we can use this method to drive jobs that
were launched by transactions that may have more jobs.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/iotests.py | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 561f547a97..601c802476 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -536,23 +536,21 @@ class VM(qtest.QEMUQtestMachine):
     def run_job(self, job, auto_finalize=True, auto_dismiss=False, wait=60.0):
         error = None
         while True:
-            for ev in self.get_qmp_events_filtered(wait=wait):
-                if ev['event'] == 'JOB_STATUS_CHANGE':
-                    status = ev['data']['status']
-                    if status == 'aborting':
-                        result = self.qmp('query-jobs')
-                        for j in result['return']:
-                            if j['id'] == job:
-                                error = j['error']
-                                log('Job failed: %s' % (j['error']))
-                    elif status == 'pending' and not auto_finalize:
-                        self.qmp_log('job-finalize', id=job)
-                    elif status == 'concluded' and not auto_dismiss:
-                        self.qmp_log('job-dismiss', id=job)
-                    elif status == 'null':
-                        return error
-                else:
-                    iotests.log(ev)
+            ev = self.event_wait(name='JOB_STATUS_CHANGE',
+                                 match={'data':{'id':job}})
+            status = ev['data']['status']
+            if status == 'aborting':
+                result = self.qmp('query-jobs')
+                for j in result['return']:
+                    if j['id'] == job:
+                        error = j['error']
+                        log('Job failed: %s' % (j['error']))
+            elif status == 'pending' and not auto_finalize:
+                self.qmp_log('job-finalize', id=job)
+            elif status == 'concluded' and not auto_dismiss:
+                self.qmp_log('job-dismiss', id=job)
+            elif status == 'null':
+                return error
 
     def node_info(self, node_name):
         nodes = self.qmp('query-named-block-nodes')
-- 
2.20.1


Re: [Qemu-devel] [PATCH v2 3/4] iotests.py: rewrite run_job to be pickier
Posted by Max Reitz 6 years, 8 months ago
On 10.05.19 21:03, John Snow wrote:
> Don't pull events out of the queue that don't belong to us;
> be choosier so that we can use this method to drive jobs that
> were launched by transactions that may have more jobs.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  tests/qemu-iotests/iotests.py | 32 +++++++++++++++-----------------
>  1 file changed, 15 insertions(+), 17 deletions(-)

There are a couple of conflicts because of concurrent patches to run_job
now.  I resolved those, but then noticed that the tests 245 and 255 no
longer pass; their reference output contains events like
BLOCK_JOB_PENDING and BLOCK_JOB_COMPLETED.

I’m not sure whether we should remove those event from the output.  It
feels weird to me to keep them somewhere in the back log and not show
them in tests that by design have kind of a full QMP log.  On the other
hand, I see that this patch is necessary.  Ideally, I think run_job
should log all events that relate to the job at hand -- but our current
event_wait() matching system doesn’t allow that.

Ideas? :-/

Max

Re: [Qemu-devel] [PATCH v2 3/4] iotests.py: rewrite run_job to be pickier
Posted by John Snow 6 years, 8 months ago

On 5/23/19 8:39 AM, Max Reitz wrote:
> On 10.05.19 21:03, John Snow wrote:
>> Don't pull events out of the queue that don't belong to us;
>> be choosier so that we can use this method to drive jobs that
>> were launched by transactions that may have more jobs.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>  tests/qemu-iotests/iotests.py | 32 +++++++++++++++-----------------
>>  1 file changed, 15 insertions(+), 17 deletions(-)
> 
> There are a couple of conflicts because of concurrent patches to run_job
> now.  I resolved those, but then noticed that the tests 245 and 255 no
> longer pass; their reference output contains events like
> BLOCK_JOB_PENDING and BLOCK_JOB_COMPLETED.
> 
> I’m not sure whether we should remove those event from the output.  It
> feels weird to me to keep them somewhere in the back log and not show
> them in tests that by design have kind of a full QMP log.  On the other
> hand, I see that this patch is necessary.  Ideally, I think run_job
> should log all events that relate to the job at hand -- but our current
> event_wait() matching system doesn’t allow that.
> 
> Ideas? :-/
> 

Amend event_wait to be able to wait for multiple events and criteria;
then amend run_job to wait for both legacy and contemporary job events.

Because all block_job_* events are omitted prior to the final transition
to the null state, they can remain optional waits. Whenever they are
present, they will be fully consumed and logged.

Patches comin' up.

> Max
>