Re: [PATCH v3 4/5] iotests: make qemu_img raise on non-zero rc by default

[John Snow <jsnow@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2145 bytes --]

On Thu, Mar 17, 2022, 6:41 AM Hanna Reitz <hreitz@redhat.com> wrote:

> On 17.03.22 11:25, Hanna Reitz wrote:
> > On 08.03.22 02:57, John Snow wrote:
> >> re-write qemu_img() as a function that will by default raise a
> >> VerboseProcessException (extended from CalledProcessException) on
> >> non-zero return codes. This will produce a stack trace that will show
> >> the command line arguments and return code from the failed process run.
> >
> > Why not qemu_img_pipe_and_status() as the central function where all
> > qemu-img calls go through?
>
> OK, I see that your follow-up series effectively does this.  Still
> wondering why this patch here doesn’t touch qemu_img_pipe_and_status()
> instead.
>

Just a bad habit, I guess. It's the way I wrote this series: add a new
thing, then move the old things over to use it gradually.

This patchset (and the next) is pretty much the order I actually wrote it
in.

I do prefer the shorter name qemu_img() for this fn, tho.

(I struggle a lot with the order I write not being the order most people
prefer for reading. I feel like I've never quite gotten that correct. I
suppose I like to work backwards: start at the code I want and work
backwards until it works again.)


> > It seems like this makes qemu_img() a second version of
> > qemu_img_pipe_and_status(), which is a bit weird.
> >
> > (Or perhaps it should actually be qemu_tool_pipe_and_status() that
> > receives this treatment, with qemu-io functions just passing
> > check=False by default.)
>
> (And perhaps this, but I guess qemu-io is the only other real user of
> qemu_tool_pipe_and_status(), so if it doesn’t care, then there’s no real
> reason to change that function.)
>

Similar reasoning: I'm not actually sure I can justify the change
everywhere yet. I worked through all of qemu-io, but calls to qemu-nbd and
qemu itself are not yet audited.

In the end, that's the goal. Working my way backwards until replacing all
of these functions, yes.

Sorry for my backwards brain, maybe. I felt doing it this way got us the
most benefit the quickest.


> Hanna
>
>

[-- Attachment #2: Type: text/html, Size: 3229 bytes --]