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.