Skip to contentSkip to conversation

qemu-io: Exit with error when a command failed

PATCH Superseded Max Reitz 2 participants 12 messages Apr 28, 2018 – Apr 30, 2018

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Max Reitz

author
#0 lore raw

cover qemu-io: Exit with error when a command failed

Right now, qemu-io's exit code is rather useless as it is usually 0. Except sometimes, then it's 1 in case of an error (mostly when you specify a filename as an argument and it cannot open that).

At the same time, most command functions' return values are rather useless as they are usually 0 (meaning "continue execution"). There is only a single function that breaks that pattern, which is "quit". On one hand, this is pointless because "quit" is in qemu-io.c, so it can easily signal that fact through a global (yet static) variable. On the other, it breaks the usual pattern of I/O functions returning error codes.

This series resolves the overlap between both issues by making the command functions' return error values instead of whether to continue execution or not, and thus makes qemu-io return 1 if any of the commands executed has failed and 0 only if all of them have succeeded.

Patch 5 showcases how that may be useful for iotests.

See also: https://bugzilla.redhat.com/show_bug.cgi?id=1519617

Max Reitz (5):
qemu-io: Drop command functions' return values
qemu-io: Let command functions return error code
qemu-io: Exit with error when a command failed iotests.py: Add qemu_io_silent
iotests: Let 216 make use of qemu-io's exit code

 include/qemu-io.h             |   4 +-
 qemu-io-cmds.c                | 276 ++++++++++++++++++++++++------------------
 qemu-io.c                     |  62 +++++++---
 tests/qemu-iotests/216        |  23 ++--
 tests/qemu-iotests/216.out    |  17 +--
 tests/qemu-iotests/iotests.py |   9 ++
 6 files changed, 226 insertions(+), 165 deletions(-)
Signature · 1 line
2.14.3
author
#1 lore raw

PATCH 1/5 qemu-io: Drop command functions' return values

For qemu-io, a function returns an integer with two possible values: 0 for "qemu-io may continue execution", or 1 for "qemu-io should exit". However, there is only a single command that returns 1, and that is "quit".

So let's turn this case into a global variable instead so we can make better use of the return value in a later patch.

Signed-off-by Max Reitz <[email protected]>
---
 include/qemu-io.h |   6 +-
 qemu-io-cmds.c    | 294 +++++++++++++++++++++++++-----------------------------
 qemu-io.c         |  36 +++----
 3 files changed, 157 insertions(+), 179 deletions(-)
include/qemu-io.h +3 -3
index 196fde0f3a..06cdfbf660 100644
@@ -22,7 +22,7 @@
 
 #define CMD_FLAG_GLOBAL ((int)0x80000000) /* don't iterate "args" */
 
-typedef int (*cfunc_t)(BlockBackend *blk, int argc, char **argv);
+typedef void (*cfunc_t)(BlockBackend *blk, int argc, char **argv);
 typedef void (*helpfunc_t)(void);
 
 typedef struct cmdinfo {

@@ -41,10 +41,10 @@ typedef struct cmdinfo {
 
 extern bool qemuio_misalign;
 
-bool qemuio_command(BlockBackend *blk, const char *cmd);
+void qemuio_command(BlockBackend *blk, const char *cmd);
 
 void qemuio_add_command(const cmdinfo_t *ci);
-int qemuio_command_usage(const cmdinfo_t *ci);
+void qemuio_command_usage(const cmdinfo_t *ci);
 void qemuio_complete_command(const char *input,
                              void (*fn)(const char *cmd, void *opaque),
                              void *opaque);
qemu-io-cmds.c +136 -158

1,052 lines not on this page · raw message

qemu-io.c +18 -18
index e692c555e0..4578ccfaa5 100644
@@ -37,6 +37,7 @@
 static char *progname;
 
 static BlockBackend *qemuio_blk;
+static bool quit_qemu_io;
 
 /* qemu-io commands passed using -c */
 static int ncmdline;

@@ -65,11 +66,10 @@ static int get_eof_char(void)
 #endif
 }
 
-static int close_f(BlockBackend *blk, int argc, char **argv)
+static void close_f(BlockBackend *blk, int argc, char **argv)
 {
     blk_unref(qemuio_blk);
     qemuio_blk = NULL;
-    return 0;
 }
 
 static const cmdinfo_t close_cmd = {

@@ -136,7 +136,7 @@ static void open_help(void)
 "\n");
 }
 
-static int open_f(BlockBackend *blk, int argc, char **argv);
+static void open_f(BlockBackend *blk, int argc, char **argv);
 
 static const cmdinfo_t open_cmd = {
     .name       = "open",

@@ -160,7 +160,7 @@ static QemuOptsList empty_opts = {
     },
 };
 
-static int open_f(BlockBackend *blk, int argc, char **argv)
+static void open_f(BlockBackend *blk, int argc, char **argv)
 {
     int flags = BDRV_O_UNMAP;
     int readonly = 0;

@@ -192,25 +192,25 @@ static int open_f(BlockBackend *blk, int argc, char **argv)
             if (bdrv_parse_cache_mode(optarg, &flags, &writethrough) < 0) {
                 error_report("Invalid cache option: %s", optarg);
                 qemu_opts_reset(&empty_opts);
-                return 0;
+                return;
             }
             break;
         case 'd':
             if (bdrv_parse_discard_flags(optarg, &flags) < 0) {
                 error_report("Invalid discard option: %s", optarg);
                 qemu_opts_reset(&empty_opts);
-                return 0;
+                return;
             }
             break;
         case 'o':
             if (imageOpts) {
                 printf("--image-opts and 'open -o' are mutually exclusive\n");
                 qemu_opts_reset(&empty_opts);
-                return 0;
+                return;
             }
             if (!qemu_opts_parse_noisily(&empty_opts, optarg, false)) {
                 qemu_opts_reset(&empty_opts);
-                return 0;
+                return;
             }
             break;
         case 'U':

@@ -218,7 +218,8 @@ static int open_f(BlockBackend *blk, int argc, char **argv)
             break;
         default:
             qemu_opts_reset(&empty_opts);
-            return qemuio_command_usage(&open_cmd);
+            qemuio_command_usage(&open_cmd);
+            return;
         }
     }
 

@@ -229,7 +230,7 @@ static int open_f(BlockBackend *blk, int argc, char **argv)
     if (imageOpts && (optind == argc - 1)) {
         if (!qemu_opts_parse_noisily(&empty_opts, argv[optind], false)) {
             qemu_opts_reset(&empty_opts);
-            return 0;
+            return;
         }
         optind++;
     }

@@ -246,12 +247,11 @@ static int open_f(BlockBackend *blk, int argc, char **argv)
         QDECREF(opts);
         qemuio_command_usage(&open_cmd);
     }
-    return 0;
 }
 
-static int quit_f(BlockBackend *blk, int argc, char **argv)
+static void quit_f(BlockBackend *blk, int argc, char **argv)
 {
-    return 1;
+    quit_qemu_io = true;
 }
 
 static const cmdinfo_t quit_cmd = {

@@ -392,18 +392,18 @@ static void prep_fetchline(void *opaque)
 
 static void command_loop(void)
 {
-    int i, done = 0, fetchable = 0, prompted = 0;
+    int i, fetchable = 0, prompted = 0;
     char *input;
 
-    for (i = 0; !done && i < ncmdline; i++) {
-        done = qemuio_command(qemuio_blk, cmdline[i]);
+    for (i = 0; !quit_qemu_io && i < ncmdline; i++) {
+        qemuio_command(qemuio_blk, cmdline[i]);
     }
     if (cmdline) {
         g_free(cmdline);
         return;
     }
 
-    while (!done) {
+    while (!quit_qemu_io) {
         if (!prompted) {
             printf("%s", get_prompt());
             fflush(stdout);

@@ -421,7 +421,7 @@ static void command_loop(void)
         if (input == NULL) {
             break;
         }
-        done = qemuio_command(qemuio_blk, input);
+        qemuio_command(qemuio_blk, input);
         g_free(input);
 
         prompted = 0;
Signature · 1 line
2.14.3
Reviewed-by on v1 1/5
#2 lore raw

Re: [Qemu-devel] [PATCH 1/5] qemu-io: Drop command functions' return values

Quoted 8 lines · Max Reitz

For qemu-io, a function returns an integer with two possible values: 0 for "qemu-io may continue execution", or 1 for "qemu-io should exit". However, there is only a single command that returns 1, and that is "quit".

So let's turn this case into a global variable instead so we can make better use of the return value in a later patch.

Quoted 7 lines
Signed-off-by Max Reitz <[email protected]>
---
  include/qemu-io.h |   6 +-
  qemu-io-cmds.c    | 294 +++++++++++++++++++++++++-----------------------------
  qemu-io.c         |  36 +++----
  3 files changed, 157 insertions(+), 179 deletions(-)
Reviewed-by Eric Blake <[email protected]>
Signature · 3 lines
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
author
#3 lore raw

PATCH 2/5 qemu-io: Let command functions return error code

In reply to Max Reitz

This is basically what everything else in the qemu code base does, so we can do it here, too.

Signed-off-by Max Reitz <[email protected]>
---
 include/qemu-io.h |   4 +-
 qemu-io-cmds.c    | 346 ++++++++++++++++++++++++++++++++----------------------
 qemu-io.c         |  34 ++++--
 3 files changed, 227 insertions(+), 157 deletions(-)
include/qemu-io.h +2 -2
index 06cdfbf660..380724ad59 100644
@@ -22,7 +22,7 @@
 
 #define CMD_FLAG_GLOBAL ((int)0x80000000) /* don't iterate "args" */
 
-typedef void (*cfunc_t)(BlockBackend *blk, int argc, char **argv);
+typedef int (*cfunc_t)(BlockBackend *blk, int argc, char **argv);
 typedef void (*helpfunc_t)(void);
 
 typedef struct cmdinfo {

@@ -41,7 +41,7 @@ typedef struct cmdinfo {
 
 extern bool qemuio_misalign;
 
-void qemuio_command(BlockBackend *blk, const char *cmd);
+int qemuio_command(BlockBackend *blk, const char *cmd);
 
 void qemuio_add_command(const cmdinfo_t *ci);
 void qemuio_command_usage(const cmdinfo_t *ci);
qemu-io-cmds.c +203 -143

1,189 lines not on this page · raw message

qemu-io.c +22 -12
index 4578ccfaa5..e82dd0b999 100644
@@ -66,10 +66,11 @@ static int get_eof_char(void)
 #endif
 }
 
-static void close_f(BlockBackend *blk, int argc, char **argv)
+static int close_f(BlockBackend *blk, int argc, char **argv)
 {
     blk_unref(qemuio_blk);
     qemuio_blk = NULL;
+    return 0;
 }
 
 static const cmdinfo_t close_cmd = {

@@ -136,7 +137,7 @@ static void open_help(void)
 "\n");
 }
 
-static void open_f(BlockBackend *blk, int argc, char **argv);
+static int open_f(BlockBackend *blk, int argc, char **argv);
 
 static const cmdinfo_t open_cmd = {
     .name       = "open",

@@ -160,12 +161,13 @@ static QemuOptsList empty_opts = {
     },
 };
 
-static void open_f(BlockBackend *blk, int argc, char **argv)
+static int open_f(BlockBackend *blk, int argc, char **argv)
 {
     int flags = BDRV_O_UNMAP;
     int readonly = 0;
     bool writethrough = true;
     int c;
+    int ret;
     QemuOpts *qopts;
     QDict *opts;
     bool force_share = false;

@@ -192,25 +194,25 @@ static void open_f(BlockBackend *blk, int argc, char **argv)
             if (bdrv_parse_cache_mode(optarg, &flags, &writethrough) < 0) {
                 error_report("Invalid cache option: %s", optarg);
                 qemu_opts_reset(&empty_opts);
-                return;
+                return -EINVAL;
             }
             break;
         case 'd':
             if (bdrv_parse_discard_flags(optarg, &flags) < 0) {
                 error_report("Invalid discard option: %s", optarg);
                 qemu_opts_reset(&empty_opts);
-                return;
+                return -EINVAL;
             }
             break;
         case 'o':
             if (imageOpts) {
                 printf("--image-opts and 'open -o' are mutually exclusive\n");
                 qemu_opts_reset(&empty_opts);
-                return;
+                return -EINVAL;
             }
             if (!qemu_opts_parse_noisily(&empty_opts, optarg, false)) {
                 qemu_opts_reset(&empty_opts);
-                return;
+                return -EINVAL;
             }
             break;
         case 'U':

@@ -219,7 +221,7 @@ static void open_f(BlockBackend *blk, int argc, char **argv)
         default:
             qemu_opts_reset(&empty_opts);
             qemuio_command_usage(&open_cmd);
-            return;
+            return -EINVAL;
         }
     }
 

@@ -230,7 +232,7 @@ static void open_f(BlockBackend *blk, int argc, char **argv)
     if (imageOpts && (optind == argc - 1)) {
         if (!qemu_opts_parse_noisily(&empty_opts, argv[optind], false)) {
             qemu_opts_reset(&empty_opts);
-            return;
+            return -EINVAL;
         }
         optind++;
     }

@@ -240,18 +242,26 @@ static void open_f(BlockBackend *blk, int argc, char **argv)
     qemu_opts_reset(&empty_opts);
 
     if (optind == argc - 1) {
-        openfile(argv[optind], flags, writethrough, force_share, opts);
+        ret = openfile(argv[optind], flags, writethrough, force_share, opts);
     } else if (optind == argc) {
-        openfile(NULL, flags, writethrough, force_share, opts);
+        ret = openfile(NULL, flags, writethrough, force_share, opts);
     } else {
         QDECREF(opts);
         qemuio_command_usage(&open_cmd);
+        return -EINVAL;
+    }
+
+    if (ret) {
+        return -EINVAL;
     }
+
+    return 0;
 }
 
-static void quit_f(BlockBackend *blk, int argc, char **argv)
+static int quit_f(BlockBackend *blk, int argc, char **argv)
 {
     quit_qemu_io = true;
+    return 0;
 }
 
 static const cmdinfo_t quit_cmd = {
Signature · 1 line
2.14.3
Reviewed-by on v1 2/5
#4 lore raw

Re: [Qemu-devel] [PATCH 2/5] qemu-io: Let command functions return error code

Quoted 15 lines · Max Reitz

This is basically what everything else in the qemu code base does, so we can do it here, too.

Signed-off-by Max Reitz <[email protected]>
---
  include/qemu-io.h |   4 +-
  qemu-io-cmds.c    | 346 ++++++++++++++++++++++++++++++++----------------------
  qemu-io.c         |  34 ++++--
  3 files changed, 227 insertions(+), 157 deletions(-)
include/qemu-io.h +0 -0
index 06cdfbf660..380724ad59 100644

include/qemu-io.h line 25

Quoted 5 lines
include/qemu-io.h +1 -1
@@ -22,7 +22,7 @@
 
  #define CMD_FLAG_GLOBAL ((int)0x80000000) /* don't iterate "args" */
 
-typedef void (*cfunc_t)(BlockBackend *blk, int argc, char **argv);
+typedef int (*cfunc_t)(BlockBackend *blk, int argc, char **argv);

Quite a bit of churn on the definition here; squashing 1 and 2 might make for a smaller patch. But I agree that doing it this way (2 separate patches) forces us to audit (with the compiler's help) that all callers are caught and adjusted to the new semantics.

However, a comment before the typedef would be useful, so that writing a new command knows what semantics it must provide. It can be as simple as:

/* Implement a qemu-io command.
* Operate on @blk using @argc/@argv as the command's arguments, and
* return 0 on success or negative errno on failure.
*/

With a comment added,

Reviewed-by Eric Blake <[email protected]>
Signature · 3 lines
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
author
#5 lore raw

Re: [Qemu-devel] [PATCH 2/5] qemu-io: Let command functions return error code

Quoted 26 lines · Eric Blake
Quoted 16 lines · Max Reitz

This is basically what everything else in the qemu code base does, so we can do it here, too.

Signed-off-by Max Reitz <[email protected]>

---
  include/qemu-io.h |   4 +-
  qemu-io-cmds.c    | 346
++++++++++++++++++++++++++++++++----------------------
  qemu-io.c         |  34 ++++--

  3 files changed, 227 insertions(+), 157 deletions(-)
include/qemu-io.h +0 -0
index 06cdfbf660..380724ad59 100644

include/qemu-io.h line 24

Quoted 3 lines
include/qemu-io.h +1 -0
@@ -22,7 +22,7 @@
     #define CMD_FLAG_GLOBAL ((int)0x80000000) /* don't iterate "args" */
   -typedef void (*cfunc_t)(BlockBackend *blk, int argc, char **argv);
+typedef int (*cfunc_t)(BlockBackend *blk, int argc, char **argv);

Quite a bit of churn on the definition here; squashing 1 and 2 might make for a smaller patch.  But I agree that doing it this way (2 separate patches) forces us to audit (with the compiler's help) that all callers are caught and adjusted to the new semantics.

Quoted 7 lines

However, a comment before the typedef would be useful, so that writing a new command knows what semantics it must provide.  It can be as simple as:

/* Implement a qemu-io command.
 * Operate on @blk using @argc/@argv as the command's arguments, and
 * return 0 on success or negative errno on failure.
 */

Yep, will do.

Quoted 2 lines

With a comment added,

Reviewed-by Eric Blake <[email protected]>

Thanks!

Max

author
#6 lore raw

PATCH 3/5 qemu-io: Exit with error when a command failed

In reply to Max Reitz

Currently, qemu-io basically always returns success when it gets to interactive mode (so once the whole command line has been parsed; even before the commands on the command line are interpreted). That is not very useful.

This patch makes qemu-io return failure when any of the executed commands failed.

Buglink https://bugzilla.redhat.com/show_bug.cgi?id=1519617
Signed-off-by Max Reitz <[email protected]>
---
 qemu-io.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)
qemu-io.c +22 -6
index e82dd0b999..c95886d22a 100644
@@ -400,17 +400,21 @@ static void prep_fetchline(void *opaque)
     *fetchable= 1;
 }
 
-static void command_loop(void)
+static int command_loop(void)
 {
     int i, fetchable = 0, prompted = 0;
+    int ret, last_error = 0;
     char *input;
 
     for (i = 0; !quit_qemu_io && i < ncmdline; i++) {
-        qemuio_command(qemuio_blk, cmdline[i]);
+        ret = qemuio_command(qemuio_blk, cmdline[i]);
+        if (ret < 0) {
+            last_error = ret;
+        }
     }
     if (cmdline) {
         g_free(cmdline);
-        return;
+        return last_error;
     }
 
     while (!quit_qemu_io) {

@@ -431,13 +435,19 @@ static void command_loop(void)
         if (input == NULL) {
             break;
         }
-        qemuio_command(qemuio_blk, input);
+        ret = qemuio_command(qemuio_blk, input);
         g_free(input);
 
+        if (ret < 0) {
+            last_error = ret;
+        }
+
         prompted = 0;
         fetchable = 0;
     }
     qemu_set_fd_handler(STDIN_FILENO, NULL, NULL, NULL);
+
+    return last_error;
 }
 
 static void add_user_command(char *optarg)

@@ -502,6 +512,7 @@ int main(int argc, char **argv)
     int c;
     int opt_index = 0;
     int flags = BDRV_O_UNMAP;
+    int ret;
     bool writethrough = true;
     Error *local_error = NULL;
     QDict *opts = NULL;

@@ -663,7 +674,7 @@ int main(int argc, char **argv)
             }
         }
     }
-    command_loop();
+    ret = command_loop();
 
     /*
      * Make sure all outstanding requests complete before the program exits.

@@ -672,5 +683,10 @@ int main(int argc, char **argv)
 
     blk_unref(qemuio_blk);
     g_free(readline_state);
-    return 0;
+
+    if (ret < 0) {
+        return 1;
+    } else {
+        return 0;
+    }
 }
Signature · 1 line
2.14.3
Reviewed-by on v1 3/5
#7 lore raw
Quoted 16 lines · Max Reitz

Currently, qemu-io basically always returns success when it gets to interactive mode (so once the whole command line has been parsed; even before the commands on the command line are interpreted). That is not very useful.

This patch makes qemu-io return failure when any of the executed commands failed.

Buglink https://bugzilla.redhat.com/show_bug.cgi?id=1519617
Signed-off-by Max Reitz <[email protected]>
---
  qemu-io.c | 28 ++++++++++++++++++++++------
  1 file changed, 22 insertions(+), 6 deletions(-)

qemu-io.c int main(int argc, char **argv) · line 691

Quoted 10 lines
qemu-io.c +6 -1
@@ -672,5 +683,10 @@ int main(int argc, char **argv)
 
      blk_unref(qemuio_blk);
      g_free(readline_state);
-    return 0;
+
+    if (ret < 0) {
+        return 1;
+    } else {
+        return 0;
+    }

Would spelling this EXIT_SUCCESS/EXIT_FAILURE make it any more legible? But we aren't consistently using those anywhere else, so you aren't making it worse by using 0/1.

Reviewed-by Eric Blake <[email protected]>
Signature · 3 lines
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
author
#8 lore raw

PATCH 4/5 iotests.py: Add qemu_io_silent

In reply to Max Reitz

With qemu-io now returning a useful exit code, some tests may find it sufficient to just query that instead of logging (and filtering) the whole output.

Signed-off-by Max Reitz <[email protected]>
---
 tests/qemu-iotests/iotests.py | 9 +++++++++
 1 file changed, 9 insertions(+)
tests/qemu-iotests/iotests.py +9 -0
index b25d48a91b..9747ca9eba 100644
@@ -119,6 +119,15 @@ def qemu_io(*args):
         sys.stderr.write('qemu-io received signal %i: %s\n' % (-exitcode, ' '.join(args)))
     return subp.communicate()[0]
 
+def qemu_io_silent(*args):
+    '''Run qemu-io and return the exit code, suppressing stdout'''
+    args = qemu_io_args + list(args)
+    exitcode = subprocess.call(args, stdout=open('/dev/null', 'w'))
+    if exitcode < 0:
+        sys.stderr.write('qemu-io received signal %i: %s\n' %
+                         (-exitcode, ' '.join(args)))
+    return exitcode
+
 
 class QemuIoInteractive:
     def __init__(self, *args):
Signature · 1 line
2.14.3
Reviewed-by on v1 4/5
#9 lore raw

Re: [Qemu-devel] [PATCH 4/5] iotests.py: Add qemu_io_silent

Quoted 8 lines · Max Reitz

With qemu-io now returning a useful exit code, some tests may find it sufficient to just query that instead of logging (and filtering) the whole output.

Signed-off-by Max Reitz <[email protected]>
---
  tests/qemu-iotests/iotests.py | 9 +++++++++
  1 file changed, 9 insertions(+)
Reviewed-by Eric Blake <[email protected]>
Signature · 3 lines
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
author
#10 lore raw

PATCH 5/5 iotests: Let 216 make use of qemu-io's exit code

In reply to Max Reitz

As a showcase of how you can use qemu-io's exit code to determine success or failure (same for qemu-img), this test is changed to use qemu_io_silent() instead of qemu_io(), and to assert the exit code instead of logging the filtered result.

One real advantage of this is that in case of an error, you get a backtrace that helps you locate the issue in the test file quickly.

Signed-off-by Max Reitz <[email protected]>
---
 tests/qemu-iotests/216     | 23 ++++++++++++-----------
 tests/qemu-iotests/216.out | 17 ++---------------
 2 files changed, 14 insertions(+), 26 deletions(-)
tests/qemu-iotests/216 +12 -11
index b362cf93a8..072707449d 100755
@@ -22,7 +22,7 @@
 # Non-shared storage migration test using NBD server and drive-mirror
 
 import iotests
-from iotests import log, qemu_img_pipe, qemu_io, filter_qemu_io
+from iotests import log, qemu_img, qemu_io_silent
 
 # Need backing file support
 iotests.verify_image_format(supported_fmts=['qcow2', 'qcow', 'qed', 'vmdk'])

@@ -52,14 +52,13 @@ with iotests.FilePath('base.img') as base_img_path, \
     log('--- Setting up images ---')
     log('')
 
-    qemu_img_pipe('create', '-f', iotests.imgfmt, base_img_path, '64M')
+    assert qemu_img('create', '-f', iotests.imgfmt, base_img_path, '64M') == 0
+    assert qemu_io_silent(base_img_path, '-c', 'write -P 1 0M 1M') == 0
+    assert qemu_img('create', '-f', iotests.imgfmt, '-b', base_img_path,
+                    top_img_path) == 0
+    assert qemu_io_silent(top_img_path,  '-c', 'write -P 2 1M 1M') == 0
 
-    log(filter_qemu_io(qemu_io(base_img_path, '-c', 'write -P 1 0M 1M')))
-
-    qemu_img_pipe('create', '-f', iotests.imgfmt, '-b', base_img_path,
-                  top_img_path)
-
-    log(filter_qemu_io(qemu_io(top_img_path,  '-c', 'write -P 2 1M 1M')))
+    log('Done')
 
     log('')
     log('--- Doing COR ---')

@@ -112,6 +111,8 @@ with iotests.FilePath('base.img') as base_img_path, \
     log('--- Checking COR result ---')
     log('')
 
-    log(filter_qemu_io(qemu_io(base_img_path, '-c', 'discard 0 64M')))
-    log(filter_qemu_io(qemu_io(top_img_path,  '-c', 'read -P 1 0M 1M')))
-    log(filter_qemu_io(qemu_io(top_img_path,  '-c', 'read -P 2 1M 1M')))
+    assert qemu_io_silent(base_img_path, '-c', 'discard 0 64M') == 0
+    assert qemu_io_silent(top_img_path,  '-c', 'read -P 1 0M 1M') == 0
+    assert qemu_io_silent(top_img_path,  '-c', 'read -P 2 1M 1M') == 0
+
+    log('Done')
tests/qemu-iotests/216.out +2 -15
index d3fc590d29..45ea857ee1 100644
@@ -3,12 +3,7 @@
 
 --- Setting up images ---
 
-wrote 1048576/1048576 bytes at offset 0
-1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-wrote 1048576/1048576 bytes at offset 1048576
-1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
+Done
 
 --- Doing COR ---
 

@@ -17,12 +12,4 @@ wrote 1048576/1048576 bytes at offset 1048576
 
 --- Checking COR result ---
 
-discard 67108864/67108864 bytes at offset 0
-64 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-read 1048576/1048576 bytes at offset 0
-1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-read 1048576/1048576 bytes at offset 1048576
-1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
+Done
Signature · 1 line
2.14.3
Reviewed-by on v1 5/5
#11 lore raw

Re: [Qemu-devel] [PATCH 5/5] iotests: Let 216 make use of qemu-io's exit code

Quoted 14 lines · Max Reitz

As a showcase of how you can use qemu-io's exit code to determine success or failure (same for qemu-img), this test is changed to use qemu_io_silent() instead of qemu_io(), and to assert the exit code instead of logging the filtered result.

One real advantage of this is that in case of an error, you get a backtrace that helps you locate the issue in the test file quickly.

Signed-off-by Max Reitz <[email protected]>
---
  tests/qemu-iotests/216     | 23 ++++++++++++-----------
  tests/qemu-iotests/216.out | 17 ++---------------
  2 files changed, 14 insertions(+), 26 deletions(-)

tests/qemu-iotests/216 with iotests.FilePath('base.img') as base_img_path, \ · line 55

Quoted 2 lines
tests/qemu-iotests/216 +1 -1
-    qemu_img_pipe('create', '-f', iotests.imgfmt, base_img_path, '64M')
+    assert qemu_img('create', '-f', iotests.imgfmt, base_img_path, '64M') == 0

Yeah, I can see how writing the setup for a test gets simpler when you don't have to worry about logging and filtering the stuff that isn't the heart of the test.

Reviewed-by Eric Blake <[email protected]>
Signature · 3 lines
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org