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 <hreitz@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.
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.)