Re: [PATCH v6 14/14] python: use vm.cmd() instead of vm.qmp() where appropriate

[Vladimir Sementsov-Ogievskiy]

Thanks a lot for reviewing!

On 05.10.23 23:29, Eric Blake wrote:
> On Thu, Oct 05, 2023 at 04:55:50PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> > In many cases we just want an effect of qmp command and want to raise on
> > failure. Use vm.cmd() method which does exactly this.
> >
> > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> > ---
> >   tests/avocado/vnc.py                          |  16 +-
> >   tests/qemu-iotests/030                        | 168 +++---
> >   tests/qemu-iotests/040                        | 172 +++----

[..]

> >   tests/qemu-iotests/tests/mirror-top-perms     |  16 +-
> >   tests/qemu-iotests/tests/nbd-multiconn        |  12 +-
> >   tests/qemu-iotests/tests/reopen-file          |   3 +-
> >   .../qemu-iotests/tests/stream-error-on-reset  |   6 +-
> >   41 files changed, 951 insertions(+), 1407 deletions(-)
>
> Big but mechanical.  It would be worth amending the commit message to
> describe how you found all these spots (in case someone backporting
> this patch has to redo the work over a different subset of files based
> on what has changed since the two trees diverged).

Oops good notice.

I ofcourse didn't remember:) I started from searching for 'self.assert_qmp(result, 
'return', {})", and found that there are unhandled cases, so the patch is a bit 
outdated and misses some new cases.

I started to write a script and found that this is not easy) I decided to 
search the script in my old good scripts folder, and found it! I remember, that 
I didn't posted it, because it was neither simple nor beautiful.. And too big 
for commit message. But seems, better is to post it still, as a separate commit 
maybe.

Feel sorry to send unupdated patch :(

> > diff --git a/tests/avocado/vnc.py b/tests/avocado/vnc.py
> > index aeeefc70be..862c8996a8 100644
> > --- a/tests/avocado/vnc.py
> > +++ b/tests/avocado/vnc.py
> > @@ -88,9 +88,8 @@ def test_change_password(self):
> >           self.vm.add_args('-nodefaults', '-S', '-vnc', ':0,password=on')
> >           self.vm.launch()
> >           self.assertTrue(self.vm.qmp('query-vnc')['return']['enabled'])
> > -        set_password_response = self.vm.qmp('change-vnc-password',
> > -                                            password='new_password')
> > -        self.assertEqual(set_password_response['return'], {})
> > +        self.vm.cmd('change-vnc-password',
> > +                    password='new_password')
>
> Indeed a nicer idiom, where you are able to use it (whether by
> self.assertEqual or by self.assert_qmp).
>
> Reviewed-by: Eric Blake <eblake@redhat.com>

--
Best regards,
Vladimir