Re: [PATCH v4 1/3] qmp.py: change event_wait to use a dict

2020-06-16 Thread John Snow



On 6/16/20 5:41 PM, Eric Blake wrote:
> On 5/14/20 2:31 PM, John Snow wrote:
> 

 Nah, it's fine. I'll clean it up. This is pretty close to an RFC series
 anyway, so I didn't really polish it.

 (Or, I will try to clean it up. I probably won't work on it again in
 the
 near term. I think I just wanted to see if this seemed useful in
 general
 to people.
>>>
>>> Ah, there isn't much missing for this series, though. We don't have to
>>> wait for a fix-the-world series when we can incrementally improve
>>> things.
>>>
>>
>> Alright, I'll try to hit it halfway -- I spent some time thinking about
>> a "full" job running framework but ran into some dead-ends I wasn't too
>> happy with, and wasn't convinced this was a simplification of any kind.
>>
>> Still, seeing part of the job running code get duplicated in 040 was a
>> motivation to try and provide some universal job-running monster that
>> would be extensible for nearly any task.
>>
>> Unfortunately that complexity does generally make the calling sites look
>> worse, so I cooled off on the idea since.
>>
>> So I did intend this as an RFC, because I'm not really 100% happy with
>> the design.
> 
> I noticed that the block-dirty-bitmap-populate series depends on this
> one; is it going to be simpler for me to fix the few things that Kevin
> pointed out here, or to wait for you to post a v5 of this series, or to
> rewrite the iotest in that series to not depend on JobRunner after all?
> 

It should be pretty trivial (I think) to just rebase the bitpop job on
top of mainline QEMU without needing this, I'd recommend doing that.

I started porting the job runner to the standalone qemu package instead
and it's going to take me longer to do that than it would be to just not
use this patchset for the bitpop test.

If you ping me on IRC tomorrow (Sorry) I can wean the dependency myself.

--js




Re: [PATCH v4 1/3] qmp.py: change event_wait to use a dict

2020-06-16 Thread Eric Blake

On 5/14/20 2:31 PM, John Snow wrote:



Nah, it's fine. I'll clean it up. This is pretty close to an RFC series
anyway, so I didn't really polish it.

(Or, I will try to clean it up. I probably won't work on it again in the
near term. I think I just wanted to see if this seemed useful in general
to people.


Ah, there isn't much missing for this series, though. We don't have to
wait for a fix-the-world series when we can incrementally improve
things.



Alright, I'll try to hit it halfway -- I spent some time thinking about
a "full" job running framework but ran into some dead-ends I wasn't too
happy with, and wasn't convinced this was a simplification of any kind.

Still, seeing part of the job running code get duplicated in 040 was a
motivation to try and provide some universal job-running monster that
would be extensible for nearly any task.

Unfortunately that complexity does generally make the calling sites look
worse, so I cooled off on the idea since.

So I did intend this as an RFC, because I'm not really 100% happy with
the design.


I noticed that the block-dirty-bitmap-populate series depends on this 
one; is it going to be simpler for me to fix the few things that Kevin 
pointed out here, or to wait for you to post a v5 of this series, or to 
rewrite the iotest in that series to not depend on JobRunner after all?


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




Re: [PATCH v4 1/3] qmp.py: change event_wait to use a dict

2020-05-14 Thread John Snow



On 5/14/20 11:59 AM, Kevin Wolf wrote:
> Am 14.05.2020 um 17:07 hat John Snow geschrieben:
>>
>>
>> On 5/14/20 10:47 AM, Kevin Wolf wrote:
>>> Am 14.05.2020 um 04:25 hat John Snow geschrieben:
 It's easier to work with than a list of tuples, because we can check the
 keys for membership.

 Signed-off-by: John Snow 
 ---
  python/qemu/machine.py| 10 +-
  tests/qemu-iotests/040| 12 ++--
  tests/qemu-iotests/260|  5 +++--
  tests/qemu-iotests/iotests.py | 16 
  4 files changed, 22 insertions(+), 21 deletions(-)
>>>
>>> I think you need to convert scripts/simplebench/bench_block_job.py, too.
>>
>> Oh, right -- that one is new since I did this. A lot of these scripts
>> need to be moved over into the python/ directory and managed under the
>> same pylint/mypy regime as everything else.
>>
>> A *ton* of our scripts are in various states of disrepair.
> 
> Is python/ actually supposed to have executable files in it? I thought
> it was more for libraries.
> 

Welll. At the moment it's library only. but one of the
things you can do with a library is define executable entry-points into
that library.

If you haven't cast an eye at that 32 patch series yet, it basically
creates a structure like this:

> ./python/qemu/lib/[qmp|machine|qtest|accel].py

qemu/ forms a PEP420 namespace; the idea is to be able to modularly
create and independently package subpackages.

qemu/lib forms a proper python package in which there are no
executables, just a library, as you say.

My idea is that anything under python/*/ ought to form a properly
formatted package. So we could, for instance, have a
python/qemu/devtools namespace which packages and collects a bunch of
our little scripts.

Then we could make sure we hit them with the same
mypy/pylint/flake8/whatever as the core libraries those scripts are
using to keep them in sync better.

And, ideally, if they are all using the same kind of paradigms for
import and dependency management it will be easier to use them and keep
them up to date, etc.

For using them as a developer, you could, say,
cd  ~/src/qemu/python
pip3 install --user -e .

and install the source packages to your local environment and then have
access to e.g.

> qmp-shell

right on your CLI, without having to fuss with PYTHONPATH or anything
else. As you update the source repo, you'll get the new versions of the
package living in your python environment automatically.

Of course, this maybe has downsides too; so you can always use a virtual
environment to adopt a context in which you have these tools. For that,

> pip3 install --user pipenv  # or use dnf, or apt, w/e.
> cd ~/src/qemu/python
> pipenv shell
> pip install -e .

And from here you'll have the dev package installed to a development
venv that you can use.

*cough* anyway, that's wildly off-topic.

Generally, you want to format a library such that you have a callable
entry point, maybe named main(). So you'd have some qmp-shell module and
it has a main() function.

Then, in the setup.py script, you'd define qemu.lib.qmp_shell:main() as
an entry point and give it a name like 'qmp-shell'. When pip/setuptools
processes your package installation, it'll create a shim for you in e.g.
~/.local/bin/qmp-shell that will just load the library and execute that
entrypoint for you.

I was thinking I'd do this for all of our python scripts so I could
spend my energy on a pylint/mypy test infrastructure *once* and *in one
place* and then it would be easier to detect regressions for scripts
that don't actually run as part of the test suite.

>>>
 diff --git a/python/qemu/machine.py b/python/qemu/machine.py
 index b9a98e2c86..eaedc25172 100644
 --- a/python/qemu/machine.py
 +++ b/python/qemu/machine.py
 @@ -478,21 +478,21 @@ def event_wait(self, name, timeout=60.0, match=None):
  timeout: QEMUMonitorProtocol.pull_event timeout parameter.
  match: Optional match criteria. See event_match for details.
  """
 -return self.events_wait([(name, match)], timeout)
 +return self.events_wait({name: match}, timeout)
  
  def events_wait(self, events, timeout=60.0):
  """
  events_wait waits for and returns a named event from QMP with a 
 timeout.
  
 -events: a sequence of (name, match_criteria) tuples.
 +events: a mapping containing {name: match_criteria}.
  The match criteria are optional and may be None.
  See event_match for details.  timeout:
  QEMUMonitorProtocol.pull_event timeout parameter.
  """
  def _match(event):
 -for name, match in events:
 -if event['event'] == name and self.event_match(event, 
 match):
 -return True
 +name = event['event']
 +

Re: [PATCH v4 1/3] qmp.py: change event_wait to use a dict

2020-05-14 Thread Kevin Wolf
Am 14.05.2020 um 17:07 hat John Snow geschrieben:
> 
> 
> On 5/14/20 10:47 AM, Kevin Wolf wrote:
> > Am 14.05.2020 um 04:25 hat John Snow geschrieben:
> >> It's easier to work with than a list of tuples, because we can check the
> >> keys for membership.
> >>
> >> Signed-off-by: John Snow 
> >> ---
> >>  python/qemu/machine.py| 10 +-
> >>  tests/qemu-iotests/040| 12 ++--
> >>  tests/qemu-iotests/260|  5 +++--
> >>  tests/qemu-iotests/iotests.py | 16 
> >>  4 files changed, 22 insertions(+), 21 deletions(-)
> > 
> > I think you need to convert scripts/simplebench/bench_block_job.py, too.
> 
> Oh, right -- that one is new since I did this. A lot of these scripts
> need to be moved over into the python/ directory and managed under the
> same pylint/mypy regime as everything else.
> 
> A *ton* of our scripts are in various states of disrepair.

Is python/ actually supposed to have executable files in it? I thought
it was more for libraries.

> > 
> >> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
> >> index b9a98e2c86..eaedc25172 100644
> >> --- a/python/qemu/machine.py
> >> +++ b/python/qemu/machine.py
> >> @@ -478,21 +478,21 @@ def event_wait(self, name, timeout=60.0, match=None):
> >>  timeout: QEMUMonitorProtocol.pull_event timeout parameter.
> >>  match: Optional match criteria. See event_match for details.
> >>  """
> >> -return self.events_wait([(name, match)], timeout)
> >> +return self.events_wait({name: match}, timeout)
> >>  
> >>  def events_wait(self, events, timeout=60.0):
> >>  """
> >>  events_wait waits for and returns a named event from QMP with a 
> >> timeout.
> >>  
> >> -events: a sequence of (name, match_criteria) tuples.
> >> +events: a mapping containing {name: match_criteria}.
> >>  The match criteria are optional and may be None.
> >>  See event_match for details.  timeout:
> >>  QEMUMonitorProtocol.pull_event timeout parameter.
> >>  """
> >>  def _match(event):
> >> -for name, match in events:
> >> -if event['event'] == name and self.event_match(event, 
> >> match):
> >> -return True
> >> +name = event['event']
> >> +if name in events:
> >> +return self.event_match(event, events[name])
> > 
> > This part confused me a bit for a second. Of course, that's probably
> > mostly just me, but I feel 'events' isn't a good name any more when the
> > values of the dict are match strings rather than events.
> > 
> 
> This is honestly a really bad function. When I was trying to type
> everything, this one was at the bottom of the pile and it was the worst.
> 
> It needs an overhaul.
> 
> In my 32 patch series, I left the "match" types as "Any" pretty much
> everywhere, because it's such a laissez-faire series of functions.

It would require recursive types, which aren't supported yet. So I guess
Any is the best we can do at the moment.

> I'll keep the feedback in mind.
> 
> >>  return False
> >>  
> >>  # Search cached events
> >> diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
> >> index 32c82b4ec6..90b59081ff 100755
> >> --- a/tests/qemu-iotests/040
> >> +++ b/tests/qemu-iotests/040
> >> @@ -485,12 +485,12 @@ class TestErrorHandling(iotests.QMPTestCase):
> >>  
> >>  def run_job(self, expected_events, error_pauses_job=False):
> >>  match_device = {'data': {'device': 'job0'}}
> >> -events = [
> >> -('BLOCK_JOB_COMPLETED', match_device),
> >> -('BLOCK_JOB_CANCELLED', match_device),
> >> -('BLOCK_JOB_ERROR', match_device),
> >> -('BLOCK_JOB_READY', match_device),
> >> -]
> >> +events = {
> >> +'BLOCK_JOB_COMPLETED': match_device,
> >> +'BLOCK_JOB_CANCELLED': match_device,
> >> +'BLOCK_JOB_ERROR': match_device,
> >> +'BLOCK_JOB_READY': match_device,
> >> +}
> >>  
> >>  completed = False
> >>  log = []
> >> diff --git a/tests/qemu-iotests/260 b/tests/qemu-iotests/260
> >> index 804a7addb9..729f031122 100755
> >> --- a/tests/qemu-iotests/260
> >> +++ b/tests/qemu-iotests/260
> >> @@ -67,8 +67,9 @@ def test(persistent, restart):
> >>  
> >>  vm.qmp_log('block-commit', device='drive0', top=top,
> >> filters=[iotests.filter_qmp_testfiles])
> >> -ev = vm.events_wait((('BLOCK_JOB_READY', None),
> >> - ('BLOCK_JOB_COMPLETED', None)))
> >> +ev = vm.events_wait({
> >> +'BLOCK_JOB_READY': None,
> >> +'BLOCK_JOB_COMPLETED': None })
> > 
> > So, I'm not sure if this is nitpicking or rather bikeshedding, but
> > having the closing brackets on the next line would be more consistent
> > with the other instances in this patch.
> > 
> 
> Nah, it's fine. 

Re: [PATCH v4 1/3] qmp.py: change event_wait to use a dict

2020-05-14 Thread John Snow



On 5/14/20 10:47 AM, Kevin Wolf wrote:
> Am 14.05.2020 um 04:25 hat John Snow geschrieben:
>> It's easier to work with than a list of tuples, because we can check the
>> keys for membership.
>>
>> Signed-off-by: John Snow 
>> ---
>>  python/qemu/machine.py| 10 +-
>>  tests/qemu-iotests/040| 12 ++--
>>  tests/qemu-iotests/260|  5 +++--
>>  tests/qemu-iotests/iotests.py | 16 
>>  4 files changed, 22 insertions(+), 21 deletions(-)
> 
> I think you need to convert scripts/simplebench/bench_block_job.py, too.

Oh, right -- that one is new since I did this. A lot of these scripts
need to be moved over into the python/ directory and managed under the
same pylint/mypy regime as everything else.

A *ton* of our scripts are in various states of disrepair.

> 
>> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
>> index b9a98e2c86..eaedc25172 100644
>> --- a/python/qemu/machine.py
>> +++ b/python/qemu/machine.py
>> @@ -478,21 +478,21 @@ def event_wait(self, name, timeout=60.0, match=None):
>>  timeout: QEMUMonitorProtocol.pull_event timeout parameter.
>>  match: Optional match criteria. See event_match for details.
>>  """
>> -return self.events_wait([(name, match)], timeout)
>> +return self.events_wait({name: match}, timeout)
>>  
>>  def events_wait(self, events, timeout=60.0):
>>  """
>>  events_wait waits for and returns a named event from QMP with a 
>> timeout.
>>  
>> -events: a sequence of (name, match_criteria) tuples.
>> +events: a mapping containing {name: match_criteria}.
>>  The match criteria are optional and may be None.
>>  See event_match for details.  timeout:
>>  QEMUMonitorProtocol.pull_event timeout parameter.
>>  """
>>  def _match(event):
>> -for name, match in events:
>> -if event['event'] == name and self.event_match(event, 
>> match):
>> -return True
>> +name = event['event']
>> +if name in events:
>> +return self.event_match(event, events[name])
> 
> This part confused me a bit for a second. Of course, that's probably
> mostly just me, but I feel 'events' isn't a good name any more when the
> values of the dict are match strings rather than events.
> 

This is honestly a really bad function. When I was trying to type
everything, this one was at the bottom of the pile and it was the worst.

It needs an overhaul.

In my 32 patch series, I left the "match" types as "Any" pretty much
everywhere, because it's such a laissez-faire series of functions.

I'll keep the feedback in mind.

>>  return False
>>  
>>  # Search cached events
>> diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
>> index 32c82b4ec6..90b59081ff 100755
>> --- a/tests/qemu-iotests/040
>> +++ b/tests/qemu-iotests/040
>> @@ -485,12 +485,12 @@ class TestErrorHandling(iotests.QMPTestCase):
>>  
>>  def run_job(self, expected_events, error_pauses_job=False):
>>  match_device = {'data': {'device': 'job0'}}
>> -events = [
>> -('BLOCK_JOB_COMPLETED', match_device),
>> -('BLOCK_JOB_CANCELLED', match_device),
>> -('BLOCK_JOB_ERROR', match_device),
>> -('BLOCK_JOB_READY', match_device),
>> -]
>> +events = {
>> +'BLOCK_JOB_COMPLETED': match_device,
>> +'BLOCK_JOB_CANCELLED': match_device,
>> +'BLOCK_JOB_ERROR': match_device,
>> +'BLOCK_JOB_READY': match_device,
>> +}
>>  
>>  completed = False
>>  log = []
>> diff --git a/tests/qemu-iotests/260 b/tests/qemu-iotests/260
>> index 804a7addb9..729f031122 100755
>> --- a/tests/qemu-iotests/260
>> +++ b/tests/qemu-iotests/260
>> @@ -67,8 +67,9 @@ def test(persistent, restart):
>>  
>>  vm.qmp_log('block-commit', device='drive0', top=top,
>> filters=[iotests.filter_qmp_testfiles])
>> -ev = vm.events_wait((('BLOCK_JOB_READY', None),
>> - ('BLOCK_JOB_COMPLETED', None)))
>> +ev = vm.events_wait({
>> +'BLOCK_JOB_READY': None,
>> +'BLOCK_JOB_COMPLETED': None })
> 
> So, I'm not sure if this is nitpicking or rather bikeshedding, but
> having the closing brackets on the next line would be more consistent
> with the other instances in this patch.
> 

Nah, it's fine. I'll clean it up. This is pretty close to an RFC series
anyway, so I didn't really polish it.

(Or, I will try to clean it up. I probably won't work on it again in the
near term. I think I just wanted to see if this seemed useful in general
to people.

As part of maybe moving the python library onto a package, I thought
that maybe developing a JobRunner tool would be useful to have in that
library. As you can see, I nestled it into iotests.py, though.)

>>  log(filter_qmp_event(ev))
>> 

Re: [PATCH v4 1/3] qmp.py: change event_wait to use a dict

2020-05-14 Thread Kevin Wolf
Am 14.05.2020 um 04:25 hat John Snow geschrieben:
> It's easier to work with than a list of tuples, because we can check the
> keys for membership.
> 
> Signed-off-by: John Snow 
> ---
>  python/qemu/machine.py| 10 +-
>  tests/qemu-iotests/040| 12 ++--
>  tests/qemu-iotests/260|  5 +++--
>  tests/qemu-iotests/iotests.py | 16 
>  4 files changed, 22 insertions(+), 21 deletions(-)

I think you need to convert scripts/simplebench/bench_block_job.py, too.

> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
> index b9a98e2c86..eaedc25172 100644
> --- a/python/qemu/machine.py
> +++ b/python/qemu/machine.py
> @@ -478,21 +478,21 @@ def event_wait(self, name, timeout=60.0, match=None):
>  timeout: QEMUMonitorProtocol.pull_event timeout parameter.
>  match: Optional match criteria. See event_match for details.
>  """
> -return self.events_wait([(name, match)], timeout)
> +return self.events_wait({name: match}, timeout)
>  
>  def events_wait(self, events, timeout=60.0):
>  """
>  events_wait waits for and returns a named event from QMP with a 
> timeout.
>  
> -events: a sequence of (name, match_criteria) tuples.
> +events: a mapping containing {name: match_criteria}.
>  The match criteria are optional and may be None.
>  See event_match for details.  timeout:
>  QEMUMonitorProtocol.pull_event timeout parameter.
>  """
>  def _match(event):
> -for name, match in events:
> -if event['event'] == name and self.event_match(event, match):
> -return True
> +name = event['event']
> +if name in events:
> +return self.event_match(event, events[name])

This part confused me a bit for a second. Of course, that's probably
mostly just me, but I feel 'events' isn't a good name any more when the
values of the dict are match strings rather than events.

>  return False
>  
>  # Search cached events
> diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
> index 32c82b4ec6..90b59081ff 100755
> --- a/tests/qemu-iotests/040
> +++ b/tests/qemu-iotests/040
> @@ -485,12 +485,12 @@ class TestErrorHandling(iotests.QMPTestCase):
>  
>  def run_job(self, expected_events, error_pauses_job=False):
>  match_device = {'data': {'device': 'job0'}}
> -events = [
> -('BLOCK_JOB_COMPLETED', match_device),
> -('BLOCK_JOB_CANCELLED', match_device),
> -('BLOCK_JOB_ERROR', match_device),
> -('BLOCK_JOB_READY', match_device),
> -]
> +events = {
> +'BLOCK_JOB_COMPLETED': match_device,
> +'BLOCK_JOB_CANCELLED': match_device,
> +'BLOCK_JOB_ERROR': match_device,
> +'BLOCK_JOB_READY': match_device,
> +}
>  
>  completed = False
>  log = []
> diff --git a/tests/qemu-iotests/260 b/tests/qemu-iotests/260
> index 804a7addb9..729f031122 100755
> --- a/tests/qemu-iotests/260
> +++ b/tests/qemu-iotests/260
> @@ -67,8 +67,9 @@ def test(persistent, restart):
>  
>  vm.qmp_log('block-commit', device='drive0', top=top,
> filters=[iotests.filter_qmp_testfiles])
> -ev = vm.events_wait((('BLOCK_JOB_READY', None),
> - ('BLOCK_JOB_COMPLETED', None)))
> +ev = vm.events_wait({
> +'BLOCK_JOB_READY': None,
> +'BLOCK_JOB_COMPLETED': None })

So, I'm not sure if this is nitpicking or rather bikeshedding, but
having the closing brackets on the next line would be more consistent
with the other instances in this patch.

>  log(filter_qmp_event(ev))
>  if (ev['event'] == 'BLOCK_JOB_COMPLETED'):
>  vm.shutdown()

Kevin




[PATCH v4 1/3] qmp.py: change event_wait to use a dict

2020-05-13 Thread John Snow
It's easier to work with than a list of tuples, because we can check the
keys for membership.

Signed-off-by: John Snow 
---
 python/qemu/machine.py| 10 +-
 tests/qemu-iotests/040| 12 ++--
 tests/qemu-iotests/260|  5 +++--
 tests/qemu-iotests/iotests.py | 16 
 4 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index b9a98e2c86..eaedc25172 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -478,21 +478,21 @@ def event_wait(self, name, timeout=60.0, match=None):
 timeout: QEMUMonitorProtocol.pull_event timeout parameter.
 match: Optional match criteria. See event_match for details.
 """
-return self.events_wait([(name, match)], timeout)
+return self.events_wait({name: match}, timeout)
 
 def events_wait(self, events, timeout=60.0):
 """
 events_wait waits for and returns a named event from QMP with a 
timeout.
 
-events: a sequence of (name, match_criteria) tuples.
+events: a mapping containing {name: match_criteria}.
 The match criteria are optional and may be None.
 See event_match for details.
 timeout: QEMUMonitorProtocol.pull_event timeout parameter.
 """
 def _match(event):
-for name, match in events:
-if event['event'] == name and self.event_match(event, match):
-return True
+name = event['event']
+if name in events:
+return self.event_match(event, events[name])
 return False
 
 # Search cached events
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index 32c82b4ec6..90b59081ff 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -485,12 +485,12 @@ class TestErrorHandling(iotests.QMPTestCase):
 
 def run_job(self, expected_events, error_pauses_job=False):
 match_device = {'data': {'device': 'job0'}}
-events = [
-('BLOCK_JOB_COMPLETED', match_device),
-('BLOCK_JOB_CANCELLED', match_device),
-('BLOCK_JOB_ERROR', match_device),
-('BLOCK_JOB_READY', match_device),
-]
+events = {
+'BLOCK_JOB_COMPLETED': match_device,
+'BLOCK_JOB_CANCELLED': match_device,
+'BLOCK_JOB_ERROR': match_device,
+'BLOCK_JOB_READY': match_device,
+}
 
 completed = False
 log = []
diff --git a/tests/qemu-iotests/260 b/tests/qemu-iotests/260
index 804a7addb9..729f031122 100755
--- a/tests/qemu-iotests/260
+++ b/tests/qemu-iotests/260
@@ -67,8 +67,9 @@ def test(persistent, restart):
 
 vm.qmp_log('block-commit', device='drive0', top=top,
filters=[iotests.filter_qmp_testfiles])
-ev = vm.events_wait((('BLOCK_JOB_READY', None),
- ('BLOCK_JOB_COMPLETED', None)))
+ev = vm.events_wait({
+'BLOCK_JOB_READY': None,
+'BLOCK_JOB_COMPLETED': None })
 log(filter_qmp_event(ev))
 if (ev['event'] == 'BLOCK_JOB_COMPLETED'):
 vm.shutdown()
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 6c0e781af7..aada94f4f9 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -635,14 +635,14 @@ def run_job(self, job, auto_finalize=True, 
auto_dismiss=False,
 """
 match_device = {'data': {'device': job}}
 match_id = {'data': {'id': job}}
-events = [
-('BLOCK_JOB_COMPLETED', match_device),
-('BLOCK_JOB_CANCELLED', match_device),
-('BLOCK_JOB_ERROR', match_device),
-('BLOCK_JOB_READY', match_device),
-('BLOCK_JOB_PENDING', match_id),
-('JOB_STATUS_CHANGE', match_id)
-]
+events = {
+'BLOCK_JOB_COMPLETED': match_device,
+'BLOCK_JOB_CANCELLED': match_device,
+'BLOCK_JOB_ERROR': match_device,
+'BLOCK_JOB_READY': match_device,
+'BLOCK_JOB_PENDING': match_id,
+'JOB_STATUS_CHANGE': match_id,
+}
 error = None
 while True:
 ev = filter_qmp_event(self.events_wait(events, timeout=wait))
-- 
2.21.1