On 21.03.22 14:14, Hanna Reitz wrote:
On 18.03.22 22:14, John Snow wrote:
On Fri, Mar 18, 2022 at 9:36 AM Hanna Reitz <hre...@redhat.com> wrote:
On 18.03.22 00:49, John Snow wrote:
Hiya!
This series effectively replaces qemu_img_pipe_and_status() with a
rewritten function named qemu_img() that raises an exception on
non-zero
return code by default. By the end of the series, every last
invocation
of the qemu-img binary ultimately goes through qemu_img().
The exception that this function raises includes stdout/stderr output
when the traceback is printed in a a little decorated text box so that
it stands out from the jargony Python traceback readout.
(You can test what this looks like for yourself, or at least you
could,
by disabling ztsd support and then running qcow2 iotest 065.)
Negative tests are still possible in two ways:
- Passing check=False to qemu_img, qemu_img_log, or img_info_log
- Catching and handling the CalledProcessError exception at the
callsite.
Thanks! Applied to my block branch:
https://gitlab.com/hreitz/qemu/-/commits/block
Hanna
Actually, hold it -- this looks like it is causing problems with the
Gitlab CI. I need to investigate these.
https://gitlab.com/jsnow/qemu/-/pipelines/495155073/failures
... and, ugh, naturally the nice error diagnostics are suppressed here
so I can't see them. Well, there's one more thing to try and fix
somehow.
I hope this patch by Thomas fixes the logging at least:
https://lists.nongnu.org/archive/html/qemu-devel/2022-03/msg02946.html
So I found three issues:
1. check-patch wrongfully complains about the comment added in in
“python/utils: add add_visual_margin() text decoration utility” that
shows an example for how the output looks. It complains the lines
consisting mostly of “━━━━━━━━” were too long. I believe that’s because
it counts bytes, not characters.
Not fatal, i.e. doesn’t break the pipeline. We should ignore that.
2. riscv64-debian-cross-container breaks, but that looks pre-existing.
apt complains about some dependencies.
Also marked as allowed-to-fail, so I believe we should also just ignore
that. (Seems to fail on `master`, too.)
3. The rest are runs complaining about
`subprocess.CompletedProcess[str]`. Looks like the same issue I was
facing for ec88eed8d14088b36a3495710368b8d1a3c33420, where I had to
specify the type as a string.
Indeed this is fixed by something like
https://gitlab.com/hreitz/qemu/-/commit/87615eb536bdca7babe8eb4a35fd4ea810d1da24
. Maybe squash that in? (If it’s the correct way to go about this?)
Hanna
