On Tue, Mar 22, 2022, 11:04 AM Hanna Reitz <hre...@redhat.com> wrote:
> On 18.03.22 21:36, John Snow wrote:
> > Rework qemu_io() to be analogous to qemu_img(); a function that requires
> > a return code of zero by default unless disabled explicitly.
> >
> > Tests that use qemu_io():
> > 030 040 041 044 055 056 093 124 129 132 136 148 149 151 152 163 165 205
> > 209 219 236 245 248 254 255 257 260 264 280 298 300 302 304
> > image-fleecing migrate-bitmaps-postcopy-test migrate-bitmaps-test
> > migrate-during-backup migration-permissions
> >
> > Test that use qemu_io_log():
> > 242 245 255 274 303 307 nbd-reconnect-on-open
> >
> > Signed-off-by: John Snow <js...@redhat.com>
> >
> > ---
> >
> > Note: This breaks several tests at this point. I'll be fixing each
> > broken test one by one in the subsequent commits. We can squash them all
> > on merge to avoid test regressions.
>
> Well, absolutely.
>
> > (Seems like a way to have your cake and eat it too with regards to
> > maintaining bisectability while also having nice mailing list patches.)
>
> I personally find reviewability to not be affected whether this is one
> patch or multiple, given that the changes are in different files anyway.
>
> I am afraid someone might forgot to squash when merging this series,
> though...
>
> Also, I don’t know how to squash R-b tags, and I don’t feel like I can
> give an R-b for a patch that decidedly breaks tests.
>
> >
> > Copy-pastables:
> >
> > ./check -qcow2 030 040 041 044 055 056 124 129 132 151 152 163 165 209 \
> > 219 236 242 245 248 254 255 257 260 264 274 \
> > 280 298 300 302 303 304 307 image-fleecing \
> > migrate-bitmaps-postcopy-test migrate-bitmaps-test \
> > migrate-during-backup nbd-reconnect-on-open
> >
> > ./check -raw 093 136 148 migration-permissions
> >
> > ./check -nbd 205
> >
> > # ./configure configure --disable-gnutls --enable-gcrypt
> > # this ALSO requires passwordless sudo.
> > ./check -luks 149
> >
> >
> > # Just the ones that fail:
> > ./check -qcow2 030 040 242 245
> > ./check -raw migration-permissions
> > ./check -nbd 205
> > ./check -luks 149
> >
> > Signed-off-by: John Snow <js...@redhat.com>
> > ---
> > tests/qemu-iotests/iotests.py | 19 +++++++++++++------
> > 1 file changed, 13 insertions(+), 6 deletions(-)
> >
> > diff --git a/tests/qemu-iotests/iotests.py
> b/tests/qemu-iotests/iotests.py
> > index 974a2b0c8d..58ea766568 100644
> > --- a/tests/qemu-iotests/iotests.py
> > +++ b/tests/qemu-iotests/iotests.py
> > @@ -354,16 +354,23 @@ def qemu_io_wrap_args(args: Sequence[str]) ->
> List[str]:
> > def qemu_io_popen(*args):
> > return qemu_tool_popen(qemu_io_wrap_args(args))
> >
> > -def qemu_io(*args):
> > - '''Run qemu-io and return the stdout data'''
> > - return qemu_tool_pipe_and_status('qemu-io',
> qemu_io_wrap_args(args))[0]
> > +def qemu_io(*args: str, check: bool = True, combine_stdio: bool = True
> > + ) -> subprocess.CompletedProcess[str]:
>
> I guess this return type probably has to be quoted.
>
Yep. Sent this just before I figured out the problem from the prior series.
I'll make sure this whole series passes CI before I send it out a second
time.
I'll rebase on your staging branch and take my time with v2.
> > + """
> > + Run QEMU_IO_PROG and return the status code and console output.
> > +
> > + This function always prepends either QEMU_IO_OPTIONS or
> > + QEMU_IO_OPTIONS_NO_FMT.
> > + """
> > + return qemu_tool(*qemu_io_wrap_args(args),
> > + check=check, combine_stdio=combine_stdio)
> >
> > def qemu_io_pipe_and_status(*args):
> > return qemu_tool_pipe_and_status('qemu-io',
> qemu_io_wrap_args(args))
> >
> > -def qemu_io_log(*args):
> > - result = qemu_io(*args)
> > - log(result, filters=[filter_testfiles, filter_qemu_io])
> > +def qemu_io_log(*args: str) -> subprocess.CompletedProcess[str]:
>
> ...and this one.
>
> Hanna
>
> > + result = qemu_io(*args, check=False)
> > + log(result.stdout, filters=[filter_testfiles, filter_qemu_io])
> > return result
> >
> > def qemu_io_silent(*args):
>
>
