qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: John Snow <jsnow@redhat.com>
To: qemu-devel@nongnu.org
Cc: Eduardo Habkost <eduardo@habkost.net>,
	Kevin Wolf <kwolf@redhat.com>, Thomas Huth <thuth@redhat.com>,
	Beraldo Leal <bleal@redhat.com>,
	qemu-block@nongnu.org, Hanna Reitz <hreitz@redhat.com>,
	Cleber Rosa <crosa@redhat.com>, John Snow <jsnow@redhat.com>
Subject: [PATCH 4/4] iotests: make qemu_img raise on non-zero rc by default
Date: Tue, 15 Feb 2022 17:08:53 -0500	[thread overview]
Message-ID: <20220215220853.4179173-5-jsnow@redhat.com> (raw)
In-Reply-To: <20220215220853.4179173-1-jsnow@redhat.com>

re-configure qemu_img() into 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.

Users that want something more flexible (There appears to be only one)
can use check=False and manage the return themselves.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/257        |  8 ++++--
 tests/qemu-iotests/iotests.py | 53 +++++++++++++++++++++++++++++++----
 2 files changed, 53 insertions(+), 8 deletions(-)

diff --git a/tests/qemu-iotests/257 b/tests/qemu-iotests/257
index fb5359c581e..e7e7a2317e3 100755
--- a/tests/qemu-iotests/257
+++ b/tests/qemu-iotests/257
@@ -241,11 +241,13 @@ def compare_images(image, reference, baseimg=None, expected_match=True):
     expected_ret = 0 if expected_match else 1
     if baseimg:
         qemu_img("rebase", "-u", "-b", baseimg, '-F', iotests.imgfmt, image)
-    ret = qemu_img("compare", image, reference)
+
+    sub = qemu_img("compare", image, reference, check=False)
+
     log('qemu_img compare "{:s}" "{:s}" ==> {:s}, {:s}'.format(
         image, reference,
-        "Identical" if ret == 0 else "Mismatch",
-        "OK!" if ret == expected_ret else "ERROR!"),
+        "Identical" if sub.returncode == 0 else "Mismatch",
+        "OK!" if sub.returncode == expected_ret else "ERROR!"),
         filters=[iotests.filter_testfiles])
 
 def test_bitmap_sync(bsync_mode, msync_mode='bitmap', failure=None):
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 7df393df2c3..8a244fafece 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -249,9 +249,51 @@ def qemu_img_pipe_and_status(*args: str) -> Tuple[str, int]:
     return qemu_tool_pipe_and_status('qemu-img', full_args,
                                      drop_successful_output=is_create)
 
-def qemu_img(*args: str) -> int:
-    '''Run qemu-img and return the exit code'''
-    return qemu_img_pipe_and_status(*args)[1]
+def qemu_img(*args: str, check: bool = True, combine_stdio: bool = True
+             ) -> subprocess.CompletedProcess[str]:
+    """
+    Run qemu_img, returning a CompletedProcess instance.
+
+    The CompletedProcess object has args, returncode, and output properties.
+    If streams are not combined, It will also have stdout/stderr properties.
+
+    :param args: command-line arguments to qemu_img.
+    :param check: set to False to suppress VerboseProcessError.
+    :param combine_stdio: set to False to keep stdout/stderr separated.
+
+    :raise VerboseProcessError:
+        On non-zero exit code, when 'check=True' was provided. This
+        exception has 'stderr', 'stdout' and 'returncode' properties
+        that may be inspected to show greater detail. If this exception
+        is not handled, The command-line, return code, and all console
+        output will be included at the bottom of the stack trace.
+    """
+    full_args = qemu_img_args + qemu_img_create_prepare_args(list(args))
+
+    subp = subprocess.run(
+        full_args,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.STDOUT if combine_stdio else subprocess.PIPE,
+        universal_newlines=True,
+        check=False
+    )
+
+    # For behavioral consistency with qemu_tool_pipe_and_status():
+    # Print out an immediate error when the return code is negative.
+    if subp.returncode < 0:
+        cmd = ' '.join(full_args)
+        sys.stderr.write(
+            f'qemu-img received signal {-subp.returncode}: {cmd}\n')
+
+    if check and subp.returncode:
+        raise VerboseProcessError(
+            subp.returncode, full_args,
+            output=subp.stdout,
+            stderr=subp.stderr,
+        )
+
+    return subp
+
 
 def ordered_qmp(qmsg, conv_keys=True):
     # Dictionaries are not ordered prior to 3.6, therefore:
@@ -469,8 +511,9 @@ def qemu_nbd_popen(*args):
 
 def compare_images(img1, img2, fmt1=imgfmt, fmt2=imgfmt):
     '''Return True if two image files are identical'''
-    return qemu_img('compare', '-f', fmt1,
-                    '-F', fmt2, img1, img2) == 0
+    res = qemu_img('compare', '-f', fmt1,
+                   '-F', fmt2, img1, img2, check=False)
+    return res.returncode == 0
 
 def create_image(name, size):
     '''Create a fully-allocated raw image with sector markers'''
-- 
2.34.1



  parent reply	other threads:[~2022-02-15 22:28 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-15 22:08 [PATCH 0/4] iotests: add detailed tracebacks to qemu_img() failures John Snow
2022-02-15 22:08 ` [PATCH 1/4] python/utils: add enboxify() text decoration utility John Snow
2022-02-15 22:55   ` Eric Blake
2022-02-15 23:53     ` John Snow
2022-02-15 23:57       ` Philippe Mathieu-Daudé via
2022-02-16 16:16         ` John Snow
2022-02-16 16:54           ` Hanna Reitz
2022-02-15 22:08 ` [PATCH 2/4] iotests: add VerboseProcessError John Snow
2022-02-15 22:58   ` Eric Blake
2022-02-15 23:54     ` John Snow
2022-02-15 22:08 ` [PATCH 3/4] iotests: Remove explicit checks for qemu_img() == 0 John Snow
2022-02-15 23:04   ` Eric Blake
2022-02-15 23:57     ` John Snow
2022-02-15 22:08 ` John Snow [this message]
2022-02-15 23:09   ` [PATCH 4/4] iotests: make qemu_img raise on non-zero rc by default Eric Blake
2022-02-16  0:02     ` John Snow

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220215220853.4179173-5-jsnow@redhat.com \
    --to=jsnow@redhat.com \
    --cc=bleal@redhat.com \
    --cc=crosa@redhat.com \
    --cc=eduardo@habkost.net \
    --cc=hreitz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).