[PATCH v2 0/7] Change fssum to support POSIX

[PATCH v2 0/7] Change fssum to support POSIX

[Arvind Raghavan]

This patch series modifies fssum to be able to verify POSIX
fsync/fdatasync guarantees. In particular, it allows single
files/symlinks to be passed as input, which allows us to verify
an fsync on a single file without checking the entire directory.

It also adds an option for non-recursive directory traversal,
which is useful because POSIX fsync doesn't guarantee that all
subdirectories are synced. Finally, it adds a flag that allows
file size to be ignored (currently enabled by default). This is
useful because POSIX fsync of a directory doesn't guarantee that
the data in the children is persisted.

These changes will be followed up with several Crashmonkey [1]
tests which make use of these changes.

[1] https://github.com/utsaslab/crashmonkey

Changes since v1:
- Change readlink to readlinkat to fix bug
- Added helper sum_one function to simplify single file input
- Moved fstat -> fstatat change and both of the former into separate
  commit

Manually verified that all of the tests that use fssum still pass for
each commit:

tests/btrfs/083
tests/btrfs/050
tests/btrfs/007
tests/btrfs/016
tests/btrfs/128
tests/btrfs/040
tests/btrfs/092
tests/btrfs/087
tests/btrfs/039
tests/btrfs/127
tests/btrfs/044
tests/btrfs/144
tests/btrfs/168
tests/btrfs/134
tests/btrfs/045
tests/btrfs/129
tests/btrfs/043
tests/btrfs/145
tests/btrfs/191
tests/btrfs/147
tests/btrfs/030
tests/btrfs/077
tests/btrfs/155
tests/btrfs/133
tests/btrfs/038
tests/btrfs/053
tests/btrfs/189
tests/btrfs/084
tests/btrfs/135
tests/btrfs/200
tests/btrfs/178
tests/btrfs/051
tests/generic/474
tests/generic/547

Arvind Raghavan (7):
  src/fssum: Make sum_file_data global
  src/fssum: Refactoring changes for recursive traversal
  src/fssum: Refactor recursive traversal
  src/fssum: Add flag -R for non-recursive mode
  src/fssum: Add a flag for including file size in checksum
  src/fssum: Allow single file input
  src/fssum: Fix whitespace in usage

 src/fssum.c | 354 ++++++++++++++++++++++++++++++----------------------
 1 file changed, 203 insertions(+), 151 deletions(-)

-- 
2.20.1

[PATCH v2 1/7] src/fssum: Make sum_file_data global

[Arvind Raghavan]

Makes deciding which function to use for summing file data happen once
in main instead of for each file.

Signed-off-by: Arvind Raghavan <raghavan.arvind@gmail.com>
Signed-off-by: Jayashree Mohan <jaya@cs.utexas.edu>
Signed-off-by: Vijay Chidambaram <vijay@cs.utexas.edu>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 src/fssum.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/fssum.c b/src/fssum.c
index 3d97a70b..30f456c2 100644
--- a/src/fssum.c
+++ b/src/fssum.c
@@ -62,6 +62,7 @@ int n_excludes = 0;
 int verbose = 0;
 FILE *out_fp;
 FILE *in_fp;
+sum_file_data_t sum_file_data;
 
 enum _flags {
 	FLAG_UID,
@@ -514,8 +515,6 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 	int ret;
 	int fd;
 	int excl;
-	sum_file_data_t sum_file_data = flags[FLAG_STRUCTURE] ?
-			sum_file_data_strict : sum_file_data_permissive;
 	struct stat64 dir_st;
 
 	if (fstat64(dirfd, &dir_st)) {
@@ -824,6 +823,9 @@ main(int argc, char *argv[])
 			flagstring[i] -= 'a' - 'A';
 	}
 
+	sum_file_data = flags[FLAG_STRUCTURE] ?
+			sum_file_data_strict : sum_file_data_permissive;
+
 	path = argv[optind];
 	plen = strlen(path);
 	if (path[plen - 1] == '/') {
-- 
2.20.1

[PATCH v2 2/7] src/fssum: Refactoring changes for recursive traversal

[Arvind Raghavan]

Refactoring changes needed for recursive traversal and single file
input. Makes fstat and readlink use the 'at' alternatives, and creates a
helper function for opening files.

Signed-off-by: Arvind Raghavan <raghavan.arvind@gmail.com>
Signed-off-by: Jayashree Mohan <jaya@cs.utexas.edu>
Signed-off-by: Vijay Chidambaram <vijay@cs.utexas.edu>
---
 src/fssum.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/src/fssum.c b/src/fssum.c
index 30f456c2..135dd60f 100644
--- a/src/fssum.c
+++ b/src/fssum.c
@@ -503,6 +503,13 @@ malformed:
 		excess_file(fn);
 }
 
+int open_one(int dirfd, const char *name)
+{
+	if (!name || !*name)
+		return dup(dirfd);
+	return openat(dirfd, name, 0);
+}
+
 void
 sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 {
@@ -563,12 +570,7 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 				goto next;
 		}
 
-		ret = fchdir(dirfd);
-		if (ret == -1) {
-			perror("fchdir");
-			exit(-1);
-		}
-		ret = lstat64(namelist[i], &st);
+		ret = fstatat64(dirfd, namelist[i], &st, AT_SYMLINK_NOFOLLOW);
 		if (ret) {
 			fprintf(stderr, "stat failed for %s/%s: %s\n",
 				path_prefix, path, strerror(errno));
@@ -597,7 +599,7 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 			sum_add_time(&meta, st.st_ctime);
 		if (flags[FLAG_XATTRS] &&
 		    (S_ISDIR(st.st_mode) || S_ISREG(st.st_mode))) {
-			fd = openat(dirfd, namelist[i], 0);
+			fd = open_one(dirfd, namelist[i]);
 			if (fd == -1 && flags[FLAG_OPEN_ERROR]) {
 				sum_add_u64(&meta, errno);
 			} else if (fd == -1) {
@@ -618,7 +620,7 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 			}
 		}
 		if (S_ISDIR(st.st_mode)) {
-			fd = openat(dirfd, namelist[i], 0);
+			fd = open_one(dirfd, namelist[i]);
 			if (fd == -1 && flags[FLAG_OPEN_ERROR]) {
 				sum_add_u64(&meta, errno);
 			} else if (fd == -1) {
@@ -635,7 +637,7 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 				if (verbose)
 					fprintf(stderr, "file %s\n",
 						namelist[i]);
-				fd = openat(dirfd, namelist[i], 0);
+				fd = open_one(dirfd, namelist[i]);
 				if (fd == -1 && flags[FLAG_OPEN_ERROR]) {
 					sum_add_u64(&meta, errno);
 				} else if (fd == -1) {
@@ -659,7 +661,7 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 				}
 			}
 		} else if (S_ISLNK(st.st_mode)) {
-			ret = readlink(namelist[i], buf, sizeof(buf));
+			ret = readlinkat(dirfd, namelist[i], buf, sizeof(buf));
 			if (ret == -1) {
 				perror("readlink");
 				exit(-1);
-- 
2.20.1

[PATCH v2 3/7] src/fssum: Recursive traversal refactoring

[Arvind Raghavan]

Moves some logic from the recursive directory traversal into a helper
function to make it easier to add support for regular files. Does not
change functionality.

Signed-off-by: Arvind Raghavan <raghavan.arvind@gmail.com>
Signed-off-by: Jayashree Mohan <jaya@cs.utexas.edu>
Signed-off-by: Vijay Chidambaram <vijay@cs.utexas.edu>
---
 src/fssum.c | 257 ++++++++++++++++++++++++++++------------------------
 1 file changed, 139 insertions(+), 118 deletions(-)

diff --git a/src/fssum.c b/src/fssum.c
index 135dd60f..506ca7fe 100644
--- a/src/fssum.c
+++ b/src/fssum.c
@@ -510,6 +510,10 @@ int open_one(int dirfd, const char *name)
 	return openat(dirfd, name, 0);
 }
 
+void
+sum_one(int dirfd, int level, sum_t *dircs, char *path_prefix,
+		char *path_in, char *name);
+
 void
 sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 {
@@ -520,8 +524,6 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 	int entries = 0;
 	int i;
 	int ret;
-	int fd;
-	int excl;
 	struct stat64 dir_st;
 
 	if (fstat64(dirfd, &dir_st)) {
@@ -556,147 +558,166 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 	qsort(namelist, entries, sizeof(*namelist), namecmp);
 	for (i = 0; i < entries; ++i) {
 		struct stat64 st;
-		sum_t cs;
-		sum_t meta;
-		char *path;
-
-		sum_init(&cs);
-		sum_init(&meta);
-		path = alloc(strlen(path_in) + strlen(namelist[i]) + 3);
-		sprintf(path, "%s/%s", path_in, namelist[i]);
-		for (excl = 0; excl < n_excludes; ++excl) {
-			if (strncmp(excludes[excl].path, path,
-			    excludes[excl].len) == 0)
-				goto next;
-		}
 
 		ret = fstatat64(dirfd, namelist[i], &st, AT_SYMLINK_NOFOLLOW);
 		if (ret) {
-			fprintf(stderr, "stat failed for %s/%s: %s\n",
-				path_prefix, path, strerror(errno));
+			fprintf(stderr, "stat failed for %s/%s/%s: %s\n",
+				path_prefix, path_in, namelist[i],
+				strerror(errno));
 			exit(-1);
 		}
 
 		/* We are crossing into a different subvol, skip this subtree. */
 		if (st.st_dev != dir_st.st_dev)
-			goto next;
-
-		sum_add_u64(&meta, level);
-		sum_add(&meta, namelist[i], strlen(namelist[i]));
-		if (!S_ISDIR(st.st_mode))
-			sum_add_u64(&meta, st.st_nlink);
-		if (flags[FLAG_UID])
-			sum_add_u64(&meta, st.st_uid);
-		if (flags[FLAG_GID])
-			sum_add_u64(&meta, st.st_gid);
-		if (flags[FLAG_MODE])
-			sum_add_u64(&meta, st.st_mode);
-		if (flags[FLAG_ATIME])
-			sum_add_time(&meta, st.st_atime);
-		if (flags[FLAG_MTIME])
-			sum_add_time(&meta, st.st_mtime);
-		if (flags[FLAG_CTIME])
-			sum_add_time(&meta, st.st_ctime);
-		if (flags[FLAG_XATTRS] &&
-		    (S_ISDIR(st.st_mode) || S_ISREG(st.st_mode))) {
-			fd = open_one(dirfd, namelist[i]);
-			if (fd == -1 && flags[FLAG_OPEN_ERROR]) {
-				sum_add_u64(&meta, errno);
-			} else if (fd == -1) {
-				fprintf(stderr, "open failed for %s/%s: %s\n",
-					path_prefix, path, strerror(errno));
+			continue;
+
+		sum_one(dirfd, level, dircs, path_prefix, path_in, namelist[i]);
+	}
+}
+
+void
+sum_one(int dirfd, int level, sum_t *dircs, char *path_prefix,
+		char *path_in, char *name) {
+	sum_t cs;
+	sum_t meta;
+	int fd;
+	int ret;
+	int excl;
+	char* path;
+	struct stat64 st;
+
+	sum_init(&cs);
+	sum_init(&meta);
+	path = alloc(strlen(path_in) + strlen(name) + 3);
+	sprintf(path, "%s/%s", path_in, name);
+	for (excl = 0; excl < n_excludes; ++excl) {
+		if (strncmp(excludes[excl].path, path,
+			excludes[excl].len) == 0)
+			goto out;
+	}
+
+	ret = fstatat64(dirfd, name, &st, AT_SYMLINK_NOFOLLOW);
+	if (ret) {
+		fprintf(stderr, "stat failed for %s/%s: %s\n",
+			path_prefix, path, strerror(errno));
+		exit(-1);
+	}
+
+	sum_add_u64(&meta, level);
+	sum_add(&meta, name, strlen(name));
+	if (!S_ISDIR(st.st_mode))
+		sum_add_u64(&meta, st.st_nlink);
+	if (flags[FLAG_UID])
+		sum_add_u64(&meta, st.st_uid);
+	if (flags[FLAG_GID])
+		sum_add_u64(&meta, st.st_gid);
+	if (flags[FLAG_MODE])
+		sum_add_u64(&meta, st.st_mode);
+	if (flags[FLAG_ATIME])
+		sum_add_time(&meta, st.st_atime);
+	if (flags[FLAG_MTIME])
+		sum_add_time(&meta, st.st_mtime);
+	if (flags[FLAG_CTIME])
+		sum_add_time(&meta, st.st_ctime);
+	if (flags[FLAG_XATTRS] &&
+		(S_ISDIR(st.st_mode) || S_ISREG(st.st_mode))) {
+		fd = open_one(dirfd, name);
+		if (fd == -1 && flags[FLAG_OPEN_ERROR]) {
+			sum_add_u64(&meta, errno);
+		} else if (fd == -1) {
+			fprintf(stderr, "open failed for %s/%s: %s\n",
+				path_prefix, path, strerror(errno));
+			exit(-1);
+		} else {
+			ret = sum_xattrs(fd, &meta);
+			close(fd);
+			if (ret < 0) {
+				fprintf(stderr,
+					"failed to read xattrs from "
+					"%s/%s: %s\n",
+					path_prefix, path,
+					strerror(-ret));
 				exit(-1);
-			} else {
-				ret = sum_xattrs(fd, &meta);
-				close(fd);
-				if (ret < 0) {
-					fprintf(stderr,
-						"failed to read xattrs from "
-						"%s/%s: %s\n",
-						path_prefix, path,
-						strerror(-ret));
-					exit(-1);
-				}
 			}
 		}
-		if (S_ISDIR(st.st_mode)) {
-			fd = open_one(dirfd, namelist[i]);
+	}
+	if (S_ISDIR(st.st_mode)) {
+		fd = open_one(dirfd, name);
+		if (fd == -1 && flags[FLAG_OPEN_ERROR]) {
+			sum_add_u64(&meta, errno);
+		} else if (fd == -1) {
+			fprintf(stderr, "open failed for %s/%s: %s\n",
+				path_prefix, path, strerror(errno));
+			exit(-1);
+		} else {
+			sum(fd, level + 1, &cs, path_prefix, path);
+			close(fd);
+		}
+	} else if (S_ISREG(st.st_mode)) {
+		sum_add_u64(&meta, st.st_size);
+		if (flags[FLAG_DATA]) {
+			if (verbose)
+				fprintf(stderr, "file %s\n",
+					name);
+			fd = open_one(dirfd, name);
 			if (fd == -1 && flags[FLAG_OPEN_ERROR]) {
 				sum_add_u64(&meta, errno);
 			} else if (fd == -1) {
-				fprintf(stderr, "open failed for %s/%s: %s\n",
-					path_prefix, path, strerror(errno));
+				fprintf(stderr,
+					"open failed for %s/%s: %s\n",
+					path_prefix, path,
+					strerror(errno));
 				exit(-1);
-			} else {
-				sum(fd, level + 1, &cs, path_prefix, path);
-				close(fd);
 			}
-		} else if (S_ISREG(st.st_mode)) {
-			sum_add_u64(&meta, st.st_size);
-			if (flags[FLAG_DATA]) {
-				if (verbose)
-					fprintf(stderr, "file %s\n",
-						namelist[i]);
-				fd = open_one(dirfd, namelist[i]);
-				if (fd == -1 && flags[FLAG_OPEN_ERROR]) {
-					sum_add_u64(&meta, errno);
-				} else if (fd == -1) {
+			if (fd != -1) {
+				ret = sum_file_data(fd, &cs);
+				if (ret < 0) {
 					fprintf(stderr,
-						"open failed for %s/%s: %s\n",
+						"read failed for "
+						"%s/%s: %s\n",
 						path_prefix, path,
 						strerror(errno));
 					exit(-1);
 				}
-				if (fd != -1) {
-					ret = sum_file_data(fd, &cs);
-					if (ret < 0) {
-						fprintf(stderr,
-							"read failed for "
-							"%s/%s: %s\n",
-							path_prefix, path,
-							strerror(errno));
-						exit(-1);
-					}
-					close(fd);
-				}
-			}
-		} else if (S_ISLNK(st.st_mode)) {
-			ret = readlinkat(dirfd, namelist[i], buf, sizeof(buf));
-			if (ret == -1) {
-				perror("readlink");
-				exit(-1);
+				close(fd);
 			}
-			sum_add(&cs, buf, ret);
-		} else if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) {
-			sum_add_u64(&cs, major(st.st_rdev));
-			sum_add_u64(&cs, minor(st.st_rdev));
 		}
-		sum_fini(&cs);
-		sum_fini(&meta);
-		if (gen_manifest || in_manifest) {
-			char *fn;
-			char *m;
-			char *c;
-
-			if (S_ISDIR(st.st_mode))
-				strcat(path, "/");
-			fn = escape(path);
-			m = sum_to_string(&meta);
-			c = sum_to_string(&cs);
-
-			if (gen_manifest)
-				fprintf(out_fp, "%s %s %s\n", fn, m, c);
-			if (in_manifest)
-				check_manifest(fn, m, c, 0);
-			free(c);
-			free(m);
-			free(fn);
+	} else if (S_ISLNK(st.st_mode)) {
+		ret = readlinkat(dirfd, name, buf, sizeof(buf));
+		if (ret == -1) {
+			perror("readlink");
+			exit(-1);
 		}
-		sum_add_sum(dircs, &cs);
-		sum_add_sum(dircs, &meta);
-next:
-		free(path);
+		sum_add(&cs, buf, ret);
+	} else if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) {
+		sum_add_u64(&cs, major(st.st_rdev));
+		sum_add_u64(&cs, minor(st.st_rdev));
 	}
+	sum_fini(&cs);
+	sum_fini(&meta);
+	if (gen_manifest || in_manifest) {
+		char *fn;
+		char *m;
+		char *c;
+
+		if (S_ISDIR(st.st_mode))
+			strcat(path, "/");
+		fn = escape(path);
+		m = sum_to_string(&meta);
+		c = sum_to_string(&cs);
+
+		if (gen_manifest)
+			fprintf(out_fp, "%s %s %s\n", fn, m, c);
+		if (in_manifest)
+			check_manifest(fn, m, c, 0);
+		free(c);
+		free(m);
+		free(fn);
+	}
+	sum_add_sum(dircs, &cs);
+	sum_add_sum(dircs, &meta);
+out:
+	free(path);
 }
 
 int
-- 
2.20.1

[PATCH v2 4/7] src/fssum: Add flag -R for non-recursive mode

[Arvind Raghavan]

By default, fssum walks the input directory recursively. This patch adds
a non-recursive mode which is useful because POSIX standards dictate
that fsyncing a directory only guarantees its immediate dirents are
synced.

Signed-off-by: Arvind Raghavan <raghavan.arvind@gmail.com>
Signed-off-by: Jayashree Mohan <jaya@cs.utexas.edu>
Signed-off-by: Vijay Chidambaram <vijay@cs.utexas.edu>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 src/fssum.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/fssum.c b/src/fssum.c
index 506ca7fe..3667ec2f 100644
--- a/src/fssum.c
+++ b/src/fssum.c
@@ -56,6 +56,7 @@ typedef int (*sum_file_data_t)(int fd, sum_t *dst);
 
 int gen_manifest = 0;
 int in_manifest = 0;
+int recursive = 1;
 char *checksum = NULL;
 struct excludes *excludes;
 int n_excludes = 0;
@@ -151,6 +152,7 @@ usage(void)
 	fprintf(stderr, "    -n           : reset all flags\n");
 	fprintf(stderr, "    -N           : set all flags\n");
 	fprintf(stderr, "    -x path      : exclude path when building checksum (multiple ok)\n");
+	fprintf(stderr, "    -R           : traverse dirs non-recursively (recursive is default)\n");
 	fprintf(stderr, "    -h           : this help\n\n");
 	fprintf(stderr, "The default field mask is ugoamCdtES. If the checksum/manifest is read from a\n");
 	fprintf(stderr, "file, the mask is taken from there and the values given on the command line\n");
@@ -641,7 +643,7 @@ sum_one(int dirfd, int level, sum_t *dircs, char *path_prefix,
 			}
 		}
 	}
-	if (S_ISDIR(st.st_mode)) {
+	if (S_ISDIR(st.st_mode) && recursive) {
 		fd = open_one(dirfd, name);
 		if (fd == -1 && flags[FLAG_OPEN_ERROR]) {
 			sum_add_u64(&meta, errno);
@@ -734,7 +736,7 @@ main(int argc, char *argv[])
 	int plen;
 	int elen;
 	int n_flags = 0;
-	const char *allopts = "heEfuUgGoOaAmMcCdDtTsSnNw:r:vx:";
+	const char *allopts = "heEfuUgGoOaAmMcCdDtTsSnNRw:r:vx:";
 
 	out_fp = stdout;
 	while ((c = getopt(argc, argv, allopts)) != EOF) {
@@ -742,6 +744,9 @@ main(int argc, char *argv[])
 		case 'f':
 			gen_manifest = 1;
 			break;
+		case 'R':
+			recursive = 0;
+			break;
 		case 'u':
 		case 'U':
 		case 'g':
-- 
2.20.1

[PATCH v2 5/7] src/fssum: Add a flag for including file size in checksum

[Arvind Raghavan]

File size is currently included by default. It is useful to ignore file
sizes when testing the fsync of a directory, as the data in children is
not guaranteed to be persisted.

Signed-off-by: Arvind Raghavan <raghavan.arvind@gmail.com>
Signed-off-by: Jayashree Mohan <jaya@cs.utexas.edu>
Signed-off-by: Vijay Chidambaram <vijay@cs.utexas.edu>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 src/fssum.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/src/fssum.c b/src/fssum.c
index 3667ec2f..26b0096c 100644
--- a/src/fssum.c
+++ b/src/fssum.c
@@ -76,13 +76,14 @@ enum _flags {
 	FLAG_XATTRS,
 	FLAG_OPEN_ERROR,
 	FLAG_STRUCTURE,
+	FLAG_SIZE,
 	NUM_FLAGS
 };
 
-const char flchar[] = "ugoamcdtes";
+const char flchar[] = "ugoamcdtesz";
 char line[65536];
 
-int flags[NUM_FLAGS] = {1, 1, 1, 1, 1, 0, 1, 1, 0, 0};
+int flags[NUM_FLAGS] = {1, 1, 1, 1, 1, 0, 1, 1, 0, 0, 1};
 
 char *
 getln(char *buf, int size, FILE *fp)
@@ -137,7 +138,7 @@ usage(void)
 	fprintf(stderr, "    -w <file>    : send output to file\n");
 	fprintf(stderr, "    -v           : verbose mode (debugging only)\n");
 	fprintf(stderr, "    -r <file>    : read checksum or manifest from file\n");
-	fprintf(stderr, "    -[ugoamcdtes]: specify which fields to include in checksum calculation.\n");
+	fprintf(stderr, "    -[ugoamcdtesz]: specify which fields to include in checksum calculation.\n");
 	fprintf(stderr, "         u       : include uid\n");
 	fprintf(stderr, "         g       : include gid\n");
 	fprintf(stderr, "         o       : include mode\n");
@@ -148,13 +149,14 @@ usage(void)
 	fprintf(stderr, "         t       : include xattrs\n");
 	fprintf(stderr, "         e       : include open errors (aborts otherwise)\n");
 	fprintf(stderr, "         s       : include block structure (holes)\n");
-	fprintf(stderr, "    -[UGOAMCDTES]: exclude respective field from calculation\n");
+	fprintf(stderr, "         z       : include file size\n");
+	fprintf(stderr, "    -[UGOAMCDTESZ]: exclude respective field from calculation\n");
 	fprintf(stderr, "    -n           : reset all flags\n");
 	fprintf(stderr, "    -N           : set all flags\n");
 	fprintf(stderr, "    -x path      : exclude path when building checksum (multiple ok)\n");
 	fprintf(stderr, "    -R           : traverse dirs non-recursively (recursive is default)\n");
 	fprintf(stderr, "    -h           : this help\n\n");
-	fprintf(stderr, "The default field mask is ugoamCdtES. If the checksum/manifest is read from a\n");
+	fprintf(stderr, "The default field mask is ugoamCdtESz. If the checksum/manifest is read from a\n");
 	fprintf(stderr, "file, the mask is taken from there and the values given on the command line\n");
 	fprintf(stderr, "are ignored.\n");
 	exit(-1);
@@ -656,7 +658,8 @@ sum_one(int dirfd, int level, sum_t *dircs, char *path_prefix,
 			close(fd);
 		}
 	} else if (S_ISREG(st.st_mode)) {
-		sum_add_u64(&meta, st.st_size);
+		if (flags[FLAG_SIZE])
+			sum_add_u64(&meta, st.st_size);
 		if (flags[FLAG_DATA]) {
 			if (verbose)
 				fprintf(stderr, "file %s\n",
@@ -736,7 +739,7 @@ main(int argc, char *argv[])
 	int plen;
 	int elen;
 	int n_flags = 0;
-	const char *allopts = "heEfuUgGoOaAmMcCdDtTsSnNRw:r:vx:";
+	const char *allopts = "heEfuUgGoOaAmMcCdDtTsSzZnNRw:r:vx:";
 
 	out_fp = stdout;
 	while ((c = getopt(argc, argv, allopts)) != EOF) {
@@ -767,6 +770,8 @@ main(int argc, char *argv[])
 		case 'E':
 		case 's':
 		case 'S':
+		case 'z':
+		case 'Z':
 			++n_flags;
 			parse_flag(c);
 			break;
-- 
2.20.1

[PATCH v2 6/7] src/fssum: Allow single file input

[Arvind Raghavan]

Allow regular links and symlinks to be passed as input to fssum.

Signed-off-by: Arvind Raghavan <raghavan.arvind@gmail.com>
Signed-off-by: Jayashree Mohan <jaya@cs.utexas.edu>
Signed-off-by: Vijay Chidambaram <vijay@cs.utexas.edu>
---
Since v1:
	- Simplified logic by using sum_one
 src/fssum.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/fssum.c b/src/fssum.c
index 26b0096c..c5c27486 100644
--- a/src/fssum.c
+++ b/src/fssum.c
@@ -29,6 +29,7 @@
 #include <inttypes.h>
 #include <assert.h>
 #include <endian.h>
+#include <libgen.h>
 
 #define CS_SIZE 16
 #define CHUNKS	128
@@ -600,7 +601,8 @@ sum_one(int dirfd, int level, sum_t *dircs, char *path_prefix,
 			goto out;
 	}
 
-	ret = fstatat64(dirfd, name, &st, AT_SYMLINK_NOFOLLOW);
+	ret = fstatat64(dirfd, name, &st, AT_SYMLINK_NOFOLLOW |
+			AT_EMPTY_PATH);
 	if (ret) {
 		fprintf(stderr, "stat failed for %s/%s: %s\n",
 			path_prefix, path, strerror(errno));
@@ -890,8 +892,23 @@ main(int argc, char *argv[])
 	if (gen_manifest)
 		fprintf(out_fp, "Flags: %s\n", flagstring);
 
+	struct stat64 path_st;
+	if (fstat64(fd, &path_st)) {
+		perror("fstat");
+		exit(-1);
+	}
+
 	sum_init(&cs);
-	sum(fd, 1, &cs, path, "");
+
+	if (S_ISDIR(path_st.st_mode)) {
+		sum(fd, 1, &cs, path, "");
+	} else if (S_ISREG(path_st.st_mode) || S_ISLNK(path_st.st_mode)) {
+		sum_one(fd, 1, &cs, path, "", "");
+	} else {
+		fprintf(stderr, "path must be file or dir: %s", path);
+		exit(-1);
+	}
+
 	sum_fini(&cs);
 
 	close(fd);
-- 
2.20.1

[PATCH v2 7/7] src/fssum: Fix whitespace in usage

[Arvind Raghavan]

Aligns the usage whitespace properly.

Signed-off-by: Arvind Raghavan <raghavan.arvind@gmail.com>
Signed-off-by: Jayashree Mohan <jaya@cs.utexas.edu>
Signed-off-by: Vijay Chidambaram <vijay@cs.utexas.edu>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 src/fssum.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/src/fssum.c b/src/fssum.c
index c5c27486..4c20162c 100644
--- a/src/fssum.c
+++ b/src/fssum.c
@@ -135,28 +135,28 @@ usage(void)
 {
 	fprintf(stderr, "usage: fssum <options> <path>\n");
 	fprintf(stderr, "  options:\n");
-	fprintf(stderr, "    -f           : write out a full manifest file\n");
-	fprintf(stderr, "    -w <file>    : send output to file\n");
-	fprintf(stderr, "    -v           : verbose mode (debugging only)\n");
-	fprintf(stderr, "    -r <file>    : read checksum or manifest from file\n");
+	fprintf(stderr, "    -f            : write out a full manifest file\n");
+	fprintf(stderr, "    -w <file>     : send output to file\n");
+	fprintf(stderr, "    -v            : verbose mode (debugging only)\n");
+	fprintf(stderr, "    -r <file>     : read checksum or manifest from file\n");
 	fprintf(stderr, "    -[ugoamcdtesz]: specify which fields to include in checksum calculation.\n");
-	fprintf(stderr, "         u       : include uid\n");
-	fprintf(stderr, "         g       : include gid\n");
-	fprintf(stderr, "         o       : include mode\n");
-	fprintf(stderr, "         m       : include mtime\n");
-	fprintf(stderr, "         a       : include atime\n");
-	fprintf(stderr, "         c       : include ctime\n");
-	fprintf(stderr, "         d       : include file data\n");
-	fprintf(stderr, "         t       : include xattrs\n");
-	fprintf(stderr, "         e       : include open errors (aborts otherwise)\n");
-	fprintf(stderr, "         s       : include block structure (holes)\n");
-	fprintf(stderr, "         z       : include file size\n");
+	fprintf(stderr, "         u        : include uid\n");
+	fprintf(stderr, "         g        : include gid\n");
+	fprintf(stderr, "         o        : include mode\n");
+	fprintf(stderr, "         m        : include mtime\n");
+	fprintf(stderr, "         a        : include atime\n");
+	fprintf(stderr, "         c        : include ctime\n");
+	fprintf(stderr, "         d        : include file data\n");
+	fprintf(stderr, "         t        : include xattrs\n");
+	fprintf(stderr, "         e        : include open errors (aborts otherwise)\n");
+	fprintf(stderr, "         s        : include block structure (holes)\n");
+	fprintf(stderr, "         z        : include file size\n");
 	fprintf(stderr, "    -[UGOAMCDTESZ]: exclude respective field from calculation\n");
-	fprintf(stderr, "    -n           : reset all flags\n");
-	fprintf(stderr, "    -N           : set all flags\n");
-	fprintf(stderr, "    -x path      : exclude path when building checksum (multiple ok)\n");
-	fprintf(stderr, "    -R           : traverse dirs non-recursively (recursive is default)\n");
-	fprintf(stderr, "    -h           : this help\n\n");
+	fprintf(stderr, "    -n            : reset all flags\n");
+	fprintf(stderr, "    -N            : set all flags\n");
+	fprintf(stderr, "    -x path       : exclude path when building checksum (multiple ok)\n");
+	fprintf(stderr, "    -R            : traverse dirs non-recursively (recursive is default)\n");
+	fprintf(stderr, "    -h            : this help\n\n");
 	fprintf(stderr, "The default field mask is ugoamCdtESz. If the checksum/manifest is read from a\n");
 	fprintf(stderr, "file, the mask is taken from there and the values given on the command line\n");
 	fprintf(stderr, "are ignored.\n");
-- 
2.20.1

Re: [PATCH v2 2/7] src/fssum: Refactoring changes for recursive traversal

[Amir Goldstein]

On Mon, Jun 22, 2020 at 2:15 AM Arvind Raghavan
<raghavan.arvind@gmail.com> wrote:
>
> Refactoring changes needed for recursive traversal and single file
> input. Makes fstat and readlink use the 'at' alternatives, and creates a
> helper function for opening files.
>
> Signed-off-by: Arvind Raghavan <raghavan.arvind@gmail.com>
> Signed-off-by: Jayashree Mohan <jaya@cs.utexas.edu>
> Signed-off-by: Vijay Chidambaram <vijay@cs.utexas.edu>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>


> ---
>  src/fssum.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/src/fssum.c b/src/fssum.c
> index 30f456c2..135dd60f 100644
> --- a/src/fssum.c
> +++ b/src/fssum.c
> @@ -503,6 +503,13 @@ malformed:
>                 excess_file(fn);
>  }
>
> +int open_one(int dirfd, const char *name)
> +{
> +       if (!name || !*name)
> +               return dup(dirfd);
> +       return openat(dirfd, name, 0);
> +}
> +
>  void
>  sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
>  {
> @@ -563,12 +570,7 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
>                                 goto next;
>                 }
>
> -               ret = fchdir(dirfd);
> -               if (ret == -1) {
> -                       perror("fchdir");
> -                       exit(-1);
> -               }
> -               ret = lstat64(namelist[i], &st);
> +               ret = fstatat64(dirfd, namelist[i], &st, AT_SYMLINK_NOFOLLOW);
>                 if (ret) {
>                         fprintf(stderr, "stat failed for %s/%s: %s\n",
>                                 path_prefix, path, strerror(errno));
> @@ -597,7 +599,7 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
>                         sum_add_time(&meta, st.st_ctime);
>                 if (flags[FLAG_XATTRS] &&
>                     (S_ISDIR(st.st_mode) || S_ISREG(st.st_mode))) {
> -                       fd = openat(dirfd, namelist[i], 0);
> +                       fd = open_one(dirfd, namelist[i]);
>                         if (fd == -1 && flags[FLAG_OPEN_ERROR]) {
>                                 sum_add_u64(&meta, errno);
>                         } else if (fd == -1) {
> @@ -618,7 +620,7 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
>                         }
>                 }
>                 if (S_ISDIR(st.st_mode)) {
> -                       fd = openat(dirfd, namelist[i], 0);
> +                       fd = open_one(dirfd, namelist[i]);
>                         if (fd == -1 && flags[FLAG_OPEN_ERROR]) {
>                                 sum_add_u64(&meta, errno);
>                         } else if (fd == -1) {
> @@ -635,7 +637,7 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
>                                 if (verbose)
>                                         fprintf(stderr, "file %s\n",
>                                                 namelist[i]);
> -                               fd = openat(dirfd, namelist[i], 0);
> +                               fd = open_one(dirfd, namelist[i]);
>                                 if (fd == -1 && flags[FLAG_OPEN_ERROR]) {
>                                         sum_add_u64(&meta, errno);
>                                 } else if (fd == -1) {
> @@ -659,7 +661,7 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
>                                 }
>                         }
>                 } else if (S_ISLNK(st.st_mode)) {
> -                       ret = readlink(namelist[i], buf, sizeof(buf));
> +                       ret = readlinkat(dirfd, namelist[i], buf, sizeof(buf));
>                         if (ret == -1) {
>                                 perror("readlink");
>                                 exit(-1);
> --
> 2.20.1
>

Re: [PATCH v2 6/7] src/fssum: Allow single file input

[Amir Goldstein]

On Mon, Jun 22, 2020 at 2:16 AM Arvind Raghavan
<raghavan.arvind@gmail.com> wrote:
>
> Allow regular links and symlinks to be passed as input to fssum.
>
> Signed-off-by: Arvind Raghavan <raghavan.arvind@gmail.com>
> Signed-off-by: Jayashree Mohan <jaya@cs.utexas.edu>
> Signed-off-by: Vijay Chidambaram <vijay@cs.utexas.edu>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>


> ---
> Since v1:
>         - Simplified logic by using sum_one
>  src/fssum.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/src/fssum.c b/src/fssum.c
> index 26b0096c..c5c27486 100644
> --- a/src/fssum.c
> +++ b/src/fssum.c
> @@ -29,6 +29,7 @@
>  #include <inttypes.h>
>  #include <assert.h>
>  #include <endian.h>
> +#include <libgen.h>
>

Is this needed by this patch? needed at all?
Should it have been added in a previous patch?

Thanks,
Amir.

Re: [PATCH v2 3/7] src/fssum: Recursive traversal refactoring

[Amir Goldstein]

[-- Attachment #1: Type: text/plain, Size: 567 bytes --]

On Mon, Jun 22, 2020 at 2:16 AM Arvind Raghavan
<raghavan.arvind@gmail.com> wrote:
>
> Moves some logic from the recursive directory traversal into a helper
> function to make it easier to add support for regular files. Does not
> change functionality.

It is VERY hard to see in review that this does not change functionality.
As opposed to the attached patches.

I know if we were working with modern code review tools, no need for
those cheap tricks, although using a helper name argument in the previous
patch would have helped in any review tool.

Thanks,
Amir.

[-- Attachment #2: 0001-src-fssum-Refactoring-changes-for-recursive-traversa.patch.txt --]
[-- Type: text/plain, Size: 3713 bytes --]

From 0c8692f28dacbcf5e0dae2b21774706946e54510 Mon Sep 17 00:00:00 2001
From: Arvind Raghavan <raghavan.arvind@gmail.com>
Date: Sun, 21 Jun 2020 19:15:44 -0400
Subject: [PATCH 1/2] src/fssum: Refactoring changes for recursive traversal

Refactoring changes needed for recursive traversal and single file
input. Makes fstat and readlink use the 'at' alternatives, and creates a
helper function for opening files.

Signed-off-by: Arvind Raghavan <raghavan.arvind@gmail.com>
Signed-off-by: Jayashree Mohan <jaya@cs.utexas.edu>
Signed-off-by: Vijay Chidambaram <vijay@cs.utexas.edu>
---
 src/fssum.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/src/fssum.c b/src/fssum.c
index 30f456c2..6192f109 100644
--- a/src/fssum.c
+++ b/src/fssum.c
@@ -503,6 +503,13 @@ malformed:
 		excess_file(fn);
 }
 
+int open_one(int dirfd, const char *name)
+{
+	if (!name || !*name)
+		return dup(dirfd);
+	return openat(dirfd, name, 0);
+}
+
 void
 sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 {
@@ -552,23 +559,19 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 		sum_t cs;
 		sum_t meta;
 		char *path;
+		char *name = namelist[i];
 
 		sum_init(&cs);
 		sum_init(&meta);
-		path = alloc(strlen(path_in) + strlen(namelist[i]) + 3);
-		sprintf(path, "%s/%s", path_in, namelist[i]);
+		path = alloc(strlen(path_in) + strlen(name) + 3);
+		sprintf(path, "%s/%s", path_in, name);
 		for (excl = 0; excl < n_excludes; ++excl) {
 			if (strncmp(excludes[excl].path, path,
 			    excludes[excl].len) == 0)
 				goto next;
 		}
 
-		ret = fchdir(dirfd);
-		if (ret == -1) {
-			perror("fchdir");
-			exit(-1);
-		}
-		ret = lstat64(namelist[i], &st);
+		ret = fstatat64(dirfd, name, &st, AT_SYMLINK_NOFOLLOW);
 		if (ret) {
 			fprintf(stderr, "stat failed for %s/%s: %s\n",
 				path_prefix, path, strerror(errno));
@@ -580,7 +583,7 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 			goto next;
 
 		sum_add_u64(&meta, level);
-		sum_add(&meta, namelist[i], strlen(namelist[i]));
+		sum_add(&meta, name, strlen(name));
 		if (!S_ISDIR(st.st_mode))
 			sum_add_u64(&meta, st.st_nlink);
 		if (flags[FLAG_UID])
@@ -597,7 +600,7 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 			sum_add_time(&meta, st.st_ctime);
 		if (flags[FLAG_XATTRS] &&
 		    (S_ISDIR(st.st_mode) || S_ISREG(st.st_mode))) {
-			fd = openat(dirfd, namelist[i], 0);
+			fd = open_one(dirfd, name);
 			if (fd == -1 && flags[FLAG_OPEN_ERROR]) {
 				sum_add_u64(&meta, errno);
 			} else if (fd == -1) {
@@ -618,7 +621,7 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 			}
 		}
 		if (S_ISDIR(st.st_mode)) {
-			fd = openat(dirfd, namelist[i], 0);
+			fd = open_one(dirfd, name);
 			if (fd == -1 && flags[FLAG_OPEN_ERROR]) {
 				sum_add_u64(&meta, errno);
 			} else if (fd == -1) {
@@ -633,9 +636,8 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 			sum_add_u64(&meta, st.st_size);
 			if (flags[FLAG_DATA]) {
 				if (verbose)
-					fprintf(stderr, "file %s\n",
-						namelist[i]);
-				fd = openat(dirfd, namelist[i], 0);
+					fprintf(stderr, "file %s\n", name);
+				fd = open_one(dirfd, name);
 				if (fd == -1 && flags[FLAG_OPEN_ERROR]) {
 					sum_add_u64(&meta, errno);
 				} else if (fd == -1) {
@@ -659,7 +661,7 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 				}
 			}
 		} else if (S_ISLNK(st.st_mode)) {
-			ret = readlink(namelist[i], buf, sizeof(buf));
+			ret = readlinkat(dirfd, name, buf, sizeof(buf));
 			if (ret == -1) {
 				perror("readlink");
 				exit(-1);
-- 
2.17.1


[-- Attachment #3: 0002-src-fssum-Recursive-traversal-refactoring.patch.txt --]
[-- Type: text/plain, Size: 3097 bytes --]

From c54d1f8fe91cffe9685268ef5717e999da5a7391 Mon Sep 17 00:00:00 2001
From: Arvind Raghavan <raghavan.arvind@gmail.com>
Date: Sun, 21 Jun 2020 19:16:03 -0400
Subject: [PATCH 2/2] src/fssum: Recursive traversal refactoring

Moves some logic from the recursive directory traversal into a helper
function to make it easier to add support for regular files. Does not
change functionality.

Signed-off-by: Arvind Raghavan <raghavan.arvind@gmail.com>
Signed-off-by: Jayashree Mohan <jaya@cs.utexas.edu>
Signed-off-by: Vijay Chidambaram <vijay@cs.utexas.edu>
---
 src/fssum.c | 44 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/src/fssum.c b/src/fssum.c
index 6192f109..7bfa8c4f 100644
--- a/src/fssum.c
+++ b/src/fssum.c
@@ -510,6 +510,10 @@ int open_one(int dirfd, const char *name)
 	return openat(dirfd, name, 0);
 }
 
+void
+sum_one(int dirfd, int level, sum_t *dircs, char *path_prefix,
+		char *path_in, char *name);
+
 void
 sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 {
@@ -520,8 +524,6 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 	int entries = 0;
 	int i;
 	int ret;
-	int fd;
-	int excl;
 	struct stat64 dir_st;
 
 	if (fstat64(dirfd, &dir_st)) {
@@ -556,10 +558,36 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 	qsort(namelist, entries, sizeof(*namelist), namecmp);
 	for (i = 0; i < entries; ++i) {
 		struct stat64 st;
+
+		ret = fstatat64(dirfd, namelist[i], &st, AT_SYMLINK_NOFOLLOW);
+		if (ret) {
+			fprintf(stderr, "stat failed for %s/%s/%s: %s\n",
+				path_prefix, path_in, namelist[i],
+				strerror(errno));
+			exit(-1);
+		}
+
+		/* We are crossing into a different subvol, skip this subtree. */
+		if (st.st_dev != dir_st.st_dev)
+			continue;
+
+		sum_one(dirfd, level, dircs, path_prefix, path_in, namelist[i]);
+	}
+}
+
+void
+sum_one(int dirfd, int level, sum_t *dircs, char *path_prefix,
+		char *path_in, char *name)
+{
+	/* TEMPORARY NESTING WILL BE REMOVED SOON */
+	{
 		sum_t cs;
 		sum_t meta;
-		char *path;
-		char *name = namelist[i];
+		int fd;
+		int ret;
+		int excl;
+		char* path;
+		struct stat64 st;
 
 		sum_init(&cs);
 		sum_init(&meta);
@@ -568,7 +596,7 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 		for (excl = 0; excl < n_excludes; ++excl) {
 			if (strncmp(excludes[excl].path, path,
 			    excludes[excl].len) == 0)
-				goto next;
+				goto out;
 		}
 
 		ret = fstatat64(dirfd, name, &st, AT_SYMLINK_NOFOLLOW);
@@ -578,10 +606,6 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 			exit(-1);
 		}
 
-		/* We are crossing into a different subvol, skip this subtree. */
-		if (st.st_dev != dir_st.st_dev)
-			goto next;
-
 		sum_add_u64(&meta, level);
 		sum_add(&meta, name, strlen(name));
 		if (!S_ISDIR(st.st_mode))
@@ -694,7 +718,7 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 		}
 		sum_add_sum(dircs, &cs);
 		sum_add_sum(dircs, &meta);
-next:
+out:
 		free(path);
 	}
 }
-- 
2.17.1