On 21.03.22 17:23, John Snow wrote:
On Mon, Mar 21, 2022, 11:50 AM Hanna Reitz <hre...@redhat.com> wrote: 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.Agree. (Though I did shorten the lines in my re-spin to see if I could make it shut up, but it didn't work. Ignoring it is.)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.) Yeah, I don't think this is me. 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?) HannaYep, sorry for not replying. I respun the series and tested it, but it became "way too Saturday" for me to hit send on the respin. Will do so today.
Great, thanks! Hanna
