|
From: | Hanna Reitz |
Subject: | Re: [PATCH 14/14] iotests: make img_info_log() call qemu_img_log() |
Date: | Thu, 17 Mar 2022 19:26:57 +0100 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 |
On 17.03.22 18:45, John Snow wrote:
On Thu, Mar 17, 2022 at 1:00 PM John Snow <jsnow@redhat.com> wrote:On Thu, Mar 17, 2022 at 11:39 AM Hanna Reitz <hreitz@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 <jsnow@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 exceptionsOh, 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.
:CI 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:)
Hanna
[Prev in Thread] | Current Thread | [Next in Thread] |