Hi, we needed these changes when we had to build a guest image compatible with a starting guest image but not backed by it in any way? We needed some tool to check our progress, comparing original and? rebuilt (from scratch) images, and virt-diff seemed the best option, but? we had to soften the comparison to reduce the noise in the output. I added some options to ignore certain informations when comparing. I built and tested it, and now for example, one can ignore file permission with --no-compare-perms, and still see permissions shown when files differ by other criterias.
Gabriele Cerami
2015-Jan-02 22:57 UTC
[Libguestfs] [PATCH] virt-diff: add additional ignore options
added:
--no-compare-xattrs
--no-compare-extra-stats
--no-compare-perms
--no-compare-uids
--no-compare-times
to ignore specified files informations when comparing.
The current strategy to disable comparison on file informations is to
flatten data structure so they return the same 0/NULL value on
comparison and be, in fact, ignored to determine if the files differ.
This patch preserve original information on copies. Comparison is still
made on flattened structures, but preserved structure are used when
showing file informations
---
diff/diff.c | 81 +++++++++++++++++++++++++++++++++++++++++++-----------
diff/virt-diff.pod | 12 ++++++++
2 files changed, 77 insertions(+), 16 deletions(-)
diff --git a/diff/diff.c b/diff/diff.c
index 6a374af..426184e 100644
--- a/diff/diff.c
+++ b/diff/diff.c
@@ -66,6 +66,11 @@ static int enable_extra_stats = 0;
static int enable_times = 0;
static int enable_uids = 0;
static int enable_xattrs = 0;
+static int no_compare_xattrs = 0;
+static int no_compare_perms = 0;
+static int no_compare_extra_stats = 0;
+static int no_compare_times = 0;
+static int no_compare_uids = 0;
static int time_t_output = 0;
static int time_relative = 0; /* 1 = seconds, 2 = days */
static const char *checksum = NULL;
@@ -125,6 +130,11 @@ usage (int status)
" -V|--version Display version and exit\n"
" -x Trace libguestfs API calls\n"
" --xattrs Display extended attributes\n"
+ " --no-compare-xattrs Do not compare extended
attributes\n"
+ " --no-compare-extra-stats Do not compare stats\n"
+ " --no-compare-perms Do not compare permissions\n"
+ " --no-compare-uids Do not compare uid and gid\n"
+ " --no-compare-times Do not compare times\n"
"For more information, see the manpage %s(1).\n"),
program_name, program_name, program_name,
program_name);
@@ -179,6 +189,11 @@ main (int argc, char *argv[])
{ "version", 0, 0, 'V' },
{ "xattr", 0, 0, 0 },
{ "xattrs", 0, 0, 0 },
+ { "no-compare-xattrs", 0, 0, 0 },
+ { "no-compare-perms", 0, 0, 0 },
+ { "no-compare-uids", 0, 0, 0 },
+ { "no-compare-times", 0, 0, 0 },
+ { "no-compare-extra-stats", 0, 0, 0 },
{ 0, 0, 0, 0 }
};
struct drv *drvs = NULL; /* First guest. */
@@ -260,6 +275,16 @@ main (int argc, char *argv[])
} else if (STREQ (long_options[option_index].name, "xattr") ||
STREQ (long_options[option_index].name, "xattrs")) {
enable_xattrs = 1;
+ } else if (STREQ (long_options[option_index].name,
"no-compare-xattrs")) {
+ no_compare_xattrs = 1;
+ } else if (STREQ (long_options[option_index].name,
"no-compare-perms")) {
+ no_compare_perms = 1;
+ } else if (STREQ (long_options[option_index].name,
"no-compare-uids")) {
+ no_compare_uids = 1;
+ } else if (STREQ (long_options[option_index].name,
"no-compare-times")) {
+ no_compare_times = 1;
+ } else if (STREQ (long_options[option_index].name,
"no-compare-extra-stats")) {
+ no_compare_extra_stats = 1;
} else {
fprintf (stderr, _("%s: unknown long option: %s (%d)\n"),
program_name, long_options[option_index].name, option_index);
@@ -404,6 +429,8 @@ struct file {
char *path;
struct guestfs_statns *stat;
struct guestfs_xattr_list *xattrs;
+ struct guestfs_statns *stat_orig;
+ struct guestfs_xattr_list *xattrs_orig;
char *csum; /* Checksum. If NULL, use file times and size.
*/
};
@@ -466,6 +493,8 @@ visit_entry (const char *dir, const char *name,
char *path = NULL, *csum = NULL;
struct guestfs_statns *stat = NULL;
struct guestfs_xattr_list *xattrs = NULL;
+ struct guestfs_statns *stat_copy = NULL;
+ struct guestfs_xattr_list *xattrs_copy = NULL;
size_t i;
path = full_path (dir, name);
@@ -474,15 +503,33 @@ visit_entry (const char *dir, const char *name,
* free them after we return.
*/
stat = guestfs_copy_statns (stat_orig);
+ stat_copy = guestfs_copy_statns (stat_orig);
if (stat == NULL) {
perror ("guestfs_copy_stat");
goto error;
}
+ if (no_compare_perms)
+ stat->st_mode &= 0170000;
+
+ if (no_compare_extra_stats)
+ stat->st_dev = stat->st_ino = stat->st_nlink = stat->st_rdev +
stat->st_blocks = 0;
+
+ if (no_compare_uids)
+ stat->st_uid = stat->st_gid = 0;
+
+ if (no_compare_times)
+ stat->st_atime_sec = stat->st_mtime_sec = stat->st_ctime_sec +
stat->st_atime_nsec = stat->st_mtime_nsec = stat->st_ctime_nsec = 0;
+
xattrs = guestfs_copy_xattr_list (xattrs_orig);
+ xattrs_copy = guestfs_copy_xattr_list (xattrs_orig);
if (xattrs == NULL) {
perror ("guestfs_copy_xattr_list");
goto error;
}
+ if (no_compare_xattrs)
+ xattrs->len = 0;
if (checksum && is_reg (stat->st_mode)) {
csum = guestfs_checksum (t->g, checksum, path);
@@ -534,6 +581,8 @@ visit_entry (const char *dir, const char *name,
t->files[i].stat = stat;
t->files[i].xattrs = xattrs;
t->files[i].csum = csum;
+ t->files[i].stat_orig = stat_copy;
+ t->files[i].xattrs_orig = xattrs_copy;
return 0;
@@ -778,29 +827,29 @@ output_file (guestfs_h *g, struct file *file)
filetype = "u";
output_string (filetype);
- output_int64_perms (file->stat->st_mode & 07777);
+ output_int64_perms (file->stat_orig->st_mode & 07777);
- output_int64_size (file->stat->st_size);
+ output_int64_size (file->stat_orig->st_size);
/* Display extra fields when enabled. */
if (enable_uids) {
- output_int64_uid (file->stat->st_uid);
- output_int64_uid (file->stat->st_gid);
+ output_int64_uid (file->stat_orig->st_uid);
+ output_int64_uid (file->stat_orig->st_gid);
}
if (enable_times) {
if (atime)
- output_int64_time (file->stat->st_atime_sec,
file->stat->st_atime_nsec);
- output_int64_time (file->stat->st_mtime_sec,
file->stat->st_mtime_nsec);
- output_int64_time (file->stat->st_ctime_sec,
file->stat->st_ctime_nsec);
+ output_int64_time (file->stat_orig->st_atime_sec,
file->stat_orig->st_atime_nsec);
+ output_int64_time (file->stat_orig->st_mtime_sec,
file->stat_orig->st_mtime_nsec);
+ output_int64_time (file->stat_orig->st_ctime_sec,
file->stat_orig->st_ctime_nsec);
}
if (enable_extra_stats) {
- output_int64_dev (file->stat->st_dev);
- output_int64 (file->stat->st_ino);
- output_int64 (file->stat->st_nlink);
- output_int64_dev (file->stat->st_rdev);
- output_int64 (file->stat->st_blocks);
+ output_int64_dev (file->stat_orig->st_dev);
+ output_int64 (file->stat_orig->st_ino);
+ output_int64 (file->stat_orig->st_nlink);
+ output_int64_dev (file->stat_orig->st_rdev);
+ output_int64 (file->stat_orig->st_blocks);
}
if (file->csum)
@@ -816,10 +865,10 @@ output_file (guestfs_h *g, struct file *file)
}
if (enable_xattrs) {
- for (i = 0; i < file->xattrs->len; ++i) {
- output_string (file->xattrs->val[i].attrname);
- output_binary (file->xattrs->val[i].attrval,
- file->xattrs->val[i].attrval_len);
+ for (i = 0; i < file->xattrs_orig->len; ++i) {
+ output_string (file->xattrs_orig->val[i].attrname);
+ output_binary (file->xattrs_orig->val[i].attrval,
+ file->xattrs_orig->val[i].attrval_len);
}
}
}
diff --git a/diff/virt-diff.pod b/diff/virt-diff.pod
index e1d67f3..4ffb33a 100644
--- a/diff/virt-diff.pod
+++ b/diff/virt-diff.pod
@@ -214,6 +214,18 @@ Enable tracing of libguestfs API calls.
Display extended attributes.
+=item B<--no-compare-xattrs>
+
+=item B<--no-compare-extra-stats>
+
+=item B<--no-compare-perms>
+
+=item B<--no-compare-uids>
+
+=item B<--no-compare-times>
+
+When comparing, do not consider files different if xattrs, extra-stats,
permissions, uid/gid and times differ, respectively
+
=back
=head1 NOTE ABOUT CSV FORMAT
--
1.9.3
Pino Toscano
2015-Jan-05 15:21 UTC
Re: [Libguestfs] [PATCH] virt-diff: add additional ignore options
Hi, In data venerdì 2 gennaio 2015 23:57:43, Gabriele Cerami ha scritto:> added: > --no-compare-xattrs > --no-compare-extra-stats > --no-compare-perms > --no-compare-uids > --no-compare-times > > to ignore specified files informations when comparing. > > The current strategy to disable comparison on file informations is to > flatten data structure so they return the same 0/NULL value on > comparison and be, in fact, ignored to determine if the files differ. > This patch preserve original information on copies. Comparison is still > made on flattened structures, but preserved structure are used when > showing file informationsHm, 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> diff --git a/diff/diff.c b/diff/diff.c > index 6a374af..426184e 100644 > --- a/diff/diff.c > +++ b/diff/diff.c > @@ -66,6 +66,11 @@ static int enable_extra_stats = 0; > static int enable_times = 0; > static int enable_uids = 0; > static int enable_xattrs = 0; > +static int no_compare_xattrs = 0; > +static int no_compare_perms = 0; > +static int no_compare_extra_stats = 0; > +static int no_compare_times = 0; > +static int no_compare_uids = 0;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) ...> static int time_t_output = 0; > static int time_relative = 0; /* 1 = seconds, 2 = days */ > static const char *checksum = NULL; > @@ -125,6 +130,11 @@ usage (int status) > " -V|--version Display version and exit\n" > " -x Trace libguestfs API calls\n" > " --xattrs Display extended attributes\n" > + " --no-compare-xattrs Do not compare extended attributes\n" > + " --no-compare-extra-stats Do not compare stats\n" > + " --no-compare-perms Do not compare permissions\n" > + " --no-compare-uids Do not compare uid and gid\n" > + " --no-compare-times Do not compare times\n" > "For more information, see the manpage %s(1).\n"),Please place them sorted alphabetically (thus after --keys-from-stdin).> program_name, program_name, program_name, > program_name); > @@ -179,6 +189,11 @@ main (int argc, char *argv[]) > { "version", 0, 0, 'V' }, > { "xattr", 0, 0, 0 }, > { "xattrs", 0, 0, 0 }, > + { "no-compare-xattrs", 0, 0, 0 }, > + { "no-compare-perms", 0, 0, 0 }, > + { "no-compare-uids", 0, 0, 0 }, > + { "no-compare-times", 0, 0, 0 }, > + { "no-compare-extra-stats", 0, 0, 0 }, > { 0, 0, 0, 0 }Ditto.> }; > struct drv *drvs = NULL; /* First guest. */ > @@ -260,6 +275,16 @@ main (int argc, char *argv[]) > } else if (STREQ (long_options[option_index].name, "xattr") || > STREQ (long_options[option_index].name, "xattrs")) { > enable_xattrs = 1; > + } else if (STREQ (long_options[option_index].name, "no-compare-xattrs")) { > + no_compare_xattrs = 1; > + } else if (STREQ (long_options[option_index].name, "no-compare-perms")) { > + no_compare_perms = 1; > + } else if (STREQ (long_options[option_index].name, "no-compare-uids")) { > + no_compare_uids = 1; > + } else if (STREQ (long_options[option_index].name, "no-compare-times")) { > + no_compare_times = 1; > + } else if (STREQ (long_options[option_index].name, "no-compare-extra-stats")) { > + no_compare_extra_stats = 1; > } else { > fprintf (stderr, _("%s: unknown long option: %s (%d)\n"), > program_name, long_options[option_index].name, option_index); > @@ -404,6 +429,8 @@ struct file { > char *path; > struct guestfs_statns *stat; > struct guestfs_xattr_list *xattrs; > + struct guestfs_statns *stat_orig; > + struct guestfs_xattr_list *xattrs_orig; > char *csum; /* Checksum. If NULL, use file times and size. */ > }; > > @@ -466,6 +493,8 @@ visit_entry (const char *dir, const char *name, > char *path = NULL, *csum = NULL; > struct guestfs_statns *stat = NULL; > struct guestfs_xattr_list *xattrs = NULL; > + struct guestfs_statns *stat_copy = NULL; > + struct guestfs_xattr_list *xattrs_copy = NULL; > size_t i; > > path = full_path (dir, name); > @@ -474,15 +503,33 @@ visit_entry (const char *dir, const char *name, > * free them after we return. > */ > stat = guestfs_copy_statns (stat_orig); > + stat_copy = guestfs_copy_statns (stat_orig); > if (stat == NULL) { > perror ("guestfs_copy_stat"); > goto error; > } > + if (no_compare_perms) > + stat->st_mode &= 0170000; > + > + if (no_compare_extra_stats) > + stat->st_dev = stat->st_ino = stat->st_nlink = stat->st_rdev > + stat->st_blocks = 0; > + > + if (no_compare_uids) > + stat->st_uid = stat->st_gid = 0; > + > + if (no_compare_times) > + stat->st_atime_sec = stat->st_mtime_sec = stat->st_ctime_sec > + stat->st_atime_nsec = stat->st_mtime_nsec = stat->st_ctime_nsec = 0; > + > xattrs = guestfs_copy_xattr_list (xattrs_orig); > + xattrs_copy = guestfs_copy_xattr_list (xattrs_orig); > if (xattrs == NULL) { > perror ("guestfs_copy_xattr_list"); > goto error; > } > + if (no_compare_xattrs) > + xattrs->len = 0;"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.> if (checksum && is_reg (stat->st_mode)) { > csum = guestfs_checksum (t->g, checksum, path); > @@ -534,6 +581,8 @@ visit_entry (const char *dir, const char *name, > t->files[i].stat = stat; > t->files[i].xattrs = xattrs; > t->files[i].csum = csum; > + t->files[i].stat_orig = stat_copy; > + t->files[i].xattrs_orig = xattrs_copy; >These need to be properly freed in free_tree.> @@ -778,29 +827,29 @@ output_file (guestfs_h *g, struct file *file) > filetype = "u"; > > output_string (filetype); > - output_int64_perms (file->stat->st_mode & 07777); > + output_int64_perms (file->stat_orig->st_mode & 07777); > > - output_int64_size (file->stat->st_size); > + output_int64_size (file->stat_orig->st_size); > > /* Display extra fields when enabled. */ > if (enable_uids) { > - output_int64_uid (file->stat->st_uid); > - output_int64_uid (file->stat->st_gid); > + output_int64_uid (file->stat_orig->st_uid); > + output_int64_uid (file->stat_orig->st_gid); > } > > if (enable_times) { > if (atime) > - output_int64_time (file->stat->st_atime_sec, file->stat->st_atime_nsec); > - output_int64_time (file->stat->st_mtime_sec, file->stat->st_mtime_nsec); > - output_int64_time (file->stat->st_ctime_sec, file->stat->st_ctime_nsec); > + output_int64_time (file->stat_orig->st_atime_sec, file->stat_orig->st_atime_nsec); > + output_int64_time (file->stat_orig->st_mtime_sec, file->stat_orig->st_mtime_nsec); > + output_int64_time (file->stat_orig->st_ctime_sec, file->stat_orig->st_ctime_nsec); > } > > if (enable_extra_stats) { > - output_int64_dev (file->stat->st_dev); > - output_int64 (file->stat->st_ino); > - output_int64 (file->stat->st_nlink); > - output_int64_dev (file->stat->st_rdev); > - output_int64 (file->stat->st_blocks); > + output_int64_dev (file->stat_orig->st_dev); > + output_int64 (file->stat_orig->st_ino); > + output_int64 (file->stat_orig->st_nlink); > + output_int64_dev (file->stat_orig->st_rdev); > + output_int64 (file->stat_orig->st_blocks); > } > > if (file->csum) > @@ -816,10 +865,10 @@ output_file (guestfs_h *g, struct file *file) > } > > if (enable_xattrs) { > - for (i = 0; i < file->xattrs->len; ++i) { > - output_string (file->xattrs->val[i].attrname); > - output_binary (file->xattrs->val[i].attrval, > - file->xattrs->val[i].attrval_len); > + for (i = 0; i < file->xattrs_orig->len; ++i) { > + output_string (file->xattrs_orig->val[i].attrname); > + output_binary (file->xattrs_orig->val[i].attrval, > + file->xattrs_orig->val[i].attrval_len); > } > } > } > diff --git a/diff/virt-diff.pod b/diff/virt-diff.pod > index e1d67f3..4ffb33a 100644 > --- a/diff/virt-diff.pod > +++ b/diff/virt-diff.pod > @@ -214,6 +214,18 @@ Enable tracing of libguestfs API calls. > > Display extended attributes. > > +=item B<--no-compare-xattrs> > + > +=item B<--no-compare-extra-stats> > + > +=item B<--no-compare-perms> > + > +=item B<--no-compare-uids> > + > +=item B<--no-compare-times> > + > +When comparing, do not consider files different if xattrs, extra-stats, permissions, uid/gid and times differ, respectively > +Just document each one separately, even if with a similar documentation snippet; something like: =item B<--no-compare-xattrs> The default is to compare the extended attributes of files. Using this flag ignores the extended attributes when comparing files. Also, as before, place them sorted alphabetically among the other options. Thanks, -- Pino Toscano