Re: [PATCH 14/15] iotests: remove qemu_io_silent() and qemu_io_silent_check().

[John Snow]

On Mon, Mar 21, 2022 at 2:16 PM Eric Blake <eblake@redhat.com> wrote:
>
> On Fri, Mar 18, 2022 at 04:36:54PM -0400, John Snow wrote:
> > Like qemu-img, qemu-io returning 0 should be the norm and not the
> > exception. Remove all calls to qemu_io_silent that just assert the
> > return code is zero (That's every last call, as it turns out), and
> > replace them with a normal qemu_io() call.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >  tests/qemu-iotests/216                        | 12 +++++-----
> >  tests/qemu-iotests/218                        |  5 ++---
> >  tests/qemu-iotests/224                        |  4 ++--
> >  tests/qemu-iotests/258                        | 12 +++++-----
> >  tests/qemu-iotests/298                        | 16 ++++++--------
> >  tests/qemu-iotests/310                        | 22 +++++++++----------
> >  tests/qemu-iotests/iotests.py                 | 16 --------------
> >  tests/qemu-iotests/tests/image-fleecing       |  4 ++--
> >  .../tests/mirror-ready-cancel-error           |  2 +-
> >  .../qemu-iotests/tests/stream-error-on-reset  |  4 ++--
> >  10 files changed, 39 insertions(+), 58 deletions(-)
>
> > +++ b/tests/qemu-iotests/258
> > @@ -21,7 +21,7 @@
> >  # Creator/Owner: Max Reitz <mreitz@redhat.com>
> >
> >  import iotests
> > -from iotests import log, qemu_img, qemu_io_silent, \
> > +from iotests import log, qemu_img, qemu_io, \
> >          filter_qmp_testfiles, filter_qmp_imgfmt
> >
> >  # Returns a node for blockdev-add
> > @@ -86,15 +86,15 @@ def test_concurrent_finish(write_to_stream_node):
> >          if write_to_stream_node:
> >              # This is what (most of the time) makes commit finish
> >              # earlier and then pull in stream
> > -            assert qemu_io_silent(node2_path,
> > -                                  '-c', 'write %iK 64K' % (65536 - 192),
> > -                                  '-c', 'write %iK 64K' % (65536 -  64)) 
> > == 0
> > +            qemu_io(node2_path,
> > +                    '-c', 'write %iK 64K' % (65536 - 192),
> > +                    '-c', 'write %iK 64K' % (65536 -  64))
> >
> >              stream_throttle='tg'
> >          else:
> >              # And this makes stream finish earlier
> > -            assert qemu_io_silent(node1_path,
> > -                                  '-c', 'write %iK 64K' % (65536 - 64)) == > > 0
> > +            qemu_io(node1_path,
> > +                    '-c', 'write %iK 64K' % (65536 - 64))
>
> This could fit on one line.  But the split matches the instance
> earlier in the hunk that needed two lines.
>
> >
> >              commit_throttle='tg'
> >
> > diff --git a/tests/qemu-iotests/298 b/tests/qemu-iotests/298
> > index fae72211b1..9d6d95d953 100755
> > --- a/tests/qemu-iotests/298
> > +++ b/tests/qemu-iotests/298
> > @@ -129,16 +129,14 @@ class TestTruncate(iotests.QMPTestCase):
> >          os.remove(refdisk)
> >
> >      def do_test(self, prealloc_mode, new_size):
> > -        ret = iotests.qemu_io_silent('--image-opts', '-c', 'write 0 10M', 
> > '-c',
> > -                                     f'truncate -m {prealloc_mode} 
> > {new_size}',
> > -                                     drive_opts)
> > -        self.assertEqual(ret, 0)
> > +        iotests.qemu_io('--image-opts', '-c', 'write 0 10M', '-c',
> > +                        f'truncate -m {prealloc_mode} {new_size}',
> > +                        drive_opts)
> >
> > -        ret = iotests.qemu_io_silent('-f', iotests.imgfmt, '-c', 'write 0 
> > 10M',
> > -                                     '-c',
> > -                                     f'truncate -m {prealloc_mode} 
> > {new_size}',
> > -                                     refdisk)
> > -        self.assertEqual(ret, 0)
> > +        iotests.qemu_io('-f', iotests.imgfmt, '-c', 'write 0 10M',
> > +                        '-c',
> > +                        f'truncate -m {prealloc_mode} {new_size}',
>
> And as long as I'm pontificating on line wraps, putting '-c' and
> f'truncate...' on the same line might make sense.
>
> At any rate, whether or not you choose to do anything about my
> observations on cosmetic line wraps:
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>

Rolled them in, why not.

--js