[Qemu-devel] [PATCH v7 0/4] Add subcommand compare for qemu-img

[Qemu-devel] [PATCH v7 0/4] Add subcommand compare for qemu-img

[mrezanin]

From: Miroslav Rezanina <mrezanin@redhat.com>

This is seventh version of  patch adding compare subcommand that
compares two images. Compare has following criteria:
 - only data part is compared
 - unallocated sectors are not read
 - in case of different image size, exceeding part of bigger disk has
 to be zeroed/unallocated to compare rest
 - qemu-img returns:
    - 0 if images are identical
    - 1 if images differ
    - 2 on error


v7:
 - split patch into pieces
 - Quiet mode added for all relevant subcommands
 - check non-shared part of disk after shared one
 - minor docummentation and naming fixes

v6:
 - added handling -?, -h options for compare subcommand

v5 (only minor changes):
 - removed redundant comment
 - removed dead code (goto after help())
 - set final total_sectors on first assignment

v4:
 - Fixed various typos
 - Added functions for empty sector check and sector-to-bytes offset
 conversion
 - Fixed command-line parameters processing

v3:
 - options -f/-F are orthogonal
 - documentation updated to new syntax and behavior
 - used byte offset instead of sector number for output
 
v2:
 - changed option for second image format to -F
 - changed handling of -f and -F [1]
 - added strict mode (-s)
 - added quiet mode (-q)
 - improved output messages [2]
 - rename variables for larger image handling
 - added man page content

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>

Miroslav Rezanina (4):
  block: Add synchronous wrapper for bdrv_co_is_allocated_above
  qemu-img: Add "Quiet mode" option
  qemu-img: Add compare subcommand
  Add qemu-img compare documentation

 block.c          |  50 +++++++-
 block.h          |   4 +-
 blockdev.c       |   6 +-
 qemu-img-cmds.hx |  34 +++--
 qemu-img.c       | 383 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
 qemu-img.texi    |  35 +++++
 6 files changed, 460 insertions(+), 52 deletions(-)

-- 
1.7.11.7

[Qemu-devel] [PATCH v7 1/4] block: Add synchronous wrapper for bdrv_co_is_allocated_above

[mrezanin]

From: Miroslav Rezanina <mrezanin@redhat.com>

There's no synchronous wrapper for bdrv_co_is_allocated_above function
so it's not possible to check for sector allocation in image with
backing file.

This patch add missing synchronous wrapper.

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
---
 block.c | 39 +++++++++++++++++++++++++++++++++++++++
 block.h |  2 ++
 2 files changed, 41 insertions(+)

diff --git a/block.c b/block.c
index c05875f..24c06ab 100644
--- a/block.c
+++ b/block.c
@@ -2699,6 +2699,7 @@ int bdrv_has_zero_init(BlockDriverState *bs)
 
 typedef struct BdrvCoIsAllocatedData {
     BlockDriverState *bs;
+    BlockDriverState *base;
     int64_t sector_num;
     int nb_sectors;
     int *pnum;
@@ -2829,6 +2830,44 @@ int coroutine_fn bdrv_co_is_allocated_above(BlockDriverState *top,
     return 0;
 }
 
+/* Coroutine wrapper for bdrv_is_allocated_above() */
+static void coroutine_fn bdrv_is_allocated_above_co_entry(void *opaque)
+{
+    BdrvCoIsAllocatedData *data = opaque;
+    BlockDriverState *top = data->bs;
+    BlockDriverState *base = data->base;
+
+    data->ret = bdrv_co_is_allocated_above(top, base, data->sector_num,
+                                           data->nb_sectors, data->pnum);
+    data->done = true;
+}
+
+/*
+ * Synchronous wrapper around bdrv_co_is_allocated_above().
+ *
+ * See bdrv_co_is_allocated_above() for details.
+ */
+int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
+                      int64_t sector_num, int nb_sectors, int *pnum)
+{
+    Coroutine *co;
+    BdrvCoIsAllocatedData data = {
+        .bs = top,
+        .base = base,
+        .sector_num = sector_num,
+        .nb_sectors = nb_sectors,
+        .pnum = pnum,
+        .done = false,
+    };
+
+    co = qemu_coroutine_create(bdrv_is_allocated_above_co_entry);
+    qemu_coroutine_enter(co, &data);
+    while (!data.done) {
+        qemu_aio_wait();
+    }
+    return data.ret;
+}
+
 BlockInfo *bdrv_query_info(BlockDriverState *bs)
 {
     BlockInfo *info = g_malloc0(sizeof(*info));
diff --git a/block.h b/block.h
index 722c620..2cb8d71 100644
--- a/block.h
+++ b/block.h
@@ -278,6 +278,8 @@ int bdrv_co_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors);
 int bdrv_has_zero_init(BlockDriverState *bs);
 int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
                       int *pnum);
+int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
+                            int64_t sector_num, int nb_sectors, int *pnum);
 
 void bdrv_set_on_error(BlockDriverState *bs, BlockdevOnError on_read_error,
                        BlockdevOnError on_write_error);
-- 
1.7.11.7

[Qemu-devel] [PATCH v7 2/4] qemu-img: Add "Quiet mode" option

[mrezanin]

From: Miroslav Rezanina <mrezanin@redhat.com>

There can be need to turn output to stdout off. This patch adds a -q option that
enable "Quiet mode". In Quiet mode, only errors are printed out.

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
---
 block.c          |  11 +++---
 block.h          |   2 +-
 blockdev.c       |   6 ++--
 qemu-img-cmds.hx |  28 +++++++--------
 qemu-img.c       | 108 +++++++++++++++++++++++++++++++++++++++++--------------
 qemu-img.texi    |   3 ++
 6 files changed, 108 insertions(+), 50 deletions(-)

diff --git a/block.c b/block.c
index 24c06ab..5450ff9 100644
--- a/block.c
+++ b/block.c
@@ -4449,7 +4449,7 @@ bdrv_acct_done(BlockDriverState *bs, BlockAcctCookie *cookie)
 
 int bdrv_img_create(const char *filename, const char *fmt,
                     const char *base_filename, const char *base_fmt,
-                    char *options, uint64_t img_size, int flags)
+                    char *options, uint64_t img_size, int flags, bool quiet)
 {
     QEMUOptionParameter *param = NULL, *create_options = NULL;
     QEMUOptionParameter *backing_fmt, *backing_file, *size;
@@ -4565,10 +4565,11 @@ int bdrv_img_create(const char *filename, const char *fmt,
         }
     }
 
-    printf("Formatting '%s', fmt=%s ", filename, fmt);
-    print_option_parameters(param);
-    puts("");
-
+    if (!quiet) {
+        printf("Formatting '%s', fmt=%s ", filename, fmt);
+        print_option_parameters(param);
+        puts("");
+    }
     ret = bdrv_create(drv, filename, param);
 
     if (ret < 0) {
diff --git a/block.h b/block.h
index 2cb8d71..a7e7220 100644
--- a/block.h
+++ b/block.h
@@ -347,7 +347,7 @@ int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf,
 
 int bdrv_img_create(const char *filename, const char *fmt,
                     const char *base_filename, const char *base_fmt,
-                    char *options, uint64_t img_size, int flags);
+                    char *options, uint64_t img_size, int flags, bool quiet);
 
 void bdrv_set_buffer_alignment(BlockDriverState *bs, int align);
 void *qemu_blockalign(BlockDriverState *bs, size_t size);
diff --git a/blockdev.c b/blockdev.c
index e73fd6e..b2fb7f7 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -789,7 +789,7 @@ void qmp_transaction(BlockdevActionList *dev_list, Error **errp)
             ret = bdrv_img_create(new_image_file, format,
                                   states->old_bs->filename,
                                   states->old_bs->drv->format_name,
-                                  NULL, -1, flags);
+                                  NULL, -1, flags, false);
             if (ret) {
                 error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file);
                 goto delete_and_fail;
@@ -1264,7 +1264,7 @@ void qmp_drive_mirror(const char *device, const char *target,
         bdrv_get_geometry(bs, &size);
         size *= 512;
         ret = bdrv_img_create(target, format,
-                              NULL, NULL, NULL, size, flags);
+                              NULL, NULL, NULL, size, flags, false);
     } else {
         switch (mode) {
         case NEW_IMAGE_MODE_EXISTING:
@@ -1275,7 +1275,7 @@ void qmp_drive_mirror(const char *device, const char *target,
             ret = bdrv_img_create(target, format,
                                   source->filename,
                                   source->drv->format_name,
-                                  NULL, -1, flags);
+                                  NULL, -1, flags,false);
             break;
         default:
             abort();
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index a181363..90b93e0 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -10,27 +10,27 @@ STEXI
 ETEXI
 
 DEF("check", img_check,
-    "check [-f fmt] [-r [leaks | all]] filename")
+    "check [-q] [-f fmt] [-r [leaks | all]] filename")
 STEXI
-@item check [-f @var{fmt}] [-r [leaks | all]] @var{filename}
+@item check [-q] [-f @var{fmt}] [-r [leaks | all]] @var{filename}
 ETEXI
 
 DEF("create", img_create,
-    "create [-f fmt] [-o options] filename [size]")
+    "create [-q] [-f fmt] [-o options] filename [size]")
 STEXI
-@item create [-f @var{fmt}] [-o @var{options}] @var{filename} [@var{size}]
+@item create [-q] [-f @var{fmt}] [-o @var{options}] @var{filename} [@var{size}]
 ETEXI
 
 DEF("commit", img_commit,
-    "commit [-f fmt] [-t cache] filename")
+    "commit [-q] [-f fmt] [-t cache] filename")
 STEXI
-@item commit [-f @var{fmt}] [-t @var{cache}] @var{filename}
+@item commit [-q] [-f @var{fmt}] [-t @var{cache}] @var{filename}
 ETEXI
 
 DEF("convert", img_convert,
-    "convert [-c] [-p] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s snapshot_name] [-S sparse_size] filename [filename2 [...]] output_filename")
+    "convert [-c] [-p] [-q] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s snapshot_name] [-S sparse_size] filename [filename2 [...]] output_filename")
 STEXI
-@item convert [-c] [-p] [-f @var{fmt}] [-t @var{cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
+@item convert [-c] [-p] [-q] [-f @var{fmt}] [-t @var{cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
 ETEXI
 
 DEF("info", img_info,
@@ -40,20 +40,20 @@ STEXI
 ETEXI
 
 DEF("snapshot", img_snapshot,
-    "snapshot [-l | -a snapshot | -c snapshot | -d snapshot] filename")
+    "snapshot [-q] [-l | -a snapshot | -c snapshot | -d snapshot] filename")
 STEXI
-@item snapshot [-l | -a @var{snapshot} | -c @var{snapshot} | -d @var{snapshot}] @var{filename}
+@item snapshot [-q] [-l | -a @var{snapshot} | -c @var{snapshot} | -d @var{snapshot}] @var{filename}
 ETEXI
 
 DEF("rebase", img_rebase,
-    "rebase [-f fmt] [-t cache] [-p] [-u] -b backing_file [-F backing_fmt] filename")
+    "rebase [-q] [-f fmt] [-t cache] [-p] [-u] -b backing_file [-F backing_fmt] filename")
 STEXI
-@item rebase [-f @var{fmt}] [-t @var{cache}] [-p] [-u] -b @var{backing_file} [-F @var{backing_fmt}] @var{filename}
+@item rebase [-q] [-f @var{fmt}] [-t @var{cache}] [-p] [-u] -b @var{backing_file} [-F @var{backing_fmt}] @var{filename}
 ETEXI
 
 DEF("resize", img_resize,
-    "resize filename [+ | -]size")
+    "resize [-q] filename [+ | -]size")
 STEXI
-@item resize @var{filename} [+ | -]@var{size}
+@item resize [-q] @var{filename} [+ | -]@var{size}
 @end table
 ETEXI
diff --git a/qemu-img.c b/qemu-img.c
index e29e01b..b9a45f1 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -32,6 +32,7 @@
 #include "block_int.h"
 #include <getopt.h>
 #include <stdio.h>
+#include <stdarg.h>
 
 #ifdef _WIN32
 #include <windows.h>
@@ -86,6 +87,7 @@ static void help(void)
            "       rebasing in this case (useful for renaming the backing file)\n"
            "  '-h' with or without a command shows this help and lists the supported formats\n"
            "  '-p' show progress of command (only certain commands)\n"
+           "  '-q' Quiet mode - do not print any output (except errors)\n"
            "  '-S' indicates the consecutive number of bytes that must contain only zeros\n"
            "       for qemu-img to create a sparse image during conversion\n"
            "  '--output' takes the format in which the output must be done (human or json)\n"
@@ -109,6 +111,18 @@ static void help(void)
     exit(1);
 }
 
+static int qprintf(bool quiet, const char* fmt, ...)
+{
+    int ret = 0;
+    if (!quiet) {
+        va_list args;
+        va_start(args, fmt);
+        ret = vprintf(fmt, args);
+        va_end(args);
+    }
+    return ret;
+}
+
 #if defined(WIN32)
 /* XXX: put correct support for win32 */
 static int read_password(char *buf, int buf_size)
@@ -227,7 +241,8 @@ static int print_block_option_help(const char *filename, const char *fmt)
 static BlockDriverState *bdrv_new_open(const char *filename,
                                        const char *fmt,
                                        int flags,
-                                       bool require_io)
+                                       bool require_io, 
+                                       bool quiet)
 {
     BlockDriverState *bs;
     BlockDriver *drv;
@@ -253,7 +268,7 @@ static BlockDriverState *bdrv_new_open(const char *filename,
     }
 
     if (bdrv_is_encrypted(bs) && require_io) {
-        printf("Disk image '%s' is encrypted.\n", filename);
+        qprintf(quiet, "Disk image '%s' is encrypted.\n", filename);
         if (read_password(password, sizeof(password)) < 0) {
             error_report("No password given");
             goto fail;
@@ -301,9 +316,10 @@ static int img_create(int argc, char **argv)
     const char *filename;
     const char *base_filename = NULL;
     char *options = NULL;
+    bool quiet = false;
 
     for(;;) {
-        c = getopt(argc, argv, "F:b:f:he6o:");
+        c = getopt(argc, argv, "F:b:f:he6o:q");
         if (c == -1) {
             break;
         }
@@ -332,6 +348,9 @@ static int img_create(int argc, char **argv)
         case 'o':
             options = optarg;
             break;
+        case 'q':
+            quiet = true;
+            break;
         }
     }
 
@@ -362,7 +381,7 @@ static int img_create(int argc, char **argv)
     }
 
     ret = bdrv_img_create(filename, fmt, base_filename, base_fmt,
-                          options, img_size, BDRV_O_FLAGS);
+                          options, img_size, BDRV_O_FLAGS, quiet);
 out:
     if (ret) {
         return 1;
@@ -386,10 +405,11 @@ static int img_check(int argc, char **argv)
     BdrvCheckResult result;
     int fix = 0;
     int flags = BDRV_O_FLAGS | BDRV_O_CHECK;
+    bool quiet = false;
 
     fmt = NULL;
     for(;;) {
-        c = getopt(argc, argv, "f:hr:");
+        c = getopt(argc, argv, "f:hr:q");
         if (c == -1) {
             break;
         }
@@ -412,6 +432,9 @@ static int img_check(int argc, char **argv)
                 help();
             }
             break;
+        case 'q':
+            quiet = true;
+            break;
         }
     }
     if (optind >= argc) {
@@ -419,7 +442,7 @@ static int img_check(int argc, char **argv)
     }
     filename = argv[optind++];
 
-    bs = bdrv_new_open(filename, fmt, flags, true);
+    bs = bdrv_new_open(filename, fmt, flags, true, quiet);
     if (!bs) {
         return 1;
     }
@@ -432,7 +455,8 @@ static int img_check(int argc, char **argv)
     }
 
     if (result.corruptions_fixed || result.leaks_fixed) {
-        printf("The following inconsistencies were found and repaired:\n\n"
+        qprintf(quiet, 
+               "The following inconsistencies were found and repaired:\n\n"
                "    %d leaked clusters\n"
                "    %d corruptions\n\n"
                "Double checking the fixed image now...\n",
@@ -442,29 +466,31 @@ static int img_check(int argc, char **argv)
     }
 
     if (!(result.corruptions || result.leaks || result.check_errors)) {
-        printf("No errors were found on the image.\n");
+        qprintf(quiet, "No errors were found on the image.\n");
     } else {
         if (result.corruptions) {
-            printf("\n%d errors were found on the image.\n"
+            qprintf(quiet, "\n%d errors were found on the image.\n"
                 "Data may be corrupted, or further writes to the image "
                 "may corrupt it.\n",
                 result.corruptions);
         }
 
         if (result.leaks) {
-            printf("\n%d leaked clusters were found on the image.\n"
+            qprintf(quiet, "\n%d leaked clusters were found on the image.\n"
                 "This means waste of disk space, but no harm to data.\n",
                 result.leaks);
         }
 
         if (result.check_errors) {
-            printf("\n%d internal errors have occurred during the check.\n",
+            qprintf(quiet, 
+                    "\n%d internal errors have occurred during the check.\n",
                 result.check_errors);
         }
     }
 
     if (result.bfi.total_clusters != 0 && result.bfi.allocated_clusters != 0) {
-        printf("%" PRId64 "/%" PRId64 "= %0.2f%% allocated, %0.2f%% fragmented\n",
+        qprintf(quiet, 
+           "%" PRId64 "/%" PRId64 "= %0.2f%% allocated, %0.2f%% fragmented\n",
         result.bfi.allocated_clusters, result.bfi.total_clusters,
         result.bfi.allocated_clusters * 100.0 / result.bfi.total_clusters,
         result.bfi.fragmented_clusters * 100.0 / result.bfi.allocated_clusters);
@@ -473,7 +499,7 @@ static int img_check(int argc, char **argv)
     bdrv_delete(bs);
 
     if (ret < 0 || result.check_errors) {
-        printf("\nAn error has occurred during the check: %s\n"
+        qprintf(quiet, "\nAn error has occurred during the check: %s\n"
             "The check is not complete and may have missed error.\n",
             strerror(-ret));
         return 1;
@@ -493,11 +519,12 @@ static int img_commit(int argc, char **argv)
     int c, ret, flags;
     const char *filename, *fmt, *cache;
     BlockDriverState *bs;
+    bool quiet = false;
 
     fmt = NULL;
     cache = BDRV_DEFAULT_CACHE;
     for(;;) {
-        c = getopt(argc, argv, "f:ht:");
+        c = getopt(argc, argv, "f:ht:q");
         if (c == -1) {
             break;
         }
@@ -512,6 +539,9 @@ static int img_commit(int argc, char **argv)
         case 't':
             cache = optarg;
             break;
+        case 'q':
+            quiet = true;
+            break;
         }
     }
     if (optind >= argc) {
@@ -526,14 +556,14 @@ static int img_commit(int argc, char **argv)
         return -1;
     }
 
-    bs = bdrv_new_open(filename, fmt, flags, true);
+    bs = bdrv_new_open(filename, fmt, flags, true, quiet);
     if (!bs) {
         return 1;
     }
     ret = bdrv_commit(bs);
     switch(ret) {
     case 0:
-        printf("Image committed.\n");
+        qprintf(quiet, "Image committed.\n");
         break;
     case -ENOENT:
         error_report("No disk inserted");
@@ -676,6 +706,7 @@ static int img_convert(int argc, char **argv)
     const char *snapshot_name = NULL;
     float local_progress = 0;
     int min_sparse = 8; /* Need at least 4k of zeros for sparse detection */
+    bool quiet = false; 
 
     fmt = NULL;
     out_fmt = "raw";
@@ -683,7 +714,7 @@ static int img_convert(int argc, char **argv)
     out_baseimg = NULL;
     compress = 0;
     for(;;) {
-        c = getopt(argc, argv, "f:O:B:s:hce6o:pS:t:");
+        c = getopt(argc, argv, "f:O:B:s:hce6o:pS:t:q");
         if (c == -1) {
             break;
         }
@@ -737,9 +768,16 @@ static int img_convert(int argc, char **argv)
         case 't':
             cache = optarg;
             break;
+        case 'q':
+            quiet = true;
+            break;
         }
     }
 
+    if (quiet) {
+        progress = 0;
+    }
+
     bs_n = argc - optind - 1;
     if (bs_n < 1) {
         help();
@@ -768,7 +806,7 @@ static int img_convert(int argc, char **argv)
 
     total_sectors = 0;
     for (bs_i = 0; bs_i < bs_n; bs_i++) {
-        bs[bs_i] = bdrv_new_open(argv[optind + bs_i], fmt, BDRV_O_FLAGS, true);
+        bs[bs_i] = bdrv_new_open(argv[optind + bs_i], fmt, BDRV_O_FLAGS, true, quiet);
         if (!bs[bs_i]) {
             error_report("Could not open '%s'", argv[optind + bs_i]);
             ret = -1;
@@ -887,7 +925,7 @@ static int img_convert(int argc, char **argv)
         return -1;
     }
 
-    out_bs = bdrv_new_open(out_filename, out_fmt, flags, true);
+    out_bs = bdrv_new_open(out_filename, out_fmt, flags, true, quiet);
     if (!out_bs) {
         ret = -1;
         goto out;
@@ -1350,7 +1388,7 @@ static ImageInfoList *collect_image_info_list(const char *filename,
         g_hash_table_insert(filenames, (gpointer)filename, NULL);
 
         bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING,
-                           false);
+                           false, false);
         if (!bs) {
             goto err;
         }
@@ -1486,11 +1524,12 @@ static int img_snapshot(int argc, char **argv)
     int c, ret = 0, bdrv_oflags;
     int action = 0;
     qemu_timeval tv;
+    bool quiet = false;
 
     bdrv_oflags = BDRV_O_FLAGS | BDRV_O_RDWR;
     /* Parse commandline parameters */
     for(;;) {
-        c = getopt(argc, argv, "la:c:d:h");
+        c = getopt(argc, argv, "la:c:d:hq");
         if (c == -1) {
             break;
         }
@@ -1531,6 +1570,9 @@ static int img_snapshot(int argc, char **argv)
             action = SNAPSHOT_DELETE;
             snapshot_name = optarg;
             break;
+        case 'q':
+            quiet = true;
+            break;
         }
     }
 
@@ -1540,7 +1582,7 @@ static int img_snapshot(int argc, char **argv)
     filename = argv[optind++];
 
     /* Open the image */
-    bs = bdrv_new_open(filename, NULL, bdrv_oflags, true);
+    bs = bdrv_new_open(filename, NULL, bdrv_oflags, true, quiet);
     if (!bs) {
         return 1;
     }
@@ -1600,6 +1642,7 @@ static int img_rebase(int argc, char **argv)
     int c, flags, ret;
     int unsafe = 0;
     int progress = 0;
+    bool quiet = false;
 
     /* Parse commandline parameters */
     fmt = NULL;
@@ -1607,7 +1650,7 @@ static int img_rebase(int argc, char **argv)
     out_baseimg = NULL;
     out_basefmt = NULL;
     for(;;) {
-        c = getopt(argc, argv, "uhf:F:b:pt:");
+        c = getopt(argc, argv, "uhf:F:b:pt:q");
         if (c == -1) {
             break;
         }
@@ -1634,9 +1677,16 @@ static int img_rebase(int argc, char **argv)
         case 't':
             cache = optarg;
             break;
+        case 'q':
+            quiet = true;
+            break;
         }
     }
 
+    if (quiet) {
+        progress = 0;
+    }
+
     if ((optind >= argc) || (!unsafe && !out_baseimg)) {
         help();
     }
@@ -1658,7 +1708,7 @@ static int img_rebase(int argc, char **argv)
      * Ignore the old backing file for unsafe rebase in case we want to correct
      * the reference to a renamed or moved backing file.
      */
-    bs = bdrv_new_open(filename, fmt, flags, true);
+    bs = bdrv_new_open(filename, fmt, flags, true, quiet);
     if (!bs) {
         return 1;
     }
@@ -1870,6 +1920,7 @@ static int img_resize(int argc, char **argv)
     int c, ret, relative;
     const char *filename, *fmt, *size;
     int64_t n, total_size;
+    bool quiet = false;
     BlockDriverState *bs = NULL;
     QemuOpts *param;
     static QemuOptsList resize_options = {
@@ -1898,7 +1949,7 @@ static int img_resize(int argc, char **argv)
     /* Parse getopt arguments */
     fmt = NULL;
     for(;;) {
-        c = getopt(argc, argv, "f:h");
+        c = getopt(argc, argv, "f:hq");
         if (c == -1) {
             break;
         }
@@ -1910,6 +1961,9 @@ static int img_resize(int argc, char **argv)
         case 'f':
             fmt = optarg;
             break;
+        case 'q':
+            quiet = true;
+            break;
         }
     }
     if (optind >= argc) {
@@ -1943,7 +1997,7 @@ static int img_resize(int argc, char **argv)
     n = qemu_opt_get_size(param, BLOCK_OPT_SIZE, 0);
     qemu_opts_del(param);
 
-    bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR, true);
+    bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR, true, quiet);
     if (!bs) {
         ret = -1;
         goto out;
@@ -1963,7 +2017,7 @@ static int img_resize(int argc, char **argv)
     ret = bdrv_truncate(bs, total_size);
     switch (ret) {
     case 0:
-        printf("Image resized.\n");
+        qprintf(quiet, "Image resized.\n");
         break;
     case -ENOTSUP:
         error_report("This image does not support resize");
diff --git a/qemu-img.texi b/qemu-img.texi
index 00fca8d..bb82a3d 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -54,6 +54,9 @@ indicates that target image must be compressed (qcow format only)
 with or without a command shows help and lists the supported formats
 @item -p
 display progress bar (convert and rebase commands only)
+@item -q
+Quiet mode - do not print any output (except errors). There's no progres bar 
+in case both @var{-q} and  @var{-p} options are used.
 @item -S @var{size}
 indicates the consecutive number of bytes that must contain only zeros
 for qemu-img to create a sparse image during conversion. This value is rounded
-- 
1.7.11.7

[Qemu-devel] [PATCH v7 3/4] qemu-img: Add compare subcommand

[mrezanin]

From: Miroslav Rezanina <mrezanin@redhat.com>

This patch adds new qemu-img subcommand that compare content of two disk
images.

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
---
 qemu-img-cmds.hx |   6 ++
 qemu-img.c       | 268 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 273 insertions(+), 1 deletion(-)

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 90b93e0..bc1ccfa 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -27,6 +27,12 @@ STEXI
 @item commit [-q] [-f @var{fmt}] [-t @var{cache}] @var{filename}
 ETEXI
 
+DEF("compare", img_compare,
+    "compare [-f fmt] [-F fmt] [-p] [-q] [-s] filename1 filename2")
+STEXI
+@item compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-q] [-s] @var{filename1} @var{filename2}
+ETEXI
+
 DEF("convert", img_convert,
     "convert [-c] [-p] [-q] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s snapshot_name] [-S sparse_size] filename [filename2 [...]] output_filename")
 STEXI
diff --git a/qemu-img.c b/qemu-img.c
index b9a45f1..8b4f01f 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -587,7 +587,7 @@ static int img_commit(int argc, char **argv)
 }
 
 /*
- * Returns true iff the first sector pointed to by 'buf' contains at least
+ * Returns true if the first sector pointed to by 'buf' contains at least
  * a non-NUL byte.
  *
  * 'pnum' is set to the number of sectors (including and immediately following
@@ -688,6 +688,272 @@ static int compare_sectors(const uint8_t *buf1, const uint8_t *buf2, int n,
 
 #define IO_BUF_SIZE (2 * 1024 * 1024)
 
+static int64_t sectors_to_bytes(int64_t sectors)
+{
+    return sectors << BDRV_SECTOR_BITS;
+}
+
+static int64_t sectors_to_process(int64_t total, int64_t from)
+{
+    int64_t ret = total - from;
+
+    if (ret > (IO_BUF_SIZE >> BDRV_SECTOR_BITS)) {
+        return IO_BUF_SIZE >> BDRV_SECTOR_BITS;
+    }
+
+    return ret;
+}
+
+/*
+ * Check if passed sectors are empty (not allocated or contain only 0 bytes)
+ *
+ * Returns 0 in case sectors are filled with 0, 1 if sectors contain non-zero
+ * data and negative value on error.
+ *
+ * @param bs:  Driver used for accessing file
+ * @param sect_num: Number of first sector to check
+ * @param sect_count: Number of sectors to check
+ * @param filename: Name of disk file we are checking (logging purpose)
+ * @param buffer: Allocated buffer for storing read data
+ * @param quiet: Flag for quiet mode
+ */
+static int check_empty_sectors(BlockDriverState *bs, int64_t sect_num,
+                               int sect_count, const char *filename,
+                               uint8_t *buffer, bool quiet)
+{
+    int pnum, ret = 0;
+    ret = bdrv_read(bs, sect_num, buffer, sect_count);
+    if (ret < 0) {
+        error_report("Error while reading offset %" PRId64 " of %s: %s",
+                     sectors_to_bytes(sect_num), filename, strerror(-ret));
+        return ret;
+    }
+    ret = is_allocated_sectors(buffer, sect_count, &pnum);
+    if (ret || pnum != sect_count) {
+        qprintf(quiet, "Content mismatch at offset %" PRId64 "!\n",
+                sectors_to_bytes(ret ? sect_num : sect_num + pnum));
+        return 1;
+    }
+
+    return 0;
+}
+
+/*
+ * Compares two images. Exit codes:
+ *
+ * 0 - Images are identical
+ * 1 - Images differ
+ * 2 - Error occurred
+ */
+static int img_compare(int argc, char **argv)
+{
+    const char *fmt1 = NULL, *fmt2 = NULL, *filename1, *filename2;
+    BlockDriverState *bs1, *bs2;
+    int64_t total_sectors1, total_sectors2;
+    uint8_t *buf1 = NULL, *buf2 = NULL;
+    int pnum1, pnum2;
+    int allocated1, allocated2;
+    int ret = 0; /* return value - 0 Ident, 1 Different, 2 Error */
+    int progress = 0, quiet = 0, strict = 0;
+    int64_t total_sectors;
+    int64_t sector_num = 0;
+    int64_t nb_sectors;
+    int c, pnum;
+    uint64_t bs_sectors;
+    uint64_t progress_base;
+
+    for (;;) {
+        c = getopt(argc, argv, "hpf:F:sq");
+        if (c == -1) {
+            break;
+        }
+        switch (c) {
+        case '?':
+        case 'h':
+            help();
+            break;
+        case 'f':
+            fmt1 = optarg;
+            break;
+        case 'F':
+            fmt2 = optarg;
+            break;
+        case 'p':
+            progress = 1;
+            break;
+        case 'q':
+            quiet = 1;
+            break;
+        case 's':
+            strict = 1;
+            break;
+        }
+    }
+
+    /* Progress is not shown in Quiet mode */
+    if (quiet) {
+        progress = 0;
+    }
+
+
+    if (optind > argc - 2) {
+        help();
+    }
+    filename1 = argv[optind++];
+    filename2 = argv[optind++];
+
+    /* Initialize before goto out */
+    qemu_progress_init(progress, 2.0);
+
+    bs1 = bdrv_new_open(filename1, fmt1, BDRV_O_FLAGS, true, quiet);
+    if (!bs1) {
+        error_report("Can't open file %s", filename1);
+        ret = 2;
+        goto out3;
+    }
+
+    bs2 = bdrv_new_open(filename2, fmt2, BDRV_O_FLAGS, true, quiet);
+    if (!bs2) {
+        error_report("Can't open file %s", filename2);
+        ret = 2;
+        goto out2;
+    }
+
+    buf1 = qemu_blockalign(bs1, IO_BUF_SIZE);
+    buf2 = qemu_blockalign(bs2, IO_BUF_SIZE);
+    bdrv_get_geometry(bs1, &bs_sectors);
+    total_sectors1 = bs_sectors;
+    bdrv_get_geometry(bs2, &bs_sectors);
+    total_sectors2 = bs_sectors;
+    total_sectors = MIN(total_sectors1, total_sectors2);
+    progress_base = MAX(total_sectors1, total_sectors2);
+
+    qemu_progress_print(0, 100);
+
+    if (strict && total_sectors1 != total_sectors2) {
+        ret = 1;
+        qprintf(quiet, "Strict mode: Image size mismatch!\n");
+        goto out;
+    }
+
+    for (;;) {
+        nb_sectors = sectors_to_process(total_sectors, sector_num);
+        if (nb_sectors <= 0) {
+            break;
+        }
+        allocated1 = bdrv_is_allocated_above(bs1, NULL, sector_num, nb_sectors,
+                                             &pnum1);
+        allocated2 = bdrv_is_allocated_above(bs2, NULL, sector_num, nb_sectors,
+                                             &pnum2);
+        nb_sectors = MIN(pnum1, pnum2);
+
+        if (allocated1 == allocated2) {
+            if (allocated1) {
+                ret = bdrv_read(bs1, sector_num, buf1, nb_sectors);
+                if (ret < 0) {
+                    error_report("error while reading offset %" PRId64 " of %s:"
+                                 " %s", sectors_to_bytes(sector_num), filename1,
+                                  strerror(-ret));
+                    ret = 2;
+                    goto out;
+                }
+                ret = bdrv_read(bs2, sector_num, buf2, nb_sectors);
+                if (ret < 0) {
+                    error_report("error while reading offset %" PRId64
+                                 " of %s: %s", sectors_to_bytes(sector_num),
+                                 filename2, strerror(-ret));
+                    ret = 2;
+                    goto out;
+                }
+                ret = compare_sectors(buf1, buf2, nb_sectors, &pnum);
+                if (ret || pnum != nb_sectors) {
+                    ret = 1;
+                    qprintf(quiet, "Content mismatch at offset %" PRId64 "!\n",
+                            sectors_to_bytes(
+                                ret ? sector_num : sector_num + pnum));
+                    goto out;
+                }
+            }
+        } else {
+            if (strict) {
+                ret = 1;
+                qprintf(quiet, "Strict mode: Offset %" PRId64
+                        " allocation mismatch!\n",
+                        sectors_to_bytes(sector_num));
+                goto out;
+            }
+
+            if (allocated1) {
+                ret = check_empty_sectors(bs1, sector_num, nb_sectors,
+                                         filename1, buf1, quiet);
+            } else {
+                ret = check_empty_sectors(bs2, sector_num, nb_sectors,
+                                         filename2, buf1, quiet);
+            }
+            if (ret) {
+                if (ret < 0) {
+                    ret = 2;
+                }
+                goto out;
+            }
+        }
+        sector_num += nb_sectors;
+        qemu_progress_print(((float) nb_sectors / progress_base)*100, 100);
+    }
+
+    if (total_sectors1 != total_sectors2) {
+        BlockDriverState *bs_over;
+        int64_t total_sectors_over;
+        const char *filename_over;
+
+        qprintf(quiet, "Warning: Image size mismatch!\n");
+        if (total_sectors1 > total_sectors2) {
+            total_sectors_over = total_sectors1;
+            bs_over = bs1;
+            filename_over = filename1;
+        } else {
+            total_sectors_over = total_sectors2;
+            bs_over = bs2;
+            filename_over = filename2;
+        }
+
+        for (;;) {
+            nb_sectors = sectors_to_process(total_sectors_over, sector_num);
+            if (nb_sectors <= 0) {
+                break;
+            }
+            ret = bdrv_is_allocated_above(bs_over, NULL, sector_num,
+                                          nb_sectors, &pnum);
+            nb_sectors = pnum;
+            if (ret) {
+                ret = check_empty_sectors(bs_over, sector_num, nb_sectors,
+                                          filename_over, buf1, quiet);
+                if (ret) {
+                    if (ret < 0) {
+                        ret = 2;
+                    }
+                    goto out;
+                }
+            }
+            sector_num += nb_sectors;
+            qemu_progress_print(((float) nb_sectors / progress_base)*100, 100);
+        }
+    }
+
+    qprintf(quiet, "Images are identical.\n");
+    ret = 0;
+
+out:
+    bdrv_delete(bs2);
+    qemu_vfree(buf1);
+    qemu_vfree(buf2);
+out2:
+    bdrv_delete(bs1);
+out3:
+    qemu_progress_end();
+    return ret;
+}
+
 static int img_convert(int argc, char **argv)
 {
     int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size, cluster_sectors;
-- 
1.7.11.7

[Qemu-devel] [PATCH v7 4/4] Add qemu-img compare documentation

[mrezanin]

From: Miroslav Rezanina <mrezanin@redhat.com>

Adding documentation for new qemu-img subcommand compare.

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
---
 qemu-img.c    |  7 ++++++-
 qemu-img.texi | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index 8b4f01f..a185e9e 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -103,7 +103,12 @@ static void help(void)
            "  '-a' applies a snapshot (revert disk to saved state)\n"
            "  '-c' creates a snapshot\n"
            "  '-d' deletes a snapshot\n"
-           "  '-l' lists all snapshots in the given image\n";
+           "  '-l' lists all snapshots in the given image\n"
+           "\n"
+           "Parameters to compare subcommand:\n"
+           "  '-f' First image format\n"
+           "  '-F' Second image format\n"
+           "  '-s' Strict mode - fail on different image size or sector allocation\n";
 
     printf("%s\nSupported formats:", help_msg);
     bdrv_iterate_format(format_print, NULL);
diff --git a/qemu-img.texi b/qemu-img.texi
index bb82a3d..7eb1934 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -84,6 +84,18 @@ deletes a snapshot
 lists all snapshots in the given image
 @end table
 
+Parameters to compare subcommand:
+
+@table @option
+
+@item -f
+First image format
+@item -F
+Second image format
+@item -s
+Strict mode - fail on on different image size or sector allocation
+@end table
+
 Command description:
 
 @table @option
@@ -117,6 +129,26 @@ it doesn't need to be specified separately in this case.
 
 Commit the changes recorded in @var{filename} in its base image.
 
+@item compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-s] [-q] @var{filename1} @var{filename2}
+
+Check if two images have the same content. You can compare images with
+different format or settings.
+
+The format is probed unless you specify it by @var{-f} (used for @var{filename1}) and/or @var{-F} (used for @var{filename2}) option.
+
+By default, images with different size are considered identical if the larger
+image contains only unallocated and/or zeroed sectors in the area after the end
+of the other image. In addition, if any sector is not allocated in one image
+and contains only zero bytes in the second one, it is evaluated as equal. You
+can use Strict mode by specifying the @var{-s} option. When compare runs in
+Strict mode, it fails in case image size differs or a sector is allocated in
+one image and is not allocated in the second one.
+
+By default, compare prints out a result message. This message displays
+information that both images are same or the position of the first different
+byte. In addition, result message can report different image size in case
+Strict mode is used.
+
 @item convert [-c] [-p] [-f @var{fmt}] [-t @var{cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
 
 Convert the disk image @var{filename} or a snapshot @var{snapshot_name} to disk image @var{output_filename}
-- 
1.7.11.7

Re: [Qemu-devel] [PATCH v7 4/4] Add qemu-img compare documentation

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1877 bytes --]

On 12/17/2012 06:39 AM, mrezanin@redhat.com wrote:
> From: Miroslav Rezanina <mrezanin@redhat.com>
> 
> Adding documentation for new qemu-img subcommand compare.
> 
> Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
> ---
>  qemu-img.c    |  7 ++++++-
>  qemu-img.texi | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 8b4f01f..a185e9e 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -103,7 +103,12 @@ static void help(void)
>             "  '-a' applies a snapshot (revert disk to saved state)\n"
>             "  '-c' creates a snapshot\n"
>             "  '-d' deletes a snapshot\n"
> -           "  '-l' lists all snapshots in the given image\n";
> +           "  '-l' lists all snapshots in the given image\n"
> +           "\n"
> +           "Parameters to compare subcommand:\n"
> +           "  '-f' First image format\n"
> +           "  '-F' Second image format\n"
> +           "  '-s' Strict mode - fail on different image size or sector allocation\n";

s/First/first/; s/Second/second/; s/Strict/strict/ for consistent
appearance of starting description with lower case

>  @table @option
> @@ -117,6 +129,26 @@ it doesn't need to be specified separately in this case.
>  
>  Commit the changes recorded in @var{filename} in its base image.
>  
> +@item compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-s] [-q] @var{filename1} @var{filename2}
> +
> +Check if two images have the same content. You can compare images with
> +different format or settings.
> +
> +The format is probed unless you specify it by @var{-f} (used for @var{filename1}) and/or @var{-F} (used for @var{filename2}) option.

Wrap this long line.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH v7 4/4] Add qemu-img compare documentation

[Miroslav Rezanina]

----- Original Message -----
> From: "Eric Blake" <eblake@redhat.com>
> To: mrezanin@redhat.com
> Cc: qemu-devel@nongnu.org, kwolf@redhat.com, pbonzini@redhat.com, stefanha@redhat.com
> Sent: Wednesday, December 19, 2012 7:15:11 PM
> Subject: Re: [Qemu-devel] [PATCH v7 4/4] Add qemu-img compare documentation
> 
> On 12/17/2012 06:39 AM, mrezanin@redhat.com wrote:
> > From: Miroslav Rezanina <mrezanin@redhat.com>
> > 
> > Adding documentation for new qemu-img subcommand compare.
> > 
> > Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
> > ---
> >  qemu-img.c    |  7 ++++++-
> >  qemu-img.texi | 32 ++++++++++++++++++++++++++++++++
> >  2 files changed, 38 insertions(+), 1 deletion(-)
> > 
> > diff --git a/qemu-img.c b/qemu-img.c
> > index 8b4f01f..a185e9e 100644
> > --- a/qemu-img.c
> > +++ b/qemu-img.c
> > @@ -103,7 +103,12 @@ static void help(void)
> >             "  '-a' applies a snapshot (revert disk to saved
> >             state)\n"
> >             "  '-c' creates a snapshot\n"
> >             "  '-d' deletes a snapshot\n"
> > -           "  '-l' lists all snapshots in the given image\n";
> > +           "  '-l' lists all snapshots in the given image\n"
> > +           "\n"
> > +           "Parameters to compare subcommand:\n"
> > +           "  '-f' First image format\n"
> > +           "  '-F' Second image format\n"
> > +           "  '-s' Strict mode - fail on different image size or
> > sector allocation\n";
> 
> s/First/first/; s/Second/second/; s/Strict/strict/ for consistent
> appearance of starting description with lower case

Yes for First and Second, but Strict mode is same case as Quiet mode
in patch 02 - it's always capital S not because it's start of description.

> 
> >  @table @option
> > @@ -117,6 +129,26 @@ it doesn't need to be specified separately in
> > this case.
> >  
> >  Commit the changes recorded in @var{filename} in its base image.
> >  
> > +@item compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-s] [-q]
> > @var{filename1} @var{filename2}
> > +
> > +Check if two images have the same content. You can compare images
> > with
> > +different format or settings.
> > +
> > +The format is probed unless you specify it by @var{-f} (used for
> > @var{filename1}) and/or @var{-F} (used for @var{filename2})
> > option.
> 
> Wrap this long line.

None of the long lines is wraped in qemu-img.c in this part of code so I did not wrap it intentionally. 
> 
> --
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 
> 
Miroslav Rezanina

Re: [Qemu-devel] [PATCH v7 3/4] qemu-img: Add compare subcommand

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1830 bytes --]

On 12/17/2012 06:39 AM, mrezanin@redhat.com wrote:
> From: Miroslav Rezanina <mrezanin@redhat.com>
> 
> This patch adds new qemu-img subcommand that compare content of two disk

s/compare/compares/

> images.
> 
> Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
> ---
> @@ -587,7 +587,7 @@ static int img_commit(int argc, char **argv)
>  }
>  
>  /*
> - * Returns true iff the first sector pointed to by 'buf' contains at least
> + * Returns true if the first sector pointed to by 'buf' contains at least

Spurious change.  'iff' is correct here, for its mathematical meaning of
if-and-only-if.

>   * a non-NUL byte.
>   *
>   * 'pnum' is set to the number of sectors (including and immediately following
> @@ -688,6 +688,272 @@ static int compare_sectors(const uint8_t *buf1, const uint8_t *buf2, int n,
>  
>  #define IO_BUF_SIZE (2 * 1024 * 1024)
>  
> +static int64_t sectors_to_bytes(int64_t sectors)
> +{
> +    return sectors << BDRV_SECTOR_BITS;

Worth checking for overflow?

> +static int check_empty_sectors(BlockDriverState *bs, int64_t sect_num,
> +                               int sect_count, const char *filename,
> +                               uint8_t *buffer, bool quiet)
> +{
> +    int pnum, ret = 0;
> +    ret = bdrv_read(bs, sect_num, buffer, sect_count);
> +    if (ret < 0) {
> +        error_report("Error while reading offset %" PRId64 " of %s: %s",
> +                     sectors_to_bytes(sect_num), filename, strerror(-ret));
> +        return ret;
> +    }
> +    ret = is_allocated_sectors(buffer, sect_count, &pnum);

Is this logic backwards?  Isn't it wasteful to read a sector prior to
seeing if it was actually allocated?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH v7 3/4] qemu-img: Add compare subcommand

[Miroslav Rezanina]

----- Original Message -----
> From: "Eric Blake" <eblake@redhat.com>
> To: mrezanin@redhat.com
> Cc: qemu-devel@nongnu.org, kwolf@redhat.com, pbonzini@redhat.com, stefanha@redhat.com
> Sent: Wednesday, December 19, 2012 7:12:49 PM
> Subject: Re: [Qemu-devel] [PATCH v7 3/4] qemu-img: Add compare subcommand
> 
> On 12/17/2012 06:39 AM, mrezanin@redhat.com wrote:
> > From: Miroslav Rezanina <mrezanin@redhat.com>
> > 
> > This patch adds new qemu-img subcommand that compare content of two
> > disk
> 
> s/compare/compares/
> 
> > images.
> > 
> > Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
> > ---
> > @@ -587,7 +587,7 @@ static int img_commit(int argc, char **argv)
> >  }
> >  
> >  /*
> > - * Returns true iff the first sector pointed to by 'buf' contains
> > at least
> > + * Returns true if the first sector pointed to by 'buf' contains
> > at least
> 
> Spurious change.  'iff' is correct here, for its mathematical meaning
> of
> if-and-only-if.

You're right. I probably just see iff and change it to if without checking
the reason.

> 
> >   * a non-NUL byte.
> >   *
> >   * 'pnum' is set to the number of sectors (including and
> >   immediately following
> > @@ -688,6 +688,272 @@ static int compare_sectors(const uint8_t
> > *buf1, const uint8_t *buf2, int n,
> >  
> >  #define IO_BUF_SIZE (2 * 1024 * 1024)
> >  
> > +static int64_t sectors_to_bytes(int64_t sectors)
> > +{
> > +    return sectors << BDRV_SECTOR_BITS;
> 
> Worth checking for overflow?
> 

I think it's not. This should be save as block driver should not allow sectors to
cause overflow.

> > +static int check_empty_sectors(BlockDriverState *bs, int64_t
> > sect_num,
> > +                               int sect_count, const char
> > *filename,
> > +                               uint8_t *buffer, bool quiet)
> > +{
> > +    int pnum, ret = 0;
> > +    ret = bdrv_read(bs, sect_num, buffer, sect_count);
> > +    if (ret < 0) {
> > +        error_report("Error while reading offset %" PRId64 " of
> > %s: %s",
> > +                     sectors_to_bytes(sect_num), filename,
> > strerror(-ret));
> > +        return ret;
> > +    }
> > +    ret = is_allocated_sectors(buffer, sect_count, &pnum);
> 
> Is this logic backwards?  Isn't it wasteful to read a sector prior to
> seeing if it was actually allocated?
> 
This is correct order. Function is_allocated_sector test if sectors contain any non-zero byte. We know that sector is "physically" allocated in the image, we test if it contains any data.
> --
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 
> 

Miroslav Rezanina

Re: [Qemu-devel] [PATCH v7 2/4] qemu-img: Add "Quiet mode" option

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 3050 bytes --]

On 12/17/2012 06:39 AM, mrezanin@redhat.com wrote:
> From: Miroslav Rezanina <mrezanin@redhat.com>

Your git send-email settings threaded each message as a reply to the
other, rather than the more typical setting of threading messages only
as a reply to the cover letter.  You may want to do 'git config
format.thread shallow' rather than your current deep approach.

> 
> There can be need to turn output to stdout off. This patch adds a -q option that

s/be need/be a need/

> enable "Quiet mode". In Quiet mode, only errors are printed out.
> 
> Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
> ---
>  block.c          |  11 +++---
>  block.h          |   2 +-
>  blockdev.c       |   6 ++--
>  qemu-img-cmds.hx |  28 +++++++--------
>  qemu-img.c       | 108 +++++++++++++++++++++++++++++++++++++++++--------------
>  qemu-img.texi    |   3 ++
>  6 files changed, 108 insertions(+), 50 deletions(-)
> 
> @@ -1275,7 +1275,7 @@ void qmp_drive_mirror(const char *device, const char *target,
>              ret = bdrv_img_create(target, format,
>                                    source->filename,
>                                    source->drv->format_name,
> -                                  NULL, -1, flags);
> +                                  NULL, -1, flags,false);

Space after comma.

> @@ -10,27 +10,27 @@ STEXI
>  ETEXI
>  
>  DEF("check", img_check,
> -    "check [-f fmt] [-r [leaks | all]] filename")
> +    "check [-q] [-f fmt] [-r [leaks | all]] filename")

Is it really better to list [-q] to all of the sub-commands, or would it
be easier to simply declare that -q is a universal option that can
appear before sub-commands, as in:
qemu-img [-q] command [command options]

>  #ifdef _WIN32
>  #include <windows.h>
> @@ -86,6 +87,7 @@ static void help(void)
>             "       rebasing in this case (useful for renaming the backing file)\n"
>             "  '-h' with or without a command shows this help and lists the supported formats\n"
>             "  '-p' show progress of command (only certain commands)\n"
> +           "  '-q' Quiet mode - do not print any output (except errors)\n"

s/Quiet/quiet/ for consistency with the nearby lines not starting with a
capital.

> +++ b/qemu-img.texi
> @@ -54,6 +54,9 @@ indicates that target image must be compressed (qcow format only)
>  with or without a command shows help and lists the supported formats
>  @item -p
>  display progress bar (convert and rebase commands only)
> +@item -q
> +Quiet mode - do not print any output (except errors). There's no progres bar 

s/progres/progress/

> +in case both @var{-q} and  @var{-p} options are used.

Should it be a hard error if both -q and -p are given?  Otherwise, when
dealing with conflicting options, it's more typical to have the semantic
of last one wins: 'qemu-img -q -p' prints progress, and only 'qemu-img
-p -q' is quiet.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH v7 2/4] qemu-img: Add "Quiet mode" option

[Miroslav Rezanina]

Hi Eric,
thanks for review, reply inline.

----- Original Message -----
> From: "Eric Blake" <eblake@redhat.com>
> To: mrezanin@redhat.com
> Cc: qemu-devel@nongnu.org, kwolf@redhat.com, pbonzini@redhat.com, stefanha@redhat.com
> Sent: Wednesday, December 19, 2012 7:06:35 PM
> Subject: Re: [Qemu-devel] [PATCH v7 2/4] qemu-img: Add "Quiet mode" option
> 
> On 12/17/2012 06:39 AM, mrezanin@redhat.com wrote:
> > From: Miroslav Rezanina <mrezanin@redhat.com>
> 
> Your git send-email settings threaded each message as a reply to the
> other, rather than the more typical setting of threading messages
> only
> as a reply to the cover letter.  You may want to do 'git config
> format.thread shallow' rather than your current deep approach.

I use temporary machine for sending patches as my usual one was not usable
for me in this case, so setting can be incorrect. I will take better care of this
next time.

> 
> > 
> > There can be need to turn output to stdout off. This patch adds a
> > -q option that
> 
> s/be need/be a need/
> 
> > enable "Quiet mode". In Quiet mode, only errors are printed out.
> > 
> > Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
> > ---
> >  block.c          |  11 +++---
> >  block.h          |   2 +-
> >  blockdev.c       |   6 ++--
> >  qemu-img-cmds.hx |  28 +++++++--------
> >  qemu-img.c       | 108
> >  +++++++++++++++++++++++++++++++++++++++++--------------
> >  qemu-img.texi    |   3 ++
> >  6 files changed, 108 insertions(+), 50 deletions(-)
> > 
> > @@ -1275,7 +1275,7 @@ void qmp_drive_mirror(const char *device,
> > const char *target,
> >              ret = bdrv_img_create(target, format,
> >                                    source->filename,
> >                                    source->drv->format_name,
> > -                                  NULL, -1, flags);
> > +                                  NULL, -1, flags,false);
> 
> Space after comma.
> 
> > @@ -10,27 +10,27 @@ STEXI
> >  ETEXI
> >  
> >  DEF("check", img_check,
> > -    "check [-f fmt] [-r [leaks | all]] filename")
> > +    "check [-q] [-f fmt] [-r [leaks | all]] filename")
> 
> Is it really better to list [-q] to all of the sub-commands, or would
> it
> be easier to simply declare that -q is a universal option that can
> appear before sub-commands, as in:
> qemu-img [-q] command [command options]
> 

-q is not useable for all commands so it is listed this ways as it is same
for other options.

> >  #ifdef _WIN32
> >  #include <windows.h>
> > @@ -86,6 +87,7 @@ static void help(void)
> >             "       rebasing in this case (useful for renaming the
> >             backing file)\n"
> >             "  '-h' with or without a command shows this help and
> >             lists the supported formats\n"
> >             "  '-p' show progress of command (only certain
> >             commands)\n"
> > +           "  '-q' Quiet mode - do not print any output (except
> > errors)\n"
> 
> s/Quiet/quiet/ for consistency with the nearby lines not starting
> with a
> capital.

I use Quiet mode as name with capital Q, so I have to choose what consistency
to break - I choose the start of the description. I have no problem with using
quiet instead of Quiet.

> 
> > +++ b/qemu-img.texi
> > @@ -54,6 +54,9 @@ indicates that target image must be compressed
> > (qcow format only)
> >  with or without a command shows help and lists the supported
> >  formats
> >  @item -p
> >  display progress bar (convert and rebase commands only)
> > +@item -q
> > +Quiet mode - do not print any output (except errors). There's no
> > progres bar
> 
> s/progres/progress/
> 
> > +in case both @var{-q} and  @var{-p} options are used.
> 
> Should it be a hard error if both -q and -p are given?  Otherwise,
> when
> dealing with conflicting options, it's more typical to have the
> semantic
> of last one wins: 'qemu-img -q -p' prints progress, and only
> 'qemu-img
> -p -q' is quiet.
> 

Depends on point of view - There's no conflict for -p and -q as
-p adds progress bar to output and -q suppress output. You can imagine 
(in theory) -q as redirecting stdout to /dev/null. 
> --
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 
> 

Re: [Qemu-devel] [PATCH v7 1/4] block: Add synchronous wrapper for bdrv_co_is_allocated_above

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1078 bytes --]

On 12/17/2012 06:39 AM, mrezanin@redhat.com wrote:
> From: Miroslav Rezanina <mrezanin@redhat.com>
> 
> There's no synchronous wrapper for bdrv_co_is_allocated_above function
> so it's not possible to check for sector allocation in image with

s/in image with/in an image with a/

> backing file.
> 
> This patch add missing synchronous wrapper.

s/add/adds the/

> 
> Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
> ---
>  block.c | 39 +++++++++++++++++++++++++++++++++++++++
>  block.h |  2 ++
>  2 files changed, 41 insertions(+)
> 
> diff --git a/block.c b/block.c
> index c05875f..24c06ab 100644
> --- a/block.c
> +/*
> + * Synchronous wrapper around bdrv_co_is_allocated_above().
> + *
> + * See bdrv_co_is_allocated_above() for details.
> + */
> +int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
> +                      int64_t sector_num, int nb_sectors, int *pnum)

Indentation looks off.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

[Qemu-devel] [PATCH v8 0/4] Add subcommand compare for qemu-img

[Miroslav Rezanina]

This is 8th version of patch adding compare subcommand that
compares two images. Compare has following criteria:
 - only data part is compared
 - unallocated sectors are not read
 - in case of different image size, exceeding part of bigger disk has
 to be zeroed/unallocated to compare rest
 - qemu-img returns:
    - 0 if images are identical
    - 1 if images differ
    - 2 on error

v8:
 - minor grammar fixes (no behavior change)
 
v7:
 - split patch into pieces
 - Quiet mode added for all relevant subcommands
 - check non-shared part of disk after shared one
 - minor docummentation and naming fixes

v6:
 - added handling -?, -h options for compare subcommand

v5 (only minor changes):
 - removed redundant comment
 - removed dead code (goto after help())
 - set final total_sectors on first assignment

v4:
 - Fixed various typos
 - Added functions for empty sector check and sector-to-bytes offset
 conversion
 - Fixed command-line parameters processing

v3:
 - options -f/-F are orthogonal
 - documentation updated to new syntax and behavior
 - used byte offset instead of sector number for output
 
v2:
 - changed option for second image format to -F
 - changed handling of -f and -F [1]
 - added strict mode (-s)
 - added quiet mode (-q)
 - improved output messages [2]
 - rename variables for larger image handling
 - added man page content

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>

Miroslav Rezanina (4):
  block: Add synchronous wrapper for bdrv_co_is_allocated_above
  qemu-img: Add "Quiet mode" option
  qemu-img: Add compare subcommand
  Add qemu-img compare documentation

 block.c               |   51 ++++++-
 blockdev.c            |    6 +-
 include/block/block.h |    5 +-
 qemu-img-cmds.hx      |   34 +++--
 qemu-img.c            |  381 +++++++++++++++++++++++++++++++++++++++++++++----
 qemu-img.texi         |   35 +++++
 6 files changed, 461 insertions(+), 51 deletions(-)

[Qemu-devel] [PATCH v8 1/4] block: Add synchronous wrapper for bdrv_co_is_allocated_above

[Miroslav Rezanina]

There's no synchronous wrapper for bdrv_co_is_allocated_above function
so it's not possible to check for sector allocation in in mage with
a backing file.

This patch adds the missing synchronous wrapper.

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
---
 block.c               |   39 +++++++++++++++++++++++++++++++++++++++
 include/block/block.h |    2 ++
 2 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index 60873ea..8ea8bb7 100644
--- a/block.c
+++ b/block.c
@@ -2716,6 +2716,7 @@ int bdrv_has_zero_init(BlockDriverState *bs)
 
 typedef struct BdrvCoIsAllocatedData {
     BlockDriverState *bs;
+    BlockDriverState *base;
     int64_t sector_num;
     int nb_sectors;
     int *pnum;
@@ -2846,6 +2847,44 @@ int coroutine_fn bdrv_co_is_allocated_above(BlockDriverState *top,
     return 0;
 }
 
+/* Coroutine wrapper for bdrv_is_allocated_above() */
+static void coroutine_fn bdrv_is_allocated_above_co_entry(void *opaque)
+{
+    BdrvCoIsAllocatedData *data = opaque;
+    BlockDriverState *top = data->bs;
+    BlockDriverState *base = data->base;
+
+    data->ret = bdrv_co_is_allocated_above(top, base, data->sector_num,
+                                           data->nb_sectors, data->pnum);
+    data->done = true;
+}
+
+/*
+ * Synchronous wrapper around bdrv_co_is_allocated_above().
+ *
+ * See bdrv_co_is_allocated_above() for details.
+ */
+int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
+                            int64_t sector_num, int nb_sectors, int *pnum)
+{
+    Coroutine *co;
+    BdrvCoIsAllocatedData data = {
+        .bs = top,
+        .base = base,
+        .sector_num = sector_num,
+        .nb_sectors = nb_sectors,
+        .pnum = pnum,
+        .done = false,
+    };
+
+    co = qemu_coroutine_create(bdrv_is_allocated_above_co_entry);
+    qemu_coroutine_enter(co, &data);
+    while (!data.done) {
+        qemu_aio_wait();
+    }
+    return data.ret;
+}
+
 BlockInfo *bdrv_query_info(BlockDriverState *bs)
 {
     BlockInfo *info = g_malloc0(sizeof(*info));
diff --git a/include/block/block.h b/include/block/block.h
index 0719339..ec62d92 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -278,6 +278,8 @@ int bdrv_co_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors);
 int bdrv_has_zero_init(BlockDriverState *bs);
 int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
                       int *pnum);
+int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
+                            int64_t sector_num, int nb_sectors, int *pnum);
 
 void bdrv_set_on_error(BlockDriverState *bs, BlockdevOnError on_read_error,
                        BlockdevOnError on_write_error);
-- 
1.7.1

Re: [Qemu-devel] [PATCH v8 1/4] block: Add synchronous wrapper for bdrv_co_is_allocated_above

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 691 bytes --]

On 01/14/2013 03:26 AM, Miroslav Rezanina wrote:
> There's no synchronous wrapper for bdrv_co_is_allocated_above function
> so it's not possible to check for sector allocation in in mage with

s/in in mage/in an image/

> a backing file.
> 
> This patch adds the missing synchronous wrapper.
> 
> Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
> ---
>  block.c               |   39 +++++++++++++++++++++++++++++++++++++++
>  include/block/block.h |    2 ++
>  2 files changed, 41 insertions(+), 0 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

[Qemu-devel] [PATCH v8 2/4] qemu-img: Add "Quiet mode" option

[Miroslav Rezanina]

There can be a need to turn output to stdout off. This patch adds a -q option
that enable "Quiet mode". In Quiet mode, only errors are printed out.

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
---
 block.c               |   12 +++--
 blockdev.c            |    6 +-
 include/block/block.h |    3 +-
 qemu-img-cmds.hx      |   28 ++++++------
 qemu-img.c            |  108 ++++++++++++++++++++++++++++++++++++------------
 qemu-img.texi         |    3 +
 6 files changed, 110 insertions(+), 50 deletions(-)

diff --git a/block.c b/block.c
index 8ea8bb7..e563ca8 100644
--- a/block.c
+++ b/block.c
@@ -4512,7 +4512,8 @@ bdrv_acct_done(BlockDriverState *bs, BlockAcctCookie *cookie)
 
 void bdrv_img_create(const char *filename, const char *fmt,
                      const char *base_filename, const char *base_fmt,
-                     char *options, uint64_t img_size, int flags, Error **errp)
+                     char *options, uint64_t img_size, int flags,
+                     Error **errp, bool quiet)
 {
     QEMUOptionParameter *param = NULL, *create_options = NULL;
     QEMUOptionParameter *backing_fmt, *backing_file, *size;
@@ -4621,10 +4622,11 @@ void bdrv_img_create(const char *filename, const char *fmt,
         }
     }
 
-    printf("Formatting '%s', fmt=%s ", filename, fmt);
-    print_option_parameters(param);
-    puts("");
-
+    if (!quiet) {
+        printf("Formatting '%s', fmt=%s ", filename, fmt);
+        print_option_parameters(param);
+        puts("");
+    }
     ret = bdrv_create(drv, filename, param);
     if (ret < 0) {
         if (ret == -ENOTSUP) {
diff --git a/blockdev.c b/blockdev.c
index d724e2d..3e377f9 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -790,7 +790,7 @@ void qmp_transaction(BlockdevActionList *dev_list, Error **errp)
             bdrv_img_create(new_image_file, format,
                             states->old_bs->filename,
                             states->old_bs->drv->format_name,
-                            NULL, -1, flags, &local_err);
+                            NULL, -1, flags, &local_err, false);
             if (error_is_set(&local_err)) {
                 error_propagate(errp, local_err);
                 goto delete_and_fail;
@@ -1265,7 +1265,7 @@ void qmp_drive_mirror(const char *device, const char *target,
         bdrv_get_geometry(bs, &size);
         size *= 512;
         bdrv_img_create(target, format,
-                        NULL, NULL, NULL, size, flags, &local_err);
+                        NULL, NULL, NULL, size, flags, &local_err, false);
     } else {
         switch (mode) {
         case NEW_IMAGE_MODE_EXISTING:
@@ -1276,7 +1276,7 @@ void qmp_drive_mirror(const char *device, const char *target,
             bdrv_img_create(target, format,
                             source->filename,
                             source->drv->format_name,
-                            NULL, -1, flags, &local_err);
+                            NULL, -1, flags, &local_err, false);
             break;
         default:
             abort();
diff --git a/include/block/block.h b/include/block/block.h
index ec62d92..03948c6 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -347,7 +347,8 @@ int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf,
 
 void bdrv_img_create(const char *filename, const char *fmt,
                      const char *base_filename, const char *base_fmt,
-                     char *options, uint64_t img_size, int flags, Error **errp);
+                     char *options, uint64_t img_size, int flags,
+                     Error **errp, bool quiet);
 
 void bdrv_set_buffer_alignment(BlockDriverState *bs, int align);
 void *qemu_blockalign(BlockDriverState *bs, size_t size);
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index a181363..90b93e0 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -10,27 +10,27 @@ STEXI
 ETEXI
 
 DEF("check", img_check,
-    "check [-f fmt] [-r [leaks | all]] filename")
+    "check [-q] [-f fmt] [-r [leaks | all]] filename")
 STEXI
-@item check [-f @var{fmt}] [-r [leaks | all]] @var{filename}
+@item check [-q] [-f @var{fmt}] [-r [leaks | all]] @var{filename}
 ETEXI
 
 DEF("create", img_create,
-    "create [-f fmt] [-o options] filename [size]")
+    "create [-q] [-f fmt] [-o options] filename [size]")
 STEXI
-@item create [-f @var{fmt}] [-o @var{options}] @var{filename} [@var{size}]
+@item create [-q] [-f @var{fmt}] [-o @var{options}] @var{filename} [@var{size}]
 ETEXI
 
 DEF("commit", img_commit,
-    "commit [-f fmt] [-t cache] filename")
+    "commit [-q] [-f fmt] [-t cache] filename")
 STEXI
-@item commit [-f @var{fmt}] [-t @var{cache}] @var{filename}
+@item commit [-q] [-f @var{fmt}] [-t @var{cache}] @var{filename}
 ETEXI
 
 DEF("convert", img_convert,
-    "convert [-c] [-p] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s snapshot_name] [-S sparse_size] filename [filename2 [...]] output_filename")
+    "convert [-c] [-p] [-q] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s snapshot_name] [-S sparse_size] filename [filename2 [...]] output_filename")
 STEXI
-@item convert [-c] [-p] [-f @var{fmt}] [-t @var{cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
+@item convert [-c] [-p] [-q] [-f @var{fmt}] [-t @var{cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
 ETEXI
 
 DEF("info", img_info,
@@ -40,20 +40,20 @@ STEXI
 ETEXI
 
 DEF("snapshot", img_snapshot,
-    "snapshot [-l | -a snapshot | -c snapshot | -d snapshot] filename")
+    "snapshot [-q] [-l | -a snapshot | -c snapshot | -d snapshot] filename")
 STEXI
-@item snapshot [-l | -a @var{snapshot} | -c @var{snapshot} | -d @var{snapshot}] @var{filename}
+@item snapshot [-q] [-l | -a @var{snapshot} | -c @var{snapshot} | -d @var{snapshot}] @var{filename}
 ETEXI
 
 DEF("rebase", img_rebase,
-    "rebase [-f fmt] [-t cache] [-p] [-u] -b backing_file [-F backing_fmt] filename")
+    "rebase [-q] [-f fmt] [-t cache] [-p] [-u] -b backing_file [-F backing_fmt] filename")
 STEXI
-@item rebase [-f @var{fmt}] [-t @var{cache}] [-p] [-u] -b @var{backing_file} [-F @var{backing_fmt}] @var{filename}
+@item rebase [-q] [-f @var{fmt}] [-t @var{cache}] [-p] [-u] -b @var{backing_file} [-F @var{backing_fmt}] @var{filename}
 ETEXI
 
 DEF("resize", img_resize,
-    "resize filename [+ | -]size")
+    "resize [-q] filename [+ | -]size")
 STEXI
-@item resize @var{filename} [+ | -]@var{size}
+@item resize [-q] @var{filename} [+ | -]@var{size}
 @end table
 ETEXI
diff --git a/qemu-img.c b/qemu-img.c
index 85d3740..8d55829 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -32,6 +32,7 @@
 #include "block/block_int.h"
 #include <getopt.h>
 #include <stdio.h>
+#include <stdarg.h>
 
 #ifdef _WIN32
 #include <windows.h>
@@ -86,6 +87,7 @@ static void help(void)
            "       rebasing in this case (useful for renaming the backing file)\n"
            "  '-h' with or without a command shows this help and lists the supported formats\n"
            "  '-p' show progress of command (only certain commands)\n"
+           "  '-q' Quiet mode - do not print any output (except errors)\n"
            "  '-S' indicates the consecutive number of bytes that must contain only zeros\n"
            "       for qemu-img to create a sparse image during conversion\n"
            "  '--output' takes the format in which the output must be done (human or json)\n"
@@ -109,6 +111,18 @@ static void help(void)
     exit(1);
 }
 
+static int qprintf(bool quiet, const char* fmt, ...)
+{
+    int ret = 0;
+    if (!quiet) {
+        va_list args;
+        va_start(args, fmt);
+        ret = vprintf(fmt, args);
+        va_end(args);
+    }
+    return ret;
+}
+
 #if defined(WIN32)
 /* XXX: put correct support for win32 */
 static int read_password(char *buf, int buf_size)
@@ -227,7 +241,8 @@ static int print_block_option_help(const char *filename, const char *fmt)
 static BlockDriverState *bdrv_new_open(const char *filename,
                                        const char *fmt,
                                        int flags,
-                                       bool require_io)
+                                       bool require_io, 
+                                       bool quiet)
 {
     BlockDriverState *bs;
     BlockDriver *drv;
@@ -253,7 +268,7 @@ static BlockDriverState *bdrv_new_open(const char *filename,
     }
 
     if (bdrv_is_encrypted(bs) && require_io) {
-        printf("Disk image '%s' is encrypted.\n", filename);
+        qprintf(quiet, "Disk image '%s' is encrypted.\n", filename);
         if (read_password(password, sizeof(password)) < 0) {
             error_report("No password given");
             goto fail;
@@ -302,9 +317,10 @@ static int img_create(int argc, char **argv)
     const char *base_filename = NULL;
     char *options = NULL;
     Error *local_err = NULL;
+    bool quiet = false;
 
     for(;;) {
-        c = getopt(argc, argv, "F:b:f:he6o:");
+        c = getopt(argc, argv, "F:b:f:he6o:q");
         if (c == -1) {
             break;
         }
@@ -333,6 +349,9 @@ static int img_create(int argc, char **argv)
         case 'o':
             options = optarg;
             break;
+        case 'q':
+            quiet = true;
+            break;
         }
     }
 
@@ -365,7 +384,7 @@ static int img_create(int argc, char **argv)
     }
 
     bdrv_img_create(filename, fmt, base_filename, base_fmt,
-                    options, img_size, BDRV_O_FLAGS, &local_err);
+                    options, img_size, BDRV_O_FLAGS, &local_err, quiet);
     if (error_is_set(&local_err)) {
         error_report("%s", error_get_pretty(local_err));
         error_free(local_err);
@@ -391,10 +410,11 @@ static int img_check(int argc, char **argv)
     BdrvCheckResult result;
     int fix = 0;
     int flags = BDRV_O_FLAGS | BDRV_O_CHECK;
+    bool quiet = false;
 
     fmt = NULL;
     for(;;) {
-        c = getopt(argc, argv, "f:hr:");
+        c = getopt(argc, argv, "f:hr:q");
         if (c == -1) {
             break;
         }
@@ -417,6 +437,9 @@ static int img_check(int argc, char **argv)
                 help();
             }
             break;
+        case 'q':
+            quiet = true;
+            break;
         }
     }
     if (optind >= argc) {
@@ -424,7 +447,7 @@ static int img_check(int argc, char **argv)
     }
     filename = argv[optind++];
 
-    bs = bdrv_new_open(filename, fmt, flags, true);
+    bs = bdrv_new_open(filename, fmt, flags, true, quiet);
     if (!bs) {
         return 1;
     }
@@ -437,7 +460,8 @@ static int img_check(int argc, char **argv)
     }
 
     if (result.corruptions_fixed || result.leaks_fixed) {
-        printf("The following inconsistencies were found and repaired:\n\n"
+        qprintf(quiet, 
+               "The following inconsistencies were found and repaired:\n\n"
                "    %d leaked clusters\n"
                "    %d corruptions\n\n"
                "Double checking the fixed image now...\n",
@@ -447,29 +471,31 @@ static int img_check(int argc, char **argv)
     }
 
     if (!(result.corruptions || result.leaks || result.check_errors)) {
-        printf("No errors were found on the image.\n");
+        qprintf(quiet, "No errors were found on the image.\n");
     } else {
         if (result.corruptions) {
-            printf("\n%d errors were found on the image.\n"
+            qprintf(quiet, "\n%d errors were found on the image.\n"
                 "Data may be corrupted, or further writes to the image "
                 "may corrupt it.\n",
                 result.corruptions);
         }
 
         if (result.leaks) {
-            printf("\n%d leaked clusters were found on the image.\n"
+            qprintf(quiet, "\n%d leaked clusters were found on the image.\n"
                 "This means waste of disk space, but no harm to data.\n",
                 result.leaks);
         }
 
         if (result.check_errors) {
-            printf("\n%d internal errors have occurred during the check.\n",
+            qprintf(quiet, 
+                    "\n%d internal errors have occurred during the check.\n",
                 result.check_errors);
         }
     }
 
     if (result.bfi.total_clusters != 0 && result.bfi.allocated_clusters != 0) {
-        printf("%" PRId64 "/%" PRId64 "= %0.2f%% allocated, %0.2f%% fragmented\n",
+        qprintf(quiet, 
+           "%" PRId64 "/%" PRId64 "= %0.2f%% allocated, %0.2f%% fragmented\n",
         result.bfi.allocated_clusters, result.bfi.total_clusters,
         result.bfi.allocated_clusters * 100.0 / result.bfi.total_clusters,
         result.bfi.fragmented_clusters * 100.0 / result.bfi.allocated_clusters);
@@ -478,7 +504,7 @@ static int img_check(int argc, char **argv)
     bdrv_delete(bs);
 
     if (ret < 0 || result.check_errors) {
-        printf("\nAn error has occurred during the check: %s\n"
+        qprintf(quiet, "\nAn error has occurred during the check: %s\n"
             "The check is not complete and may have missed error.\n",
             strerror(-ret));
         return 1;
@@ -498,11 +524,12 @@ static int img_commit(int argc, char **argv)
     int c, ret, flags;
     const char *filename, *fmt, *cache;
     BlockDriverState *bs;
+    bool quiet = false;
 
     fmt = NULL;
     cache = BDRV_DEFAULT_CACHE;
     for(;;) {
-        c = getopt(argc, argv, "f:ht:");
+        c = getopt(argc, argv, "f:ht:q");
         if (c == -1) {
             break;
         }
@@ -517,6 +544,9 @@ static int img_commit(int argc, char **argv)
         case 't':
             cache = optarg;
             break;
+        case 'q':
+            quiet = true;
+            break;
         }
     }
     if (optind >= argc) {
@@ -531,14 +561,14 @@ static int img_commit(int argc, char **argv)
         return -1;
     }
 
-    bs = bdrv_new_open(filename, fmt, flags, true);
+    bs = bdrv_new_open(filename, fmt, flags, true, quiet);
     if (!bs) {
         return 1;
     }
     ret = bdrv_commit(bs);
     switch(ret) {
     case 0:
-        printf("Image committed.\n");
+        qprintf(quiet, "Image committed.\n");
         break;
     case -ENOENT:
         error_report("No disk inserted");
@@ -681,6 +711,7 @@ static int img_convert(int argc, char **argv)
     const char *snapshot_name = NULL;
     float local_progress = 0;
     int min_sparse = 8; /* Need at least 4k of zeros for sparse detection */
+    bool quiet = false; 
 
     fmt = NULL;
     out_fmt = "raw";
@@ -688,7 +719,7 @@ static int img_convert(int argc, char **argv)
     out_baseimg = NULL;
     compress = 0;
     for(;;) {
-        c = getopt(argc, argv, "f:O:B:s:hce6o:pS:t:");
+        c = getopt(argc, argv, "f:O:B:s:hce6o:pS:t:q");
         if (c == -1) {
             break;
         }
@@ -742,9 +773,16 @@ static int img_convert(int argc, char **argv)
         case 't':
             cache = optarg;
             break;
+        case 'q':
+            quiet = true;
+            break;
         }
     }
 
+    if (quiet) {
+        progress = 0;
+    }
+
     bs_n = argc - optind - 1;
     if (bs_n < 1) {
         help();
@@ -773,7 +811,7 @@ static int img_convert(int argc, char **argv)
 
     total_sectors = 0;
     for (bs_i = 0; bs_i < bs_n; bs_i++) {
-        bs[bs_i] = bdrv_new_open(argv[optind + bs_i], fmt, BDRV_O_FLAGS, true);
+        bs[bs_i] = bdrv_new_open(argv[optind + bs_i], fmt, BDRV_O_FLAGS, true, quiet);
         if (!bs[bs_i]) {
             error_report("Could not open '%s'", argv[optind + bs_i]);
             ret = -1;
@@ -892,7 +930,7 @@ static int img_convert(int argc, char **argv)
         return -1;
     }
 
-    out_bs = bdrv_new_open(out_filename, out_fmt, flags, true);
+    out_bs = bdrv_new_open(out_filename, out_fmt, flags, true, quiet);
     if (!out_bs) {
         ret = -1;
         goto out;
@@ -1355,7 +1393,7 @@ static ImageInfoList *collect_image_info_list(const char *filename,
         g_hash_table_insert(filenames, (gpointer)filename, NULL);
 
         bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING,
-                           false);
+                           false, false);
         if (!bs) {
             goto err;
         }
@@ -1491,11 +1529,12 @@ static int img_snapshot(int argc, char **argv)
     int c, ret = 0, bdrv_oflags;
     int action = 0;
     qemu_timeval tv;
+    bool quiet = false;
 
     bdrv_oflags = BDRV_O_FLAGS | BDRV_O_RDWR;
     /* Parse commandline parameters */
     for(;;) {
-        c = getopt(argc, argv, "la:c:d:h");
+        c = getopt(argc, argv, "la:c:d:hq");
         if (c == -1) {
             break;
         }
@@ -1536,6 +1575,9 @@ static int img_snapshot(int argc, char **argv)
             action = SNAPSHOT_DELETE;
             snapshot_name = optarg;
             break;
+        case 'q':
+            quiet = true;
+            break;
         }
     }
 
@@ -1545,7 +1587,7 @@ static int img_snapshot(int argc, char **argv)
     filename = argv[optind++];
 
     /* Open the image */
-    bs = bdrv_new_open(filename, NULL, bdrv_oflags, true);
+    bs = bdrv_new_open(filename, NULL, bdrv_oflags, true, quiet);
     if (!bs) {
         return 1;
     }
@@ -1605,6 +1647,7 @@ static int img_rebase(int argc, char **argv)
     int c, flags, ret;
     int unsafe = 0;
     int progress = 0;
+    bool quiet = false;
 
     /* Parse commandline parameters */
     fmt = NULL;
@@ -1612,7 +1655,7 @@ static int img_rebase(int argc, char **argv)
     out_baseimg = NULL;
     out_basefmt = NULL;
     for(;;) {
-        c = getopt(argc, argv, "uhf:F:b:pt:");
+        c = getopt(argc, argv, "uhf:F:b:pt:q");
         if (c == -1) {
             break;
         }
@@ -1639,9 +1682,16 @@ static int img_rebase(int argc, char **argv)
         case 't':
             cache = optarg;
             break;
+        case 'q':
+            quiet = true;
+            break;
         }
     }
 
+    if (quiet) {
+        progress = 0;
+    }
+
     if ((optind >= argc) || (!unsafe && !out_baseimg)) {
         help();
     }
@@ -1663,7 +1713,7 @@ static int img_rebase(int argc, char **argv)
      * Ignore the old backing file for unsafe rebase in case we want to correct
      * the reference to a renamed or moved backing file.
      */
-    bs = bdrv_new_open(filename, fmt, flags, true);
+    bs = bdrv_new_open(filename, fmt, flags, true, quiet);
     if (!bs) {
         return 1;
     }
@@ -1875,6 +1925,7 @@ static int img_resize(int argc, char **argv)
     int c, ret, relative;
     const char *filename, *fmt, *size;
     int64_t n, total_size;
+    bool quiet = false;
     BlockDriverState *bs = NULL;
     QemuOpts *param;
     static QemuOptsList resize_options = {
@@ -1903,7 +1954,7 @@ static int img_resize(int argc, char **argv)
     /* Parse getopt arguments */
     fmt = NULL;
     for(;;) {
-        c = getopt(argc, argv, "f:h");
+        c = getopt(argc, argv, "f:hq");
         if (c == -1) {
             break;
         }
@@ -1915,6 +1966,9 @@ static int img_resize(int argc, char **argv)
         case 'f':
             fmt = optarg;
             break;
+        case 'q':
+            quiet = true;
+            break;
         }
     }
     if (optind >= argc) {
@@ -1948,7 +2002,7 @@ static int img_resize(int argc, char **argv)
     n = qemu_opt_get_size(param, BLOCK_OPT_SIZE, 0);
     qemu_opts_del(param);
 
-    bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR, true);
+    bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR, true, quiet);
     if (!bs) {
         ret = -1;
         goto out;
@@ -1968,7 +2022,7 @@ static int img_resize(int argc, char **argv)
     ret = bdrv_truncate(bs, total_size);
     switch (ret) {
     case 0:
-        printf("Image resized.\n");
+        qprintf(quiet, "Image resized.\n");
         break;
     case -ENOTSUP:
         error_report("This image does not support resize");
diff --git a/qemu-img.texi b/qemu-img.texi
index 00fca8d..4fdb19a 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -54,6 +54,9 @@ indicates that target image must be compressed (qcow format only)
 with or without a command shows help and lists the supported formats
 @item -p
 display progress bar (convert and rebase commands only)
+@item -q
+Quiet mode - do not print any output (except errors). There's no progress bar 
+in case both @var{-q} and  @var{-p} options are used.
 @item -S @var{size}
 indicates the consecutive number of bytes that must contain only zeros
 for qemu-img to create a sparse image during conversion. This value is rounded
-- 
1.7.1

Re: [Qemu-devel] [PATCH v8 2/4] qemu-img: Add "Quiet mode" option

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 845 bytes --]

On 01/14/2013 03:26 AM, Miroslav Rezanina wrote:
> There can be a need to turn output to stdout off. This patch adds a -q option
> that enable "Quiet mode". In Quiet mode, only errors are printed out.
> 
> Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
> ---

>      if (result.corruptions_fixed || result.leaks_fixed) {
> -        printf("The following inconsistencies were found and repaired:\n\n"
> +        qprintf(quiet, 
> +               "The following inconsistencies were found and repaired:\n\n"
>                 "    %d leaked clusters\n"
>                 "    %d corruptions\n\n"

Here, and in other multi-line conversions, you failed to reindent the
subsequent lines, so indentation is now off.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

[Qemu-devel] [PATCH v8 3/4] qemu-img: Add compare subcommand

[Miroslav Rezanina]

This patch adds new qemu-img subcommand that compares content of two disk
images.

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
---
 qemu-img-cmds.hx |    6 ++
 qemu-img.c       |  266 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 272 insertions(+), 0 deletions(-)

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 90b93e0..bc1ccfa 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -27,6 +27,12 @@ STEXI
 @item commit [-q] [-f @var{fmt}] [-t @var{cache}] @var{filename}
 ETEXI
 
+DEF("compare", img_compare,
+    "compare [-f fmt] [-F fmt] [-p] [-q] [-s] filename1 filename2")
+STEXI
+@item compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-q] [-s] @var{filename1} @var{filename2}
+ETEXI
+
 DEF("convert", img_convert,
     "convert [-c] [-p] [-q] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s snapshot_name] [-S sparse_size] filename [filename2 [...]] output_filename")
 STEXI
diff --git a/qemu-img.c b/qemu-img.c
index 8d55829..0c12692 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -693,6 +693,272 @@ static int compare_sectors(const uint8_t *buf1, const uint8_t *buf2, int n,
 
 #define IO_BUF_SIZE (2 * 1024 * 1024)
 
+static int64_t sectors_to_bytes(int64_t sectors)
+{
+    return sectors << BDRV_SECTOR_BITS;
+}
+
+static int64_t sectors_to_process(int64_t total, int64_t from)
+{
+    int64_t ret = total - from;
+
+    if (ret > (IO_BUF_SIZE >> BDRV_SECTOR_BITS)) {
+        return IO_BUF_SIZE >> BDRV_SECTOR_BITS;
+    }
+
+    return ret;
+}
+
+/*
+ * Check if passed sectors are empty (not allocated or contain only 0 bytes)
+ *
+ * Returns 0 in case sectors are filled with 0, 1 if sectors contain non-zero
+ * data and negative value on error.
+ *
+ * @param bs:  Driver used for accessing file
+ * @param sect_num: Number of first sector to check
+ * @param sect_count: Number of sectors to check
+ * @param filename: Name of disk file we are checking (logging purpose)
+ * @param buffer: Allocated buffer for storing read data
+ * @param quiet: Flag for quiet mode
+ */
+static int check_empty_sectors(BlockDriverState *bs, int64_t sect_num,
+                               int sect_count, const char *filename,
+                               uint8_t *buffer, bool quiet)
+{
+    int pnum, ret = 0;
+    ret = bdrv_read(bs, sect_num, buffer, sect_count);
+    if (ret < 0) {
+        error_report("Error while reading offset %" PRId64 " of %s: %s",
+                     sectors_to_bytes(sect_num), filename, strerror(-ret));
+        return ret;
+    }
+    ret = is_allocated_sectors(buffer, sect_count, &pnum);
+    if (ret || pnum != sect_count) {
+        qprintf(quiet, "Content mismatch at offset %" PRId64 "!\n",
+                sectors_to_bytes(ret ? sect_num : sect_num + pnum));
+        return 1;
+    }
+
+    return 0;
+}
+
+/*
+ * Compares two images. Exit codes:
+ *
+ * 0 - Images are identical
+ * 1 - Images differ
+ * 2 - Error occurred
+ */
+static int img_compare(int argc, char **argv)
+{
+    const char *fmt1 = NULL, *fmt2 = NULL, *filename1, *filename2;
+    BlockDriverState *bs1, *bs2;
+    int64_t total_sectors1, total_sectors2;
+    uint8_t *buf1 = NULL, *buf2 = NULL;
+    int pnum1, pnum2;
+    int allocated1, allocated2;
+    int ret = 0; /* return value - 0 Ident, 1 Different, 2 Error */
+    int progress = 0, quiet = 0, strict = 0;
+    int64_t total_sectors;
+    int64_t sector_num = 0;
+    int64_t nb_sectors;
+    int c, pnum;
+    uint64_t bs_sectors;
+    uint64_t progress_base;
+
+    for (;;) {
+        c = getopt(argc, argv, "hpf:F:sq");
+        if (c == -1) {
+            break;
+        }
+        switch (c) {
+        case '?':
+        case 'h':
+            help();
+            break;
+        case 'f':
+            fmt1 = optarg;
+            break;
+        case 'F':
+            fmt2 = optarg;
+            break;
+        case 'p':
+            progress = 1;
+            break;
+        case 'q':
+            quiet = 1;
+            break;
+        case 's':
+            strict = 1;
+            break;
+        }
+    }
+
+    /* Progress is not shown in Quiet mode */
+    if (quiet) {
+        progress = 0;
+    }
+
+
+    if (optind > argc - 2) {
+        help();
+    }
+    filename1 = argv[optind++];
+    filename2 = argv[optind++];
+
+    /* Initialize before goto out */
+    qemu_progress_init(progress, 2.0);
+
+    bs1 = bdrv_new_open(filename1, fmt1, BDRV_O_FLAGS, true, quiet);
+    if (!bs1) {
+        error_report("Can't open file %s", filename1);
+        ret = 2;
+        goto out3;
+    }
+
+    bs2 = bdrv_new_open(filename2, fmt2, BDRV_O_FLAGS, true, quiet);
+    if (!bs2) {
+        error_report("Can't open file %s", filename2);
+        ret = 2;
+        goto out2;
+    }
+
+    buf1 = qemu_blockalign(bs1, IO_BUF_SIZE);
+    buf2 = qemu_blockalign(bs2, IO_BUF_SIZE);
+    bdrv_get_geometry(bs1, &bs_sectors);
+    total_sectors1 = bs_sectors;
+    bdrv_get_geometry(bs2, &bs_sectors);
+    total_sectors2 = bs_sectors;
+    total_sectors = MIN(total_sectors1, total_sectors2);
+    progress_base = MAX(total_sectors1, total_sectors2);
+
+    qemu_progress_print(0, 100);
+
+    if (strict && total_sectors1 != total_sectors2) {
+        ret = 1;
+        qprintf(quiet, "Strict mode: Image size mismatch!\n");
+        goto out;
+    }
+
+    for (;;) {
+        nb_sectors = sectors_to_process(total_sectors, sector_num);
+        if (nb_sectors <= 0) {
+            break;
+        }
+        allocated1 = bdrv_is_allocated_above(bs1, NULL, sector_num, nb_sectors,
+                                             &pnum1);
+        allocated2 = bdrv_is_allocated_above(bs2, NULL, sector_num, nb_sectors,
+                                             &pnum2);
+        nb_sectors = MIN(pnum1, pnum2);
+
+        if (allocated1 == allocated2) {
+            if (allocated1) {
+                ret = bdrv_read(bs1, sector_num, buf1, nb_sectors);
+                if (ret < 0) {
+                    error_report("error while reading offset %" PRId64 " of %s:"
+                                 " %s", sectors_to_bytes(sector_num), filename1,
+                                  strerror(-ret));
+                    ret = 2;
+                    goto out;
+                }
+                ret = bdrv_read(bs2, sector_num, buf2, nb_sectors);
+                if (ret < 0) {
+                    error_report("error while reading offset %" PRId64
+                                 " of %s: %s", sectors_to_bytes(sector_num),
+                                 filename2, strerror(-ret));
+                    ret = 2;
+                    goto out;
+                }
+                ret = compare_sectors(buf1, buf2, nb_sectors, &pnum);
+                if (ret || pnum != nb_sectors) {
+                    ret = 1;
+                    qprintf(quiet, "Content mismatch at offset %" PRId64 "!\n",
+                            sectors_to_bytes(
+                                ret ? sector_num : sector_num + pnum));
+                    goto out;
+                }
+            }
+        } else {
+            if (strict) {
+                ret = 1;
+                qprintf(quiet, "Strict mode: Offset %" PRId64
+                        " allocation mismatch!\n",
+                        sectors_to_bytes(sector_num));
+                goto out;
+            }
+
+            if (allocated1) {
+                ret = check_empty_sectors(bs1, sector_num, nb_sectors,
+                                         filename1, buf1, quiet);
+            } else {
+                ret = check_empty_sectors(bs2, sector_num, nb_sectors,
+                                         filename2, buf1, quiet);
+            }
+            if (ret) {
+                if (ret < 0) {
+                    ret = 2;
+                }
+                goto out;
+            }
+        }
+        sector_num += nb_sectors;
+        qemu_progress_print(((float) nb_sectors / progress_base)*100, 100);
+    }
+
+    if (total_sectors1 != total_sectors2) {
+        BlockDriverState *bs_over;
+        int64_t total_sectors_over;
+        const char *filename_over;
+
+        qprintf(quiet, "Warning: Image size mismatch!\n");
+        if (total_sectors1 > total_sectors2) {
+            total_sectors_over = total_sectors1;
+            bs_over = bs1;
+            filename_over = filename1;
+        } else {
+            total_sectors_over = total_sectors2;
+            bs_over = bs2;
+            filename_over = filename2;
+        }
+
+        for (;;) {
+            nb_sectors = sectors_to_process(total_sectors_over, sector_num);
+            if (nb_sectors <= 0) {
+                break;
+            }
+            ret = bdrv_is_allocated_above(bs_over, NULL, sector_num,
+                                          nb_sectors, &pnum);
+            nb_sectors = pnum;
+            if (ret) {
+                ret = check_empty_sectors(bs_over, sector_num, nb_sectors,
+                                          filename_over, buf1, quiet);
+                if (ret) {
+                    if (ret < 0) {
+                        ret = 2;
+                    }
+                    goto out;
+                }
+            }
+            sector_num += nb_sectors;
+            qemu_progress_print(((float) nb_sectors / progress_base)*100, 100);
+        }
+    }
+
+    qprintf(quiet, "Images are identical.\n");
+    ret = 0;
+
+out:
+    bdrv_delete(bs2);
+    qemu_vfree(buf1);
+    qemu_vfree(buf2);
+out2:
+    bdrv_delete(bs1);
+out3:
+    qemu_progress_end();
+    return ret;
+}
+
 static int img_convert(int argc, char **argv)
 {
     int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size, cluster_sectors;
-- 
1.7.1

[Qemu-devel] [PATCH v8 4/4] Add qemu-img compare documentation

[Miroslav Rezanina]

Adding documentation for new qemu-img subcommand compare.

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
---
 qemu-img.c    |    7 ++++++-
 qemu-img.texi |   32 ++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 0c12692..6aebdc3 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -103,7 +103,12 @@ static void help(void)
            "  '-a' applies a snapshot (revert disk to saved state)\n"
            "  '-c' creates a snapshot\n"
            "  '-d' deletes a snapshot\n"
-           "  '-l' lists all snapshots in the given image\n";
+           "  '-l' lists all snapshots in the given image\n"
+           "\n"
+           "Parameters to compare subcommand:\n"
+           "  '-f' first image format\n"
+           "  '-F' second image format\n"
+           "  '-s' run in Strict mode - fail on different image size or sector allocation\n";
 
     printf("%s\nSupported formats:", help_msg);
     bdrv_iterate_format(format_print, NULL);
diff --git a/qemu-img.texi b/qemu-img.texi
index 4fdb19a..90f581a 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -84,6 +84,18 @@ deletes a snapshot
 lists all snapshots in the given image
 @end table
 
+Parameters to compare subcommand:
+
+@table @option
+
+@item -f
+First image format
+@item -F
+Second image format
+@item -s
+Strict mode - fail on on different image size or sector allocation
+@end table
+
 Command description:
 
 @table @option
@@ -117,6 +129,26 @@ it doesn't need to be specified separately in this case.
 
 Commit the changes recorded in @var{filename} in its base image.
 
+@item compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-s] [-q] @var{filename1} @var{filename2}
+
+Check if two images have the same content. You can compare images with
+different format or settings.
+
+The format is probed unless you specify it by @var{-f} (used for @var{filename1}) and/or @var{-F} (used for @var{filename2}) option.
+
+By default, images with different size are considered identical if the larger
+image contains only unallocated and/or zeroed sectors in the area after the end
+of the other image. In addition, if any sector is not allocated in one image
+and contains only zero bytes in the second one, it is evaluated as equal. You
+can use Strict mode by specifying the @var{-s} option. When compare runs in
+Strict mode, it fails in case image size differs or a sector is allocated in
+one image and is not allocated in the second one.
+
+By default, compare prints out a result message. This message displays
+information that both images are same or the position of the first different
+byte. In addition, result message can report different image size in case
+Strict mode is used.
+
 @item convert [-c] [-p] [-f @var{fmt}] [-t @var{cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
 
 Convert the disk image @var{filename} or a snapshot @var{snapshot_name} to disk image @var{output_filename}
-- 
1.7.1

Re: [Qemu-devel] [PATCH v8 4/4] Add qemu-img compare documentation

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 888 bytes --]

On 01/14/2013 03:26 AM, Miroslav Rezanina wrote:
> Adding documentation for new qemu-img subcommand compare.
> 
> Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
> ---
>  qemu-img.c    |    7 ++++++-
>  qemu-img.texi |   32 ++++++++++++++++++++++++++++++++
>  2 files changed, 38 insertions(+), 1 deletions(-)

Your series is lacking a test case; it would be useful to test both
identical images, images that are identical only when not in strict
mode, and differing images.

> +Check if two images have the same content. You can compare images with
> +different format or settings.
> +
> +The format is probed unless you specify it by @var{-f} (used for @var{filename1}) and/or @var{-F} (used for @var{filename2}) option.

Long line; worth wrapping.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]