[Libguestfs] [PATCH] virt-diff: add additional ignore options

Gabriele Cerami gcerami at redhat.com
Tue Jan 6 17:50:52 UTC 2015


On 05 Jan, Pino Toscano wrote:
> Hm, I'm a bit dubious whether information that are ignored when
> comparing should be shown -- after all, when not enabling some
> attribute compared by default (e.g. --atime), that attribute is not
> shown if the file differs between images.
> 
> I think it would be better to split this patch in two:
> 
> a) disabling comparison of some attributes (--no-compare-XXX), by just
>    flattening them
> 
> b) an extra parameter, or something else, to show not compared
>    attributes for files that differ

The first draft of this patch disallowed ignoring an information (e.g.
xattrs) and enabling it in output strings. But for example permissions
are always shown in diff, and I wanted definitely to check if permission
were creating a compatibility problem with the original image we wanted
to reproduce.
I can see no harm in showing all the informations even if we are
ignoring them. I think we wanted to see all them anyway  to get
confirmation that things worked they way we intended.

> Instead of "no_compare_XXXX", I'd name them "compare_XXX" defaulting
> to 1, as avoids some mind twisting in expressions like:
>   if (!no_compare_perms) ...

So what the argument should look like when invoked ?
virt-diff --compare-xattrs=0 ? What you suggest should be the name of
the argument to show all the compare=False informations ?

> 
> "len" in _list structs indicates the number of items in the list
> itself, so I'd avoid resetting it to 0 otherwise this information is
> lost.
> Much better to check (no_)compare_xattrs in compare_stats.
> Even better, if the xattrs comparison is off, then just avoid copying
> xattrs_orig.
> 

what is the problem if the information is lost ? there's the same
information in xattrs_orig. Anyway, the second proposal is feasible but
not similar to the strategies for others "ignore flags" (that is flatten)
and for example cannot be done with extra-stats. I can do it, but I did
not considered it because it moves the logic for "ignoring" away from
the other checks.
Regarding the third proposal guestfs_compare_xattr_list and
guestfs_compare_xattr expect the struct not to be NULL, so if I call
those functions, I cannot pass a NULL pointer and setting len to 0 was
the only way to make the function return the value I wanted (0 - not
diff)



Thanks for the suggestion, I'll submit the corrections as soon as I can.




More information about the Libguestfs mailing list