On Thu, Mar 17, 2022 at 2:27 PM Hanna Reitz <hre...@redhat.com> wrote: > > On 17.03.22 18:45, John Snow wrote: > > On Thu, Mar 17, 2022 at 1:00 PM John Snow <js...@redhat.com> wrote: > >> On Thu, Mar 17, 2022 at 11:39 AM Hanna Reitz <hre...@redhat.com> wrote: > >>> On 09.03.22 04:54, John Snow wrote: > >>>> Add configurable filters to qemu_img_log(), and re-write img_info_log() > >>>> to call into qemu_img_log() with a custom filter instead. > >>>> > >>>> After this patch, every last call to qemu_img() is now guaranteed to > >>>> either have its return code checked for zero, OR have its output > >>>> actually visibly logged somewhere. > >>>> > >>>> Signed-off-by: John Snow <js...@redhat.com> > >>>> --- > >>>> tests/qemu-iotests/iotests.py | 13 +++++++++---- > >>>> 1 file changed, 9 insertions(+), 4 deletions(-) > >>> From my POV, this is a regression because before this patch (not this > >>> series, though, admittedly), `img_info_log()` would throw an exception > >>> on error, and with patch 12 being as it is, it will revert to its > >>> pre-series behavior of not throwing an exception. I prefer exceptions > > Oh, actually... patch #12 does this: > > > > - output = qemu_img_pipe(*args) > > + output = qemu_img(*args, check=False).stdout > > :( > > You’re right, I missed that. > > > so I never actually toggled error checking on for this function at > > all. This isn't a regression. > > > > At a glance, img_info_log() calls fail as a matter of course in 242 > > and 266 and ... hm, I can't quite test 207, it doesn't work for me, > > even before this series. > > Ugh, broken in e3296cc796aeaf319f3ed4e064ec309baf5e4da4. > > (putting that on my TOFIX list) > > > I didn't test *all* qemu_img calls yet either, but ... I'm going to > > gently suggest that "converting logged calls to qemu_img() to be > > checked calls" is "for another series" material. > > :C > > I mean, adding a `check` parameter to `img_info_log()` and > `qemu_img_log()` would be something like a 9+/5- diff (including 242 and > 266 changes, but disregarding the necessary comment changes in > `qemu_img_log()`). I think that’d be fine, and a bit thin for its own > “series”. O:)
You're right. I uh. actually started doing this after I told you I wasn't going to, because I was surprised by how few calls there were. so I changed my mind about not wanting to audit them. Merry Christmas! --js