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?) > > Hanna > Yep, 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. (Annoying: I test under python 3.6, but I didn't *run the iotests with 3.6*, which is where this problem shows up. Meh.)
