Patchwork [v3] Add compare subcommand for qemu-img

login
register
mail settings
Submitter Miroslav Rezanina
Date Nov. 21, 2012, 9:50 a.m.
Message ID <1090097436.3721567.1353491414263.JavaMail.root@redhat.com>
Download mbox | patch
Permalink /patch/200619/
State New
Headers show

Comments

Miroslav Rezanina - Nov. 21, 2012, 9:50 a.m.
This is second 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

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 handlig 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
Stefan Hajnoczi - Nov. 22, 2012, 9:18 a.m.
On Wed, Nov 21, 2012 at 04:50:14AM -0500, Miroslav Rezanina wrote:
> diff --git a/qemu-img.c b/qemu-img.c
> index e29e01b..6294b11 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -101,7 +101,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"

Please add "\n" to separate like the other parameter docs for
subcommands.

> +           "Parameters to compare subcommand:\n"
> +           "  '-f' First image format\n"
> +           "  '-F' Second image format\n"
> +           "  '-q' Quiet mode - do not print any output (except errors)\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);
> @@ -657,6 +662,277 @@ static int compare_sectors(const uint8_t *buf1, const uint8_t *buf2, int n,
>  }
>  
>  #define IO_BUF_SIZE (2 * 1024 * 1024)
> +#define sector_to_bytes(sector) ((sector) << BDRV_SECTOR_BITS)

No macros, please.  Hiding the sector conversion isn't consistent anyway
since further down in your patch you explicitly use >> BDRV_SECTOR_BITS.

Either open code or define static inline functions so we have type
information.

> +
> +/*
> + * Get number of sectors that can be stored in IO buffer.
> + */
> +
> +static int64_t sectors_to_process(int64_t total, int64_t from)

Doc comments fit snuggly against the function definition, no newline
please.  QEMU code isn't very consistent in doc comment formatting in
general but it does not use a newline here.

> +    for (;;) {
> +        c = getopt(argc, argv, "pf:F:sq");
> +        if (c == -1) {
> +            break;
> +        }
> +        switch (c) {
> +        case 'f':
> +            fmt1 = optarg;
> +            break;
> +        case 'F':
> +            fmt2 = optarg;
> +            break;
> +        case 'p':
> +            progress = (quiet == 0) ? 1 : 0;
> +            break;
> +        case 'q':
> +            quiet = 1;
> +            if (progress == 1) {
> +                progress = 0;
> +            }
> +            break;

It's cleaner to silence progress after all options have been parsed than
to duplicate the quiet/progress checking.

/* -q overrides -p */
if (quiet) {
    progress = 0;
}

> +        case 's':
> +            strict = 1;
> +            break;
> +        }

case 'h':
case '?':
    help();
    break;

> +    }
> +    if (optind >= argc) {

This subcommand takes two filenames, so check the number of arguments is
correct:

if (optind + 1 >= argc) {

> +        help();
> +        goto out3;
> +    }
> +    filename1 = argv[optind++];
> +    filename2 = argv[optind++];
> +
> +    /* Initialize before goto out */
> +    qemu_progress_init(progress, 2.0);
> +
> +    bs1 = bdrv_new_open(filename1, fmt1, flags, true);
> +    if (!bs1) {
> +        error_report("Can't open file %s", filename1);
> +        ret = 2;
> +        goto out3;
> +    }
> +
> +    bs2 = bdrv_new_open(filename2, fmt2, flags, true);
> +    if (!bs2) {
> +        error_report("Can't open file %s:", filename2);

Stray ':'?

> +        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 = total_sectors1;
> +    progress_base = total_sectors;
> +
> +    qemu_progress_print(0, 100);
> +
> +    if (total_sectors1 != total_sectors2) {
> +        BlockDriverState *bsover;
> +        int64_t lo_total_sectors, lo_sector_num;
> +        const char *filename_over;
> +
> +        if (strict) {
> +            ret = 1;
> +            if (!quiet) {
> +                printf("Strict mode: Image size mismatch!\n");
> +            }
> +            goto out;
> +        } else {
> +            error_report("Image size mismatch!");
> +        }
> +
> +        if (total_sectors1 > total_sectors2) {
> +            total_sectors = total_sectors2;
> +            lo_total_sectors = total_sectors1;
> +            lo_sector_num = total_sectors2;
> +            bsover = bs1;
> +            filename_over = filename1;
> +        } else {
> +            total_sectors = total_sectors1;
> +            lo_total_sectors = total_sectors2;
> +            lo_sector_num = total_sectors1;
> +            bsover = bs2;
> +            filename_over = filename2;
> +        }
> +
> +        progress_base = lo_total_sectors;
> +
> +        for (;;) {
> +            nb_sectors = sectors_to_process(lo_total_sectors, lo_sector_num);
> +            if (nb_sectors <= 0) {
> +                break;
> +            }
> +            rv = bdrv_is_allocated(bsover, lo_sector_num, nb_sectors, &pnum);

We should error out if a backing image has non-zero data:

rv = bdrv_is_allocated_above(bsover, NULL, lo_sector_num, nb_sectors &pnum);

> +            nb_sectors = pnum;
> +            if (rv) {
> +                rv = bdrv_read(bsover, lo_sector_num, buf1, nb_sectors);
> +                if (rv < 0) {
> +                    error_report("error while reading data offset %" PRId64
> +                                 " of %s: %s", sector_to_bytes(lo_sector_num),
> +                                 filename_over, strerror(-rv));
> +                    ret = 2;
> +                    goto out;
> +                }
> +                rv = is_allocated_sectors(buf1, nb_sectors, &pnum);
> +                if (rv || pnum != nb_sectors) {
> +                    ret = 1;
> +                    if (!quiet) {
> +                        printf("Content mismatch - offset %" PRId64
> +                               " not available in both images!\n",
> +                               sector_to_bytes(
> +                                   rv ? lo_sector_num : lo_sector_num + pnum));
> +                    }
> +                    goto out;
> +                }

This is duplicated from the code below.

Please split this function up into helper functions.  It will make some
of the temporary variables go away too.

Patch

diff --git a/block.c b/block.c
index 854ebd6..fdc8c45 100644
--- a/block.c
+++ b/block.c
@@ -2698,6 +2698,7 @@  int bdrv_has_zero_init(BlockDriverState *bs)
 
 typedef struct BdrvCoIsAllocatedData {
     BlockDriverState *bs;
+    BlockDriverState *base;
     int64_t sector_num;
     int nb_sectors;
     int *pnum;
@@ -2828,6 +2829,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);
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index a181363..c076085 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -27,6 +27,12 @@  STEXI
 @item commit [-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] [-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 e29e01b..6294b11 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -101,7 +101,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"
+           "Parameters to compare subcommand:\n"
+           "  '-f' First image format\n"
+           "  '-F' Second image format\n"
+           "  '-q' Quiet mode - do not print any output (except errors)\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);
@@ -657,6 +662,277 @@  static int compare_sectors(const uint8_t *buf1, const uint8_t *buf2, int n,
 }
 
 #define IO_BUF_SIZE (2 * 1024 * 1024)
+#define sector_to_bytes(sector) ((sector) << BDRV_SECTOR_BITS)
+
+/*
+ * Get number of sectors that can be stored in IO buffer.
+ */
+
+static int64_t sectors_to_process(int64_t total, int64_t from)
+{
+    int64_t rv = total - from;
+
+    if (rv > (IO_BUF_SIZE >> BDRV_SECTOR_BITS)) {
+        return IO_BUF_SIZE >> BDRV_SECTOR_BITS;
+    }
+
+    return rv;
+}
+
+/*
+ * 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 flags = BDRV_O_FLAGS;
+    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, rv, pnum;
+    uint64_t bs_sectors;
+    uint64_t progress_base;
+
+
+    for (;;) {
+        c = getopt(argc, argv, "pf:F:sq");
+        if (c == -1) {
+            break;
+        }
+        switch (c) {
+        case 'f':
+            fmt1 = optarg;
+            break;
+        case 'F':
+            fmt2 = optarg;
+            break;
+        case 'p':
+            progress = (quiet == 0) ? 1 : 0;
+            break;
+        case 'q':
+            quiet = 1;
+            if (progress == 1) {
+                progress = 0;
+            }
+            break;
+        case 's':
+            strict = 1;
+            break;
+        }
+    }
+    if (optind >= argc) {
+        help();
+        goto out3;
+    }
+    filename1 = argv[optind++];
+    filename2 = argv[optind++];
+
+    /* Initialize before goto out */
+    qemu_progress_init(progress, 2.0);
+
+    bs1 = bdrv_new_open(filename1, fmt1, flags, true);
+    if (!bs1) {
+        error_report("Can't open file %s", filename1);
+        ret = 2;
+        goto out3;
+    }
+
+    bs2 = bdrv_new_open(filename2, fmt2, flags, true);
+    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 = total_sectors1;
+    progress_base = total_sectors;
+
+    qemu_progress_print(0, 100);
+
+    if (total_sectors1 != total_sectors2) {
+        BlockDriverState *bsover;
+        int64_t lo_total_sectors, lo_sector_num;
+        const char *filename_over;
+
+        if (strict) {
+            ret = 1;
+            if (!quiet) {
+                printf("Strict mode: Image size mismatch!\n");
+            }
+            goto out;
+        } else {
+            error_report("Image size mismatch!");
+        }
+
+        if (total_sectors1 > total_sectors2) {
+            total_sectors = total_sectors2;
+            lo_total_sectors = total_sectors1;
+            lo_sector_num = total_sectors2;
+            bsover = bs1;
+            filename_over = filename1;
+        } else {
+            total_sectors = total_sectors1;
+            lo_total_sectors = total_sectors2;
+            lo_sector_num = total_sectors1;
+            bsover = bs2;
+            filename_over = filename2;
+        }
+
+        progress_base = lo_total_sectors;
+
+        for (;;) {
+            nb_sectors = sectors_to_process(lo_total_sectors, lo_sector_num);
+            if (nb_sectors <= 0) {
+                break;
+            }
+            rv = bdrv_is_allocated(bsover, lo_sector_num, nb_sectors, &pnum);
+            nb_sectors = pnum;
+            if (rv) {
+                rv = bdrv_read(bsover, lo_sector_num, buf1, nb_sectors);
+                if (rv < 0) {
+                    error_report("error while reading data offset %" PRId64
+                                 " of %s: %s", sector_to_bytes(lo_sector_num),
+                                 filename_over, strerror(-rv));
+                    ret = 2;
+                    goto out;
+                }
+                rv = is_allocated_sectors(buf1, nb_sectors, &pnum);
+                if (rv || pnum != nb_sectors) {
+                    ret = 1;
+                    if (!quiet) {
+                        printf("Content mismatch - offset %" PRId64
+                               " not available in both images!\n",
+                               sector_to_bytes(
+                                   rv ? lo_sector_num : lo_sector_num + pnum));
+                    }
+                    goto out;
+                }
+            }
+            lo_sector_num += nb_sectors;
+            qemu_progress_print(((float) nb_sectors / progress_base)*100, 100);
+        }
+    }
+
+
+    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);
+        if (pnum1 < pnum2) {
+            nb_sectors = pnum1;
+        } else {
+            nb_sectors = pnum2;
+        }
+
+        if (allocated1 == allocated2) {
+            if (allocated1) {
+                rv = bdrv_read(bs1, sector_num, buf1, nb_sectors);
+                if (rv < 0) {
+                    ret = 2;
+                    error_report("error while reading offset %" PRId64 " of %s:"
+                                 " %s", sector_to_bytes(sector_num), filename1,
+                                  strerror(-rv));
+                    goto out;
+                }
+                rv = bdrv_read(bs2, sector_num, buf2, nb_sectors);
+                if (rv < 0) {
+                    ret = 2;
+                    error_report("error while reading offset %" PRId64
+                                 " of %s: %s", sector_to_bytes(sector_num),
+                                 filename2, strerror(-rv));
+                    goto out;
+                }
+                rv = compare_sectors(buf1, buf2, nb_sectors, &pnum);
+                if (rv || pnum != nb_sectors) {
+                    ret = 1;
+                    if (!quiet) {
+                        printf("Content mismatch at offset %" PRId64 "!\n",
+                               sector_to_bytes(
+                                   rv ? sector_num : sector_num + pnum));
+                    }
+                    goto out;
+                }
+            }
+        } else {
+            BlockDriverState *bstmp;
+            const char *filenametmp;
+
+            if (strict) {
+                ret = 1;
+                if (!quiet) {
+                    printf("Strict mode: Offset %" PRId64
+                           " allocation mismatch!",
+                           sector_to_bytes(sector_num));
+                }
+                goto out;
+            }
+
+            if (allocated1) {
+                bstmp = bs1;
+                filenametmp = filename1;
+            } else {
+                bstmp = bs2;
+                filenametmp = filename2;
+            }
+            rv = bdrv_read(bstmp, sector_num, buf1, nb_sectors);
+            if (rv < 0) {
+                ret = 2;
+                error_report("error while reading offset %" PRId64 " of %s: %s",
+                                sector_to_bytes(sector_num), filenametmp,
+                                strerror(-rv));
+                goto out;
+            }
+            rv = is_allocated_sectors(buf1, nb_sectors, &pnum);
+            if (rv || pnum != nb_sectors) {
+                ret = 1;
+                if (!quiet) {
+                    printf("Content mismatch at offset %" PRId64 "!\n",
+                           sector_to_bytes(
+                               rv ? sector_num : sector_num + pnum));
+                }
+                goto out;
+            }
+        }
+        sector_num += nb_sectors;
+        qemu_progress_print(((float) nb_sectors / progress_base)*100, 100);
+    }
+    if (!quiet) {
+        printf("Images are identical.\n");
+    }
+
+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)
 {
diff --git a/qemu-img.texi b/qemu-img.texi
index 60b83fc..cc54653 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -81,6 +81,20 @@  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 -q
+Quiet mode - do not print any output (except errors)
+@item -s
+Strict mode - fail on on different image size or sector allocation
+@end table
+
 Command description:
 
 @table @option
@@ -114,6 +128,28 @@  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 contains same content. You can compare images with
+different format or settings.
+
+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, compare evaluates as identical images with different size where
+bigger image contains only unallocated and/or zeroed sectors in area above
+second image size. In addition, if any sector is not allocated in one image
+and contains only zero bytes in second, it is evaluated as equal. You can use
+Strict mode by specifying @var{-s} option. When compare runs in Strict mode,
+it fails in case image size differs or sector is allocated in one image and
+is not allocated in second.
+
+By default, compare prints out result message. This message displays
+information that both images are same or position of first different byte.
+In addition, result message can report different image size in case Strict
+mode is used.
+In case you want to suppress any non-error output, you can use Quiet mode by
+specifying @var{-q} option.
+
 @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}