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 <jsnow@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 <jsnow@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):