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. _______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
