[PATCH 0/3] btrfs-progs: restore symlinks

[PATCH 0/3] btrfs-progs: restore symlinks

[Dan Merillat]

Hopefully this is sufficiently paranoid, tested with PATH_MAX length
symlinks, existing files, insufficient permissions, dangling symlinks.
I think I got the coding style correct this time, I'll fix and resend if
not.

Includes a trivial fix from my metadata patch, the documentation got
lost in the merge.

[PATCH 1/3] btrfs-progs: restore: document metadata restore.

[Dan Merillat]

This was lost in the cleanup of 71a559

Signed-off-by: Dan Merillat <dan.merillat@gmail.com>
---
 Documentation/btrfs-restore.asciidoc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/btrfs-restore.asciidoc b/Documentation/btrfs-restore.asciidoc
index 20fc366..89e0c87 100644
--- a/Documentation/btrfs-restore.asciidoc
+++ b/Documentation/btrfs-restore.asciidoc
@@ -29,6 +29,9 @@ get snapshots, btrfs restore skips snapshots in default.
 -x::
 get extended attributes.
 
+-m|--metadata::
+restore owner, mode and times.
+
 -v::
 verbose.
 
-- 
2.1.4

[PATCH 2/3] btrfs-progs: separate the overwrite check.

[Dan Merillat]

Symlink restore needs this, but the cut&paste became
too complicated.  Simplify everything.

Signed-off-by: Dan Merillat <dan.merillat@gmail.com>
---
 cmds-restore.c | 53 ++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 34 insertions(+), 19 deletions(-)

diff --git a/cmds-restore.c b/cmds-restore.c
index e877548..8869f2a 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -781,6 +781,37 @@ out:
 	return ret;
 }
 
+/* returns:
+ *  0 if the file exists and should be skipped.
+ *  1 if the file does NOT exist
+ *  2 if the file exists but is OK to overwrite
+ */
+
+static int overwrite_ok(const char * path)
+{
+	static int warn = 0;
+	struct stat st;
+	int ret;
+
+	/* don't be fooled by symlinks */
+	ret = fstatat(-1, path_name, &st, AT_SYMLINK_NOFOLLOW);
+
+	if (!ret) {
+		if (overwrite)
+			return 2;
+
+		if (verbose || !warn)
+			printf("Skipping existing file"
+				   " %s\n", path);
+		if (!warn)
+			printf("If you wish to overwrite use "
+				   "the -o option to overwrite\n");
+		warn = 1;
+		return 0;
+	}
+	return 1;
+}
+
 static int search_dir(struct btrfs_root *root, struct btrfs_key *key,
 		      const char *output_rootdir, const char *in_dir,
 		      const regex_t *mreg)
@@ -897,25 +928,9 @@ static int search_dir(struct btrfs_root *root, struct btrfs_key *key,
 		 * files, no symlinks or anything else.
 		 */
 		if (type == BTRFS_FT_REG_FILE) {
-			if (!overwrite) {
-				static int warn = 0;
-				struct stat st;
-
-				ret = stat(path_name, &st);
-				if (!ret) {
-					loops = 0;
-					if (verbose || !warn)
-						printf("Skipping existing file"
-						       " %s\n", path_name);
-					if (warn)
-						goto next;
-					printf("If you wish to overwrite use "
-					       "the -o option to overwrite\n");
-					warn = 1;
-					goto next;
-				}
-				ret = 0;
-			}
+			if (!overwrite_ok(path_name))
+				goto next;
+
 			if (verbose)
 				printf("Restoring %s\n", path_name);
 			if (dry_run)
-- 
2.1.4

Re: [PATCH 2/3] btrfs-progs: separate the overwrite check.

[David Sterba]

On Thu, Apr 23, 2015 at 12:51:33PM -0400, Dan Merillat wrote:
> +/* returns:
> + *  0 if the file exists and should be skipped.
> + *  1 if the file does NOT exist
> + *  2 if the file exists but is OK to overwrite
> + */
> +
> +static int overwrite_ok(const char * path)
> +{
> +	static int warn = 0;
> +	struct stat st;
> +	int ret;
> +
> +	/* don't be fooled by symlinks */
> +	ret = fstatat(-1, path_name, &st, AT_SYMLINK_NOFOLLOW);

Is the filedescriptor -1 correct? Previously, stat was used that uses
AT_FDCWD for the dirfd, which is -100. -1 could be intepreted as a bad
filedescriptor (EBADF).

Re: [PATCH 2/3] btrfs-progs: separate the overwrite check.

[Dan Merillat]

It's a good question.  If path_name is absolute, the file descriptor
is ignored.  I used -1 (EBADF) instead of AT_FDCWD  there so if a
non-absolute path gets in there it errors out instead of attempting to
use a relative path off the current directory.

I'm not entirely sure if it's the best way, so if anyone else has
ideas let me know.

On Fri, Apr 24, 2015 at 11:24 AM, David Sterba <dsterba@suse.cz> wrote:
> On Thu, Apr 23, 2015 at 12:51:33PM -0400, Dan Merillat wrote:
>> +/* returns:
>> + *  0 if the file exists and should be skipped.
>> + *  1 if the file does NOT exist
>> + *  2 if the file exists but is OK to overwrite
>> + */
>> +
>> +static int overwrite_ok(const char * path)
>> +{
>> +     static int warn = 0;
>> +     struct stat st;
>> +     int ret;
>> +
>> +     /* don't be fooled by symlinks */
>> +     ret = fstatat(-1, path_name, &st, AT_SYMLINK_NOFOLLOW);
>
> Is the filedescriptor -1 correct? Previously, stat was used that uses
> AT_FDCWD for the dirfd, which is -100. -1 could be intepreted as a bad
> filedescriptor (EBADF).

Re: [PATCH 2/3] btrfs-progs: separate the overwrite check.

[David Sterba]

On Sat, Apr 25, 2015 at 02:19:47PM -0400, Dan Merillat wrote:
> It's a good question.  If path_name is absolute, the file descriptor
> is ignored.

AFAICS the paths are just appended to what the user specifies on the
commandline, so it's not necessarily an absolute path. However, it still
works with -1 as expected. I'll merge the patch as-is (scheduled for
4.0.1) so there's time to test it.

[PATCH 3/3] btrfs-progs: optionally restore symlinks.

[Dan Merillat]

Restore symlinks, optionally with owner/times.

Signed-off-by: Dan Merillat <dan.merillat@gmail.com>
---
 Documentation/btrfs-restore.asciidoc |   3 +
 cmds-restore.c                       | 140 ++++++++++++++++++++++++++++++++++-
 2 files changed, 140 insertions(+), 3 deletions(-)

diff --git a/Documentation/btrfs-restore.asciidoc b/Documentation/btrfs-restore.asciidoc
index 89e0c87..06a0498 100644
--- a/Documentation/btrfs-restore.asciidoc
+++ b/Documentation/btrfs-restore.asciidoc
@@ -32,6 +32,9 @@ get extended attributes.
 -m|--metadata::
 restore owner, mode and times.
 
+-S|--symlinks::
+restore symbolic links as well as normal files.
+
 -v::
 verbose.
 
diff --git a/cmds-restore.c b/cmds-restore.c
index 8869f2a..c7a3e96 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -45,9 +45,11 @@
 
 static char fs_name[PATH_MAX];
 static char path_name[PATH_MAX];
+static char symlink_target[PATH_MAX];
 static int get_snaps = 0;
 static int verbose = 0;
 static int restore_metadata = 0;
+static int restore_symlinks = 0;
 static int ignore_errors = 0;
 static int overwrite = 0;
 static int get_xattrs = 0;
@@ -812,6 +814,125 @@ static int overwrite_ok(const char * path)
 	return 1;
 }
 
+static int copy_symlink(struct btrfs_root *root, struct btrfs_key *key,
+		     const char *file)
+{
+	struct btrfs_path *path;
+	struct extent_buffer *leaf;
+	struct btrfs_file_extent_item *extent_item;
+	struct btrfs_inode_item *inode_item;
+	u32 len;
+	int ret;
+
+	ret = overwrite_ok(path_name);
+	if (ret == 0)
+	    return 0; // skip this file.
+
+	if (ret == 2) { // symlink() can't overwrite, so unlink first.
+		ret = unlink(path_name);
+		if (ret) {
+			fprintf(stderr, "failed to unlink '%s' for overwrite\n",
+					path_name);
+			return ret;
+		}
+	}
+
+	key->type = BTRFS_EXTENT_DATA_KEY;
+	key->offset = 0;
+
+	path = btrfs_alloc_path();
+	if (!path)
+		return -ENOMEM;
+
+	ret = btrfs_search_slot(NULL, root, key, path, 0, 0);
+	if (ret < 0)
+		goto out;
+
+	leaf = path->nodes[0];
+	if (!leaf) {
+		fprintf(stderr, "Error getting leaf for symlink '%s'\n", file);
+		ret = -1;
+		goto out;
+	}
+
+	extent_item = btrfs_item_ptr(leaf, path->slots[0],
+			struct btrfs_file_extent_item);
+
+	len = btrfs_file_extent_inline_item_len(leaf,
+			btrfs_item_nr(path->slots[0]));
+	if (len > PATH_MAX) {
+		fprintf(stderr, "Symlink '%s' target length %d is longer than PATH_MAX\n",
+				fs_name, len);
+		ret = -1;
+		goto out;
+	}
+
+	u32 name_offset = (unsigned long) extent_item
+			+ offsetof(struct btrfs_file_extent_item, disk_bytenr);
+	read_extent_buffer(leaf, symlink_target, name_offset, len);
+
+	symlink_target[len] = 0;
+
+	if (!dry_run) {
+		ret = symlink(symlink_target, path_name);
+		if (ret<0) {
+			fprintf(stderr, "Failed to restore symlink '%s': %s\n",
+					path_name, strerror(errno));
+			goto out;
+		}
+	}
+	printf("SYMLINK: '%s' => '%s'\n", path_name, symlink_target);
+
+	ret = 0;
+	if (!restore_metadata)
+		goto out;
+
+	/* Symlink metadata operates differently than files/directories,
+	 * so do our own work here.
+	 */
+
+	key->type = BTRFS_INODE_ITEM_KEY;
+	key->offset = 0;
+
+	btrfs_release_path(path);
+
+	ret = btrfs_lookup_inode(NULL, root, path, key, 0);
+	if (ret) {
+		fprintf(stderr, "Failed to lookup inode for '%s'\n", file);
+		goto out;
+	}
+
+	inode_item = btrfs_item_ptr(path->nodes[0], path->slots[0],
+			struct btrfs_inode_item);
+
+	fchownat(-1, file, btrfs_inode_uid(path->nodes[0], inode_item),
+				   btrfs_inode_gid(path->nodes[0], inode_item),
+				   AT_SYMLINK_NOFOLLOW);
+	if (ret) {
+		fprintf(stderr, "Failed to change owner: %s\n",
+				strerror(errno));
+		goto out;
+	}
+
+	struct btrfs_timespec *bts;
+	struct timespec times[2];
+
+	bts = btrfs_inode_atime(inode_item);
+	times[0].tv_sec  = btrfs_timespec_sec(path->nodes[0], bts);
+	times[0].tv_nsec = btrfs_timespec_nsec(path->nodes[0], bts);
+
+	bts = btrfs_inode_mtime(inode_item);
+	times[1].tv_sec  = btrfs_timespec_sec(path->nodes[0], bts);
+	times[1].tv_nsec = btrfs_timespec_nsec(path->nodes[0], bts);
+
+	ret = utimensat(-1, file, times, AT_SYMLINK_NOFOLLOW);
+	if (ret)
+		fprintf(stderr, "Failed to set times: %s\n", strerror(errno));
+out:
+	btrfs_free_path(path);
+	return ret;
+}
+
 static int search_dir(struct btrfs_root *root, struct btrfs_key *key,
 		      const char *output_rootdir, const char *in_dir,
 		      const regex_t *mreg)
@@ -924,8 +1045,7 @@ static int search_dir(struct btrfs_root *root, struct btrfs_key *key,
 		snprintf(path_name, PATH_MAX, "%s%s", output_rootdir, fs_name);
 
 		/*
-		 * At this point we're only going to restore directories and
-		 * files, no symlinks or anything else.
+		 * Restore directories, files, symlinks and metadata.
 		 */
 		if (type == BTRFS_FT_REG_FILE) {
 			if (!overwrite_ok(path_name))
@@ -1032,6 +1152,15 @@ static int search_dir(struct btrfs_root *root, struct btrfs_key *key,
 					goto next;
 				goto out;
 			}
+		} else if (type == BTRFS_FT_SYMLINK) {
+			if (restore_symlinks)
+			ret = copy_symlink(root, &location, path_name);
+			if (ret<0) {
+				if (ignore_errors)
+					goto next;
+				btrfs_free_path(path);
+				return ret;
+			}
 		}
 next:
 		path->slots[0]++;
@@ -1258,6 +1387,7 @@ const char * const cmd_restore_usage[] = {
 	"-s              get snapshots",
 	"-x              get extended attributes",
 	"-m|--metadata   restore owner, mode and times",
+	"-S|--symlinks	 restore symbolic links"
 	"-v              verbose",
 	"-i              ignore errors",
 	"-o              overwrite",
@@ -1300,10 +1430,11 @@ int cmd_restore(int argc, char **argv)
 			{ "path-regex", required_argument, NULL, 256},
 			{ "dry-run", no_argument, NULL, 'D'},
 			{ "metadata", no_argument, NULL, 'm'},
+			{ "symlinks", no_argument, NULL, 'S'},
 			{ NULL, 0, NULL, 0}
 		};
 
-		opt = getopt_long(argc, argv, "sxviot:u:dmf:r:lDc", long_options,
+		opt = getopt_long(argc, argv, "sSxviot:u:dmf:r:lDc", long_options,
 					NULL);
 		if (opt < 0)
 			break;
@@ -1352,6 +1483,9 @@ int cmd_restore(int argc, char **argv)
 			case 'm':
 				restore_metadata = 1;
 				break;
+			case 'S':
+				restore_symlinks = 1;
+				break;
 			case 'D':
 				dry_run = 1;
 				break;
-- 
2.1.4

Re: [PATCH 0/3] btrfs-progs: restore symlinks

[Duncan]

Dan Merillat posted on Thu, 23 Apr 2015 12:47:29 -0400 as excerpted:

> Hopefully this is sufficiently paranoid, tested with PATH_MAX length
> symlinks, existing files, insufficient permissions, dangling symlinks. I
> think I got the coding style correct this time, I'll fix and resend if
> not.
> 
> Includes a trivial fix from my metadata patch, the documentation got
> lost in the merge.

Thanks for all this.  I've only had to use restore once and hopefully 
won't be using it again in the near future, but having it restore the 
metadata and symlinks as well would surely have made the experience 
easier.  There's a lot of people going to benefit from these patches over 
time as btrfs gains usage and the inevitable breakage happens to some of 
those filesystems. =:^/

-- 
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman

Re: [PATCH 0/3] btrfs-progs: restore symlinks

[Dan Merillat]

At this point I I'm done - after writing the symlink patch I restored
everything of importance off my array to a scratch disk, wiped the
array and am in the process of copying everything back.  I'll keep an
eye on this thread if changes need to be made to my patches, but
hopefully I won't be needing btrfs restore for a few more years!


On Fri, Apr 24, 2015 at 12:38 AM, Duncan <1i5t5.duncan@cox.net> wrote:
> Dan Merillat posted on Thu, 23 Apr 2015 12:47:29 -0400 as excerpted:
>
>> Hopefully this is sufficiently paranoid, tested with PATH_MAX length
>> symlinks, existing files, insufficient permissions, dangling symlinks. I
>> think I got the coding style correct this time, I'll fix and resend if
>> not.
>>
>> Includes a trivial fix from my metadata patch, the documentation got
>> lost in the merge.
>
> Thanks for all this.  I've only had to use restore once and hopefully
> won't be using it again in the near future, but having it restore the
> metadata and symlinks as well would surely have made the experience
> easier.  There's a lot of people going to benefit from these patches over
> time as btrfs gains usage and the inevitable breakage happens to some of
> those filesystems. =:^/
>
> --
> Duncan - List replies preferred.   No HTML msgs.
> "Every nonfree program has a lord, a master --
> and if you use the program, he is your master."  Richard Stallman
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html