| Message ID | 20211028102441.1878668-4-vsementsov@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qemu-img compare --stat | expand |
On Thu, Oct 28, 2021 at 12:24:40PM +0200, Vladimir Sementsov-Ogievskiy wrote: > Allow compare only top images of backing chains. This is useful to Allow the comparison of only the top image of backing chains. > compare images with same backing file or to compare incremental images > from the chain of incremental backups with "--stat" option. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > Reviewed-by: Hanna Reitz <hreitz@redhat.com> > --- > docs/tools/qemu-img.rst | 9 ++++++++- > qemu-img.c | 8 ++++++-- > qemu-img-cmds.hx | 4 ++-- > 3 files changed, 16 insertions(+), 5 deletions(-) > > diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst > index 9bfdd93d6c..c6e9306c70 100644 > --- a/docs/tools/qemu-img.rst > +++ b/docs/tools/qemu-img.rst > @@ -176,6 +176,13 @@ Parameters to compare subcommand: > - If both files don't specify cluster-size, use default of 64K > - If only one file specifies cluster-size, just use that. > > +.. option:: --shallow > + > + This option prevents opening and comparing any backing files. > + This is useful to compare images with same backing file or to compare > + incremental images from the chain of incremental backups with > + ``--stat`` option. If I understand correctly, your implementation makes --shallow an all-or-none option (either both images are compared shallow, or neither is). Does it make sense to make it a per-image option (--shallow-source, --shallow-dest), where --shallow is a synonym for the more verbose --shallow-source --shallow-dest?
29.10.2021 23:44, Eric Blake wrote: > On Thu, Oct 28, 2021 at 12:24:40PM +0200, Vladimir Sementsov-Ogievskiy wrote: >> Allow compare only top images of backing chains. This is useful to > > Allow the comparison of only the top image of backing chains. > >> compare images with same backing file or to compare incremental images >> from the chain of incremental backups with "--stat" option. >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> Reviewed-by: Hanna Reitz <hreitz@redhat.com> >> --- >> docs/tools/qemu-img.rst | 9 ++++++++- >> qemu-img.c | 8 ++++++-- >> qemu-img-cmds.hx | 4 ++-- >> 3 files changed, 16 insertions(+), 5 deletions(-) >> >> diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst >> index 9bfdd93d6c..c6e9306c70 100644 >> --- a/docs/tools/qemu-img.rst >> +++ b/docs/tools/qemu-img.rst >> @@ -176,6 +176,13 @@ Parameters to compare subcommand: >> - If both files don't specify cluster-size, use default of 64K >> - If only one file specifies cluster-size, just use that. >> >> +.. option:: --shallow >> + >> + This option prevents opening and comparing any backing files. >> + This is useful to compare images with same backing file or to compare >> + incremental images from the chain of incremental backups with >> + ``--stat`` option. > > If I understand correctly, your implementation makes --shallow an > all-or-none option (either both images are compared shallow, or > neither is). Does it make sense to make it a per-image option > (--shallow-source, --shallow-dest), where --shallow is a synonym for > the more verbose --shallow-source --shallow-dest? > Usable, to compare incremental image with "everything below it". But I'm not sure about source/dest terms in context of compare, where image roles are symmetrical. I even start to thing, that it should be an option, used together with --image-opts, applied to image in person.. And actually we already have it, like --image-opts ... driver=qcow2,file.filename=file.qcow2,backing= At least this works with qemu-io. But prints warning: >> qemu-io: warning: Use of "backing": "" is deprecated; use "backing": null instead And of course, "null" doesn't work here. May be we should drop the warning, keeping backing= syntax at least for --image-opts of qemu utilities. Or, another way, support json for --image-opts. What do you think? driver=qcow2,file.filename=a.qcow2,backing= doesn't look better than --shallow. But if we want to make shallow option per-image, we can't ignore the fact that we already have a way to specify per-image options.
diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst index 9bfdd93d6c..c6e9306c70 100644 --- a/docs/tools/qemu-img.rst +++ b/docs/tools/qemu-img.rst @@ -176,6 +176,13 @@ Parameters to compare subcommand: - If both files don't specify cluster-size, use default of 64K - If only one file specifies cluster-size, just use that. +.. option:: --shallow + + This option prevents opening and comparing any backing files. + This is useful to compare images with same backing file or to compare + incremental images from the chain of incremental backups with + ``--stat`` option. + Parameters to convert subcommand: .. program:: qemu-img-convert @@ -395,7 +402,7 @@ Command description: The rate limit for the commit process is specified by ``-r``. -.. option:: compare [--object OBJECTDEF] [--image-opts] [-f FMT] [-F FMT] [-T SRC_CACHE] [-p] [-q] [-s] [-U] [--stat [--block-size BLOCK_SIZE]] FILENAME1 FILENAME2 +.. option:: compare [--object OBJECTDEF] [--image-opts] [-f FMT] [-F FMT] [-T SRC_CACHE] [-p] [-q] [-s] [-U] [--stat [--block-size BLOCK_SIZE]] [--shallow] FILENAME1 FILENAME2 Check if two images have the same content. You can compare images with different format or settings. diff --git a/qemu-img.c b/qemu-img.c index 905150671f..b1cef4b7d1 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -85,6 +85,7 @@ enum { OPTION_SKIP_BROKEN = 277, OPTION_STAT = 278, OPTION_BLOCK_SIZE = 279, + OPTION_SHALLOW = 280, }; typedef enum OutputFormat { @@ -1483,7 +1484,7 @@ static int img_compare(int argc, char **argv) int64_t block_end; int ret = 0; /* return value - 0 Ident, 1 Different, >1 Error */ bool progress = false, quiet = false, strict = false; - int flags; + int flags = 0; bool writethrough; int64_t total_size; int64_t offset = 0; @@ -1504,6 +1505,7 @@ static int img_compare(int argc, char **argv) {"force-share", no_argument, 0, 'U'}, {"stat", no_argument, 0, OPTION_STAT}, {"block-size", required_argument, 0, OPTION_BLOCK_SIZE}, + {"shallow", no_argument, 0, OPTION_SHALLOW}, {0, 0, 0, 0} }; c = getopt_long(argc, argv, ":hf:F:T:pqsU", @@ -1569,6 +1571,9 @@ static int img_compare(int argc, char **argv) exit(2); } break; + case OPTION_SHALLOW: + flags |= BDRV_O_NO_BACKING; + break; } } @@ -1599,7 +1604,6 @@ static int img_compare(int argc, char **argv) /* Initialize before goto out */ qemu_progress_init(progress, 2.0); - flags = 0; ret = bdrv_parse_cache_mode(cache, &flags, &writethrough); if (ret < 0) { error_report("Invalid source cache option: %s", cache); diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx index 96a193eea8..6b164767fd 100644 --- a/qemu-img-cmds.hx +++ b/qemu-img-cmds.hx @@ -40,9 +40,9 @@ SRST ERST DEF("compare", img_compare, - "compare [--object objectdef] [--image-opts] [-f fmt] [-F fmt] [-T src_cache] [-p] [-q] [-s] [-U] [--stat [--block-size BLOCK_SIZE]] filename1 filename2") + "compare [--object objectdef] [--image-opts] [-f fmt] [-F fmt] [-T src_cache] [-p] [-q] [-s] [-U] [--stat [--block-size BLOCK_SIZE]] [--shallow] filename1 filename2") SRST -.. option:: compare [--object OBJECTDEF] [--image-opts] [-f FMT] [-F FMT] [-T SRC_CACHE] [-p] [-q] [-s] [-U] [--stat [--block-size BLOCK_SIZE]] FILENAME1 FILENAME2 +.. option:: compare [--object OBJECTDEF] [--image-opts] [-f FMT] [-F FMT] [-T SRC_CACHE] [-p] [-q] [-s] [-U] [--stat [--block-size BLOCK_SIZE]] [--shallow] FILENAME1 FILENAME2 ERST DEF("convert", img_convert,