Re: [Libguestfs] [PATCH] virt-diff: add additional ignore options
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.