On Fri, Mar 18, 2022 at 8:53 AM Hanna Reitz <hre...@redhat.com> wrote: > > Allow filters for VM.run_job(), and pass the filters given to > VM.blockdev_create() to it. > > (Use this opportunity to annotate VM.run_job()'s parameter types; > unfortunately, for the filter, I could not come up with anything better > than Callable[[Any], Any] that would pass mypy's scrutiny.) >
Yeah, I wrote some of this stuff ... before I started using mypy, and I'd do it differently if I had to again. (And might still do so: pulling out things like a generalized Job Runner is still on my Someday pile, especially now that I have an async QMP module to play with.) Long story short: Yeah, sure, cool, I don't want to do any better than this right now either. > At one point, a plain string is logged, so the filters passed to it must > work fine with plain strings. The only filters passed to it at this > point are the ones from VM.blockdev_create(), which are > filter_qmp_test_files() (by default) and 207's filter_hash(). Both > cannot handle plain strings yet, but we can make them by amending > filter_qmp() to treat them as plain values with a None key. > > Signed-off-by: Hanna Reitz <hre...@redhat.com> Looks fine enough to me for now. Reviewed-by: John Snow <js...@redhat.com> > --- > tests/qemu-iotests/iotests.py | 26 ++++++++++++++++---------- > 1 file changed, 16 insertions(+), 10 deletions(-) > > diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py > index 508adade9e..ad62d1f641 100644 > --- a/tests/qemu-iotests/iotests.py > +++ b/tests/qemu-iotests/iotests.py > @@ -521,8 +521,10 @@ def filter_qmp(qmsg, filter_fn): > # Iterate through either lists or dicts; > if isinstance(qmsg, list): > items = enumerate(qmsg) > - else: > + elif isinstance(qmsg, dict): > items = qmsg.items() > + else: > + return filter_fn(None, qmsg) > > for k, v in items: > if isinstance(v, (dict, list)): > @@ -858,8 +860,12 @@ def qmp_log(self, cmd, filters=(), indent=None, > **kwargs): > return result > > # Returns None on success, and an error string on failure > - def run_job(self, job, auto_finalize=True, auto_dismiss=False, > - pre_finalize=None, cancel=False, wait=60.0): > + def run_job(self, job: str, auto_finalize: bool = True, > + auto_dismiss: bool = False, > + pre_finalize: Optional[Callable[[], None]] = None, > + cancel: bool = False, wait: float = 60.0, > + filters: Iterable[Callable[[Any], Any]] = (), > + ) -> Optional[str]: > """ > run_job moves a job from creation through to dismissal. > > @@ -889,7 +895,7 @@ def run_job(self, job, auto_finalize=True, > auto_dismiss=False, > while True: > ev = filter_qmp_event(self.events_wait(events, timeout=wait)) > if ev['event'] != 'JOB_STATUS_CHANGE': > - log(ev) > + log(ev, filters=filters) > continue > status = ev['data']['status'] > if status == 'aborting': > @@ -897,18 +903,18 @@ def run_job(self, job, auto_finalize=True, > auto_dismiss=False, > for j in result['return']: > if j['id'] == job: > error = j['error'] > - log('Job failed: %s' % (j['error'])) > + log('Job failed: %s' % (j['error']), filters=filters) > elif status == 'ready': > - self.qmp_log('job-complete', id=job) > + self.qmp_log('job-complete', id=job, filters=filters) > elif status == 'pending' and not auto_finalize: > if pre_finalize: > pre_finalize() > if cancel: > - self.qmp_log('job-cancel', id=job) > + self.qmp_log('job-cancel', id=job, filters=filters) > else: > - self.qmp_log('job-finalize', id=job) > + self.qmp_log('job-finalize', id=job, filters=filters) > elif status == 'concluded' and not auto_dismiss: > - self.qmp_log('job-dismiss', id=job) > + self.qmp_log('job-dismiss', id=job, filters=filters) > elif status == 'null': > return error > > @@ -921,7 +927,7 @@ def blockdev_create(self, options, job_id='job0', > filters=None): > > if 'return' in result: > assert result['return'] == {} > - job_result = self.run_job(job_id) > + job_result = self.run_job(job_id, filters=filters) > else: > job_result = result['error'] > > -- > 2.35.1 >