Re: [PATCH v3 3/4] qemu-img: add --shallow option for qemu-img compare

[Vladimir Sementsov-Ogievskiy]

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.


--
Best regards,
Vladimir