On Mon, Mar 21, 2022, 11:50 AM Hanna Reitz <hreitz@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 <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.
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.