Btrfs-progs: Update btrfs restore so it matches all current development

Btrfs-progs: Update btrfs restore so it matches all current development

[Josef Bacik]

Hello,

I noticed today that a bunch of work that had been done to restore last year
wasn't in the normal btrfs-progs restore.  So this work forward ports all of the
patches that were missing, and redoes a bunch of the work that was done to deal
with really broken file systems so that it uses more of the generic helpers that
Chris wrote a while ago.  This is based on the latest integration branch, Dave
if you want to just pull you can do

git://github.com/josefbacik/btrfs-progs.git for-kdave

Thanks,

Josef

[PATCH 01/10] Btrfs-progs: add an option for specifying the root to restore

[Josef Bacik]

From: Josef Bacik <josef@redhat.com>

If the normal fs tree is hosed and the user has multiple subvolumes it's handy
to be able to specify just one of the subvolumes to restore.  It's also handy if
a user only wants to restore say /home instead of his entire disk.  Thanks,

Signed-off-by: Josef Bacik <josef@redhat.com>
---
 cmds-restore.c |   32 ++++++++++++++++++++++++++++----
 1 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/cmds-restore.c b/cmds-restore.c
index 4a14f93..467e5ef 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -773,13 +773,14 @@ int cmd_restore(int argc, char **argv)
 	char dir_name[128];
 	u64 tree_location = 0;
 	u64 fs_location = 0;
+	u64 root_objectid = 0;
 	int len;
 	int ret;
 	int opt;
 	int super_mirror = 0;
 	int find_dir = 0;
 
-	while ((opt = getopt(argc, argv, "sviot:u:df:")) != -1) {
+	while ((opt = getopt(argc, argv, "sviot:u:df:r:")) != -1) {
 		switch (opt) {
 			case 's':
 				get_snaps = 1;
@@ -822,6 +823,14 @@ int cmd_restore(int argc, char **argv)
 			case 'd':
 				find_dir = 1;
 				break;
+			case 'r':
+				errno = 0;
+				root_objectid = (u64)strtoll(optarg, NULL, 10);
+				if (errno != 0) {
+					fprintf(stderr, "Root objectid not valid\n");
+					exit(1);
+				}
+				break;
 			default:
 				usage(cmd_restore_usage);
 		}
@@ -852,8 +861,6 @@ int cmd_restore(int argc, char **argv)
 		}
 	}
 
-	printf("Root objectid is %Lu\n", root->objectid);
-
 	memset(path_name, 0, 4096);
 
 	strncpy(dir_name, argv[optind + 1], sizeof dir_name);
@@ -865,6 +872,23 @@ int cmd_restore(int argc, char **argv)
 		dir_name[len] = '\0';
 	}
 
+	if (root_objectid != 0) {
+		struct btrfs_root *orig_root = root;
+
+		key.objectid = root_objectid;
+		key.type = BTRFS_ROOT_ITEM_KEY;
+		key.offset = (u64)-1;
+		root = btrfs_read_fs_root(orig_root->fs_info, &key);
+		if (IS_ERR(root)) {
+			fprintf(stderr, "Error reading root\n");
+			root = orig_root;
+			ret = 1;
+			goto out;
+		}
+		key.type = 0;
+		key.offset = 0;
+	}
+
 	if (find_dir) {
 		ret = find_first_dir(root, &key.objectid);
 		if (ret)
@@ -873,7 +897,7 @@ int cmd_restore(int argc, char **argv)
 		key.objectid = BTRFS_FIRST_FREE_OBJECTID;
 	}
 
-	ret = search_dir(root->fs_info->fs_root, &key, dir_name);
+	ret = search_dir(root, &key, dir_name);
 
 out:
 	close_ctree(root);
-- 
1.7.7.6

[PATCH 02/10] Btrfs-progs: try other mirrors if decomression fails

[Josef Bacik]

From: Josef Bacik <josef@redhat.com>

This will make the restore program fall back on other mirrors if it fails to
decompress an extent for whatever reason.  Thanks,

Signed-off-by: Josef Bacik <josef@redhat.com>
---
 cmds-restore.c |   46 +++++++++++++++++++++++++---------------------
 1 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/cmds-restore.c b/cmds-restore.c
index 467e5ef..1d24691 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -65,7 +65,7 @@ static int decompress(char *inbuf, char *outbuf, u64 compress_len,
 	ret = inflate(&strm, Z_NO_FLUSH);
 	if (ret != Z_STREAM_END) {
 		(void)inflateEnd(&strm);
-		fprintf(stderr, "ret is %d\n", ret);
+		fprintf(stderr, "failed to inflate: %d\n", ret);
 		return -1;
 	}
 
@@ -198,6 +198,8 @@ static int copy_one_extent(struct btrfs_root *root, int fd,
 	int compress;
 	int ret;
 	int dev_fd;
+	int mirror_num = 0;
+	int num_copies;
 
 	compress = btrfs_file_extent_compression(leaf, fi);
 	bytenr = btrfs_file_extent_disk_bytenr(leaf, fi);
@@ -226,12 +228,10 @@ static int copy_one_extent(struct btrfs_root *root, int fd,
 again:
 	length = size_left;
 	ret = btrfs_map_block(&root->fs_info->mapping_tree, READ,
-			      bytenr, &length, &multi, 0, NULL);
+			      bytenr, &length, &multi, mirror_num, NULL);
 	if (ret) {
-		free(inbuf);
-		free(outbuf);
 		fprintf(stderr, "Error mapping block %d\n", ret);
-		return ret;
+		goto out;
 	}
 	device = multi->stripes[0].dev;
 	dev_fd = device->fd;
@@ -245,10 +245,9 @@ again:
 
 	done = pread(dev_fd, inbuf+count, length, dev_bytenr);
 	if (done < length) {
-		free(inbuf);
-		free(outbuf);
+		ret = -1;
 		fprintf(stderr, "Short read %d\n", errno);
-		return -1;
+		goto out;
 	}
 
 	count += length;
@@ -256,41 +255,46 @@ again:
 	if (size_left)
 		goto again;
 
-
 	if (compress == BTRFS_COMPRESS_NONE) {
 		while (total < ram_size) {
 			done = pwrite(fd, inbuf+total, ram_size-total,
 				      pos+total);
 			if (done < 0) {
-				free(inbuf);
+				ret = -1;
 				fprintf(stderr, "Error writing: %d %s\n", errno, strerror(errno));
-				return -1;
+				goto out;
 			}
 			total += done;
 		}
-		free(inbuf);
-		return 0;
+		ret = 0;
+		goto out;
 	}
 
 	ret = decompress(inbuf, outbuf, disk_size, ram_size);
-	free(inbuf);
 	if (ret) {
-		free(outbuf);
-		return ret;
+		num_copies = btrfs_num_copies(&root->fs_info->mapping_tree,
+					      bytenr, length);
+		mirror_num++;
+		if (mirror_num >= num_copies) {
+			ret = -1;
+			goto out;
+		}
+		fprintf(stderr, "Trying another mirror\n");
+		goto again;
 	}
 
 	while (total < ram_size) {
 		done = pwrite(fd, outbuf+total, ram_size-total, pos+total);
 		if (done < 0) {
-			free(outbuf);
-			fprintf(stderr, "Error writing: %d %s\n", errno, strerror(errno));
-			return -1;
+			ret = -1;
+			goto out;
 		}
 		total += done;
 	}
+out:
+	free(inbuf);
 	free(outbuf);
-
-	return 0;
+	return ret;
 }
 
 static int ask_to_continue(const char *file)
-- 
1.7.7.6

[PATCH 03/10] Btrfs-progs: try other mirrors on read failure

[Josef Bacik]

From: Josef Bacik <josef@redhat.com>

If we hit a bad disk and the read doesn't work, try other mirrors in case we
have other disks with good copies.  Thanks,

Signed-off-by: Josef Bacik <josef@redhat.com>
---
 cmds-restore.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/cmds-restore.c b/cmds-restore.c
index 1d24691..617f507 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -245,9 +245,16 @@ again:
 
 	done = pread(dev_fd, inbuf+count, length, dev_bytenr);
 	if (done < length) {
-		ret = -1;
-		fprintf(stderr, "Short read %d\n", errno);
-		goto out;
+		num_copies = btrfs_num_copies(&root->fs_info->mapping_tree,
+					      bytenr, length);
+		mirror_num++;
+		if (mirror_num >= num_copies) {
+			ret = -1;
+			fprintf(stderr, "Exhausted mirrors trying to read\n");
+			goto out;
+		}
+		fprintf(stderr, "Trying another mirror\n");
+		goto again;
 	}
 
 	count += length;
-- 
1.7.7.6

[PATCH 04/10] btrfs-progs: Fix error handling for failed reads in restore tool when mirrors exist

[Josef Bacik]

From: David Marcin <djmarcin@google.com>

Signed-off-by: David Marcin <djmarcin@google.com>
---
 cmds-restore.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/cmds-restore.c b/cmds-restore.c
index 617f507..9781801 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -198,7 +198,7 @@ static int copy_one_extent(struct btrfs_root *root, int fd,
 	int compress;
 	int ret;
 	int dev_fd;
-	int mirror_num = 0;
+	int mirror_num = 1;
 	int num_copies;
 
 	compress = btrfs_file_extent_compression(leaf, fi);
@@ -241,14 +241,15 @@ again:
 
 	if (size_left < length)
 		length = size_left;
-	size_left -= length;
 
 	done = pread(dev_fd, inbuf+count, length, dev_bytenr);
-	if (done < length) {
+	/* Need both checks, or we miss negative values due to u64 conversion */
+	if (done < 0 || done < length) {
 		num_copies = btrfs_num_copies(&root->fs_info->mapping_tree,
 					      bytenr, length);
 		mirror_num++;
-		if (mirror_num >= num_copies) {
+		/* mirror_num is 1-indexed, so num_copies is a valid mirror. */
+		if (mirror_num > num_copies) {
 			ret = -1;
 			fprintf(stderr, "Exhausted mirrors trying to read\n");
 			goto out;
@@ -257,6 +258,8 @@ again:
 		goto again;
 	}
 
+	mirror_num = 1;
+	size_left -= length;
 	count += length;
 	bytenr += length;
 	if (size_left)
-- 
1.7.7.6

[PATCH 05/10] restore: Split output directory and btrfs-local path search_dir() parameters

[Josef Bacik]

From: Peter Stuge <peter@stuge.se>

search_dir() recurses down the btrfs tree, and used to take the output
path for every item (i.e. in the running system, output root directory
concatenated with btrfs-local pathname) passed as the only path
parameter. Moving the output root directory to a separate parameter
and passing the btrfs-local pathname for each file and directory
separately allows easy filtering based on the btrfs-local pathname.

Signed-off-by: Peter Stuge <peter@stuge.se>
Signed-off-by: Josef Bacik <josef@redhat.com>
---
 cmds-restore.c |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/cmds-restore.c b/cmds-restore.c
index 9781801..215958f 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -39,6 +39,7 @@
 #include "utils.h"
 #include "commands.h"
 
+static char fs_name[4096];
 static char path_name[4096];
 static int get_snaps = 0;
 static int verbose = 0;
@@ -454,7 +455,7 @@ set_size:
 }
 
 static int search_dir(struct btrfs_root *root, struct btrfs_key *key,
-		      const char *dir)
+		      const char *output_rootdir, const char *dir)
 {
 	struct btrfs_path *path;
 	struct extent_buffer *leaf;
@@ -558,8 +559,11 @@ static int search_dir(struct btrfs_root *root, struct btrfs_key *key,
 		type = btrfs_dir_type(leaf, dir_item);
 		btrfs_dir_item_key_to_cpu(leaf, dir_item, &location);
 
-		snprintf(path_name, 4096, "%s/%s", dir, filename);
+		/* full path from root of btrfs being restored */
+		snprintf(fs_name, 4096, "%s/%s", dir, filename);
 
+		/* full path from system root */
+		snprintf(path_name, 4096, "%s%s", output_rootdir, fs_name);
 
 		/*
 		 * At this point we're only going to restore directories and
@@ -607,7 +611,7 @@ static int search_dir(struct btrfs_root *root, struct btrfs_key *key,
 			}
 		} else if (type == BTRFS_FT_DIR) {
 			struct btrfs_root *search_root = root;
-			char *dir = strdup(path_name);
+			char *dir = strdup(fs_name);
 
 			if (!dir) {
 				fprintf(stderr, "Ran out of memory\n");
@@ -669,7 +673,8 @@ static int search_dir(struct btrfs_root *root, struct btrfs_key *key,
 				return -1;
 			}
 			loops = 0;
-			ret = search_dir(search_root, &location, dir);
+			ret = search_dir(search_root, &location,
+					 output_rootdir, dir);
 			free(dir);
 			if (ret) {
 				if (ignore_errors)
@@ -911,7 +916,7 @@ int cmd_restore(int argc, char **argv)
 		key.objectid = BTRFS_FIRST_FREE_OBJECTID;
 	}
 
-	ret = search_dir(root, &key, dir_name);
+	ret = search_dir(root, &key, dir_name, "");
 
 out:
 	close_ctree(root);
-- 
1.7.7.6

[PATCH 06/10] restore: Add regex matching of paths and files to be restored

[Josef Bacik]

From: Peter Stuge <peter@stuge.se>

The option -m is used to specify the regex string. -c is used to
specify case insensitive matching. -i was already taken.

In order to restore only a single folder somewhere in the btrfs
tree, it is unfortunately neccessary to construct a slightly
nontrivial regex, e.g.:

restore -m '^/(|home(|/username(|/Desktop(|/.*))))$' /dev/sdb2 /output

This is needed in order to match each directory along the way to the
Desktop directory, as well as all contents below the Desktop directory.

Signed-off-by: Peter Stuge <peter@stuge.se>
Signed-off-by: Josef Bacik <josef@redhat.com>
---
 cmds-restore.c |   38 +++++++++++++++++++++++++++++++++-----
 1 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/cmds-restore.c b/cmds-restore.c
index 215958f..365cd15 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -27,8 +27,10 @@
 #include <unistd.h>
 #include <fcntl.h>
 #include <sys/stat.h>
+#include <sys/types.h>
 #include <zlib.h>
-
+#include <regex.h>
+#include "kerncompat.h"
 #include "ctree.h"
 #include "disk-io.h"
 #include "print-tree.h"
@@ -455,7 +457,8 @@ set_size:
 }
 
 static int search_dir(struct btrfs_root *root, struct btrfs_key *key,
-		      const char *output_rootdir, const char *dir)
+		      const char *output_rootdir, const char *dir,
+		      const regex_t *mreg)
 {
 	struct btrfs_path *path;
 	struct extent_buffer *leaf;
@@ -562,6 +565,9 @@ static int search_dir(struct btrfs_root *root, struct btrfs_key *key,
 		/* full path from root of btrfs being restored */
 		snprintf(fs_name, 4096, "%s/%s", dir, filename);
 
+		if (REG_NOMATCH == regexec(mreg, fs_name, 0, NULL, 0))
+			goto next;
+
 		/* full path from system root */
 		snprintf(path_name, 4096, "%s%s", output_rootdir, fs_name);
 
@@ -674,7 +680,7 @@ static int search_dir(struct btrfs_root *root, struct btrfs_key *key,
 			}
 			loops = 0;
 			ret = search_dir(search_root, &location,
-					 output_rootdir, dir);
+					 output_rootdir, dir, mreg);
 			free(dir);
 			if (ret) {
 				if (ignore_errors)
@@ -798,8 +804,12 @@ int cmd_restore(int argc, char **argv)
 	int opt;
 	int super_mirror = 0;
 	int find_dir = 0;
+	const char *match_regstr = NULL;
+	int match_cflags = REG_EXTENDED | REG_NOSUB | REG_NEWLINE;
+	regex_t match_reg, *mreg = NULL;
+	char reg_err[256];
 
-	while ((opt = getopt(argc, argv, "sviot:u:df:r:")) != -1) {
+	while ((opt = getopt(argc, argv, "sviot:u:df:r:cm:")) != -1) {
 		switch (opt) {
 			case 's':
 				get_snaps = 1;
@@ -850,6 +860,12 @@ int cmd_restore(int argc, char **argv)
 					exit(1);
 				}
 				break;
+			case 'c':
+				match_cflags |= REG_ICASE;
+				break;
+			case 'm':
+				match_regstr = optarg;
+				break;
 			default:
 				usage(cmd_restore_usage);
 		}
@@ -916,9 +932,21 @@ int cmd_restore(int argc, char **argv)
 		key.objectid = BTRFS_FIRST_FREE_OBJECTID;
 	}
 
-	ret = search_dir(root, &key, dir_name, "");
+	if (match_regstr) {
+		ret = regcomp(&match_reg, match_regstr, match_cflags);
+		if (ret) {
+			regerror(ret, &match_reg, reg_err, sizeof(reg_err));
+			fprintf(stderr, "Regex compile failed: %s\n", reg_err);
+			goto out;
+		}
+		mreg = &match_reg;
+	}
+
+	ret = search_dir(root, &key, dir_name, "", mreg);
 
 out:
+	if (mreg)
+		regfree(mreg);
 	close_ctree(root);
 	return ret;
 }
-- 
1.7.7.6

[PATCH 07/10] btrfs-progs: add lzo compression support to restore

[Josef Bacik]

From: Josef Bacik <josef@redhat.com>

This patch simply adds support to decompress lzo compressed extents in restore.

Signed-off-by: Josef Bacik <josef@redhat.com>
---
 Makefile       |    2 +-
 cmds-restore.c |   89 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 84 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index d102dee..50e2892 100644
--- a/Makefile
+++ b/Makefile
@@ -24,7 +24,7 @@ DEPFLAGS = -Wp,-MMD,$(@D)/.$(@F).d,-MT,$@
 INSTALL = install
 prefix ?= /usr/local
 bindir = $(prefix)/bin
-lib_LIBS = -luuid -lblkid -lm -lz -L.
+lib_LIBS = -luuid -lblkid -lm -lz -llzo2 -L.
 libdir ?= $(prefix)/lib
 incdir = $(prefix)/include/btrfs
 LIBS = $(lib_LIBS) $(libs_static)
diff --git a/cmds-restore.c b/cmds-restore.c
index 365cd15..3e0ada2 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -30,6 +30,8 @@
 #include <sys/types.h>
 #include <zlib.h>
 #include <regex.h>
+#include <lzo/lzoconf.h>
+#include <lzo/lzo1x.h>
 #include "kerncompat.h"
 #include "ctree.h"
 #include "disk-io.h"
@@ -48,8 +50,12 @@ static int verbose = 0;
 static int ignore_errors = 0;
 static int overwrite = 0;
 
-static int decompress(char *inbuf, char *outbuf, u64 compress_len,
-		      u64 decompress_len)
+#define LZO_LEN 4
+#define PAGE_CACHE_SIZE 4096
+#define lzo1x_worst_compress(x) ((x) + ((x) / 16) + 64 + 3)
+
+static int decompress_zlib(char *inbuf, char *outbuf, u64 compress_len,
+			   u64 decompress_len)
 {
 	z_stream strm;
 	int ret;
@@ -75,6 +81,73 @@ static int decompress(char *inbuf, char *outbuf, u64 compress_len,
 	(void)inflateEnd(&strm);
 	return 0;
 }
+static inline size_t read_compress_length(unsigned char *buf)
+{
+	__le32 dlen;
+	memcpy(&dlen, buf, LZO_LEN);
+	return le32_to_cpu(dlen);
+}
+
+static int decompress_lzo(unsigned char *inbuf, char *outbuf, u64 compress_len,
+			  u64 *decompress_len)
+{
+	size_t new_len;
+	size_t in_len;
+	size_t out_len = 0;
+	size_t tot_len;
+	size_t tot_in;
+	int ret;
+
+	ret = lzo_init();
+	if (ret != LZO_E_OK) {
+		fprintf(stderr, "lzo init returned %d\n", ret);
+		return -1;
+	}
+
+	tot_len = read_compress_length(inbuf);
+	inbuf += LZO_LEN;
+	tot_in = LZO_LEN;
+
+	while (tot_in < tot_len) {
+		in_len = read_compress_length(inbuf);
+		inbuf += LZO_LEN;
+		tot_in += LZO_LEN;
+
+		new_len = lzo1x_worst_compress(PAGE_CACHE_SIZE);
+		ret = lzo1x_decompress_safe((const unsigned char *)inbuf, in_len,
+					    (unsigned char *)outbuf, &new_len, NULL);
+		if (ret != LZO_E_OK) {
+			fprintf(stderr, "failed to inflate: %d\n", ret);
+			return -1;
+		}
+		out_len += new_len;
+		outbuf += new_len;
+		inbuf += in_len;
+		tot_in += in_len;
+	}
+
+	*decompress_len = out_len;
+
+	return 0;
+}
+
+static int decompress(char *inbuf, char *outbuf, u64 compress_len,
+		      u64 *decompress_len, int compress)
+{
+	switch (compress) {
+	case BTRFS_COMPRESS_ZLIB:
+		return decompress_zlib(inbuf, outbuf, compress_len,
+				       *decompress_len);
+	case BTRFS_COMPRESS_LZO:
+		return decompress_lzo((unsigned char *)inbuf, outbuf, compress_len,
+				      decompress_len);
+	default:
+		break;
+	}
+
+	fprintf(stderr, "invalid compression type: %d\n", compress);
+	return -1;
+}
 
 int next_leaf(struct btrfs_root *root, struct btrfs_path *path)
 {
@@ -134,11 +207,11 @@ static int copy_one_inline(int fd, struct btrfs_path *path, u64 pos)
 	struct btrfs_file_extent_item *fi;
 	char buf[4096];
 	char *outbuf;
+	u64 ram_size;
 	ssize_t done;
 	unsigned long ptr;
 	int ret;
 	int len;
-	int ram_size;
 	int compress;
 
 	fi = btrfs_item_ptr(leaf, path->slots[0],
@@ -166,7 +239,7 @@ static int copy_one_inline(int fd, struct btrfs_path *path, u64 pos)
 		return -1;
 	}
 
-	ret = decompress(buf, outbuf, len, ram_size);
+	ret = decompress(buf, outbuf, len, &ram_size, compress);
 	if (ret) {
 		free(outbuf);
 		return ret;
@@ -175,7 +248,7 @@ static int copy_one_inline(int fd, struct btrfs_path *path, u64 pos)
 	done = pwrite(fd, outbuf, ram_size, pos);
 	free(outbuf);
 	if (done < ram_size) {
-		fprintf(stderr, "Short compressed inline write, wanted %d, "
+		fprintf(stderr, "Short compressed inline write, wanted %Lu, "
 			"did %zd: %d\n", ram_size, done, errno);
 		return -1;
 	}
@@ -197,6 +270,7 @@ static int copy_one_extent(struct btrfs_root *root, int fd,
 	u64 length;
 	u64 size_left;
 	u64 dev_bytenr;
+	u64 offset;
 	u64 count = 0;
 	int compress;
 	int ret;
@@ -208,8 +282,11 @@ static int copy_one_extent(struct btrfs_root *root, int fd,
 	bytenr = btrfs_file_extent_disk_bytenr(leaf, fi);
 	disk_size = btrfs_file_extent_disk_num_bytes(leaf, fi);
 	ram_size = btrfs_file_extent_ram_bytes(leaf, fi);
+	offset = btrfs_file_extent_offset(leaf, fi);
 	size_left = disk_size;
 
+	if (offset)
+		printf("offset is %Lu\n", offset);
 	/* we found a hole */
 	if (disk_size == 0)
 		return 0;
@@ -283,7 +360,7 @@ again:
 		goto out;
 	}
 
-	ret = decompress(inbuf, outbuf, disk_size, ram_size);
+	ret = decompress(inbuf, outbuf, disk_size, &ram_size, compress);
 	if (ret) {
 		num_copies = btrfs_num_copies(&root->fs_info->mapping_tree,
 					      bytenr, length);
-- 
1.7.7.6

[PATCH 08/10] btrfs-progs: fix regexec to only work if we actually have a regexec

[Josef Bacik]

From: Josef Bacik <josef@redhat.com>

We were unconditionally executing our regular expression, even though we may not
have one, so check to make sure mreg is not null before calling regexec.
Thanks,

Signed-off-by: Josef Bacik <josef@redhat.com>
---
 cmds-restore.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/cmds-restore.c b/cmds-restore.c
index 3e0ada2..7e99970 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -642,7 +642,7 @@ static int search_dir(struct btrfs_root *root, struct btrfs_key *key,
 		/* full path from root of btrfs being restored */
 		snprintf(fs_name, 4096, "%s/%s", dir, filename);
 
-		if (REG_NOMATCH == regexec(mreg, fs_name, 0, NULL, 0))
+		if (mreg && REG_NOMATCH == regexec(mreg, fs_name, 0, NULL, 0))
 			goto next;
 
 		/* full path from system root */
-- 
1.7.7.6

[PATCH 09/10] Btrfs-progs: give restore a list roots option

[Josef Bacik]

From: Josef Bacik <josef@redhat.com>

Since restore has the ability to open really really screwed up file systems, add
a list roots option to it so we can still get the contents of the tree root on a
horribly broken fs.  Thanks,

Signed-off-by: Josef Bacik <josef@redhat.com>
---
 cmds-restore.c |   92 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 85 insertions(+), 7 deletions(-)

diff --git a/cmds-restore.c b/cmds-restore.c
index 7e99970..f4e75cf 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -776,9 +776,70 @@ next:
 	return 0;
 }
 
-static struct btrfs_root *open_fs(const char *dev, u64 root_location, int super_mirror)
+static int do_list_roots(struct btrfs_root *root)
 {
-	struct btrfs_root *root;
+	struct btrfs_key key;
+	struct btrfs_key found_key;
+	struct btrfs_disk_key disk_key;
+	struct btrfs_path *path;
+	struct extent_buffer *leaf;
+	struct btrfs_root_item ri;
+	unsigned long offset;
+	int slot;
+	int ret;
+
+	root = root->fs_info->tree_root;
+	path = btrfs_alloc_path();
+	if (!path) {
+		fprintf(stderr, "Failed to alloc path\n");
+		return -1;
+	}
+
+	key.offset = 0;
+	key.objectid = 0;
+	key.type = BTRFS_ROOT_ITEM_KEY;
+
+	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
+	if (ret < 0) {
+		fprintf(stderr, "Failed to do search %d\n", ret);
+		btrfs_free_path(path);
+		return -1;
+	}
+
+	while (1) {
+		leaf = path->nodes[0];
+		slot = path->slots[0];
+		if (slot >= btrfs_header_nritems(leaf)) {
+			ret = btrfs_next_leaf(root, path);
+			if (ret)
+				break;
+			leaf = path->nodes[0];
+			slot = path->slots[0];
+		}
+		btrfs_item_key(leaf, &disk_key, slot);
+		btrfs_disk_key_to_cpu(&found_key, &disk_key);
+		if (btrfs_key_type(&found_key) != BTRFS_ROOT_ITEM_KEY) {
+			path->slots[0]++;
+			continue;
+		}
+
+		offset = btrfs_item_ptr_offset(leaf, slot);
+		read_extent_buffer(leaf, &ri, offset, sizeof(ri));
+		printf(" tree ");
+		btrfs_print_key(&disk_key);
+		printf(" %Lu level %d\n", btrfs_root_bytenr(&ri),
+		       btrfs_root_level(&ri));
+		path->slots[0]++;
+	}
+	btrfs_free_path(path);
+
+	return 0;
+}
+
+static struct btrfs_root *open_fs(const char *dev, u64 root_location,
+				  int super_mirror, int list_roots)
+{
+	struct btrfs_root *root = NULL;
 	u64 bytenr;
 	int i;
 
@@ -786,11 +847,19 @@ static struct btrfs_root *open_fs(const char *dev, u64 root_location, int super_
 		bytenr = btrfs_sb_offset(i);
 		root = open_ctree_recovery(dev, bytenr, root_location);
 		if (root)
-			return root;
+			break;
 		fprintf(stderr, "Could not open root, trying backup super\n");
 	}
 
-	return NULL;
+	if (root && list_roots) {
+		int ret = do_list_roots(root);
+		if (ret) {
+			close_ctree(root);
+			root = NULL;
+		}
+	}
+
+	return root;
 }
 
 static int find_first_dir(struct btrfs_root *root, u64 *objectid)
@@ -885,8 +954,9 @@ int cmd_restore(int argc, char **argv)
 	int match_cflags = REG_EXTENDED | REG_NOSUB | REG_NEWLINE;
 	regex_t match_reg, *mreg = NULL;
 	char reg_err[256];
+	int list_roots = 0;
 
-	while ((opt = getopt(argc, argv, "sviot:u:df:r:cm:")) != -1) {
+	while ((opt = getopt(argc, argv, "sviot:u:df:r:cm:l")) != -1) {
 		switch (opt) {
 			case 's':
 				get_snaps = 1;
@@ -943,12 +1013,17 @@ int cmd_restore(int argc, char **argv)
 			case 'm':
 				match_regstr = optarg;
 				break;
+			case 'l':
+				list_roots = 1;
+				break;
 			default:
 				usage(cmd_restore_usage);
 		}
 	}
 
-	if (optind + 1 >= argc)
+	if (!list_roots && optind + 1 >= argc)
+		usage(cmd_restore_usage);
+	else if (list_roots && optind >= argc)
 		usage(cmd_restore_usage);
 
 	if ((ret = check_mounted(argv[optind])) < 0) {
@@ -960,10 +1035,13 @@ int cmd_restore(int argc, char **argv)
 		return 1;
 	}
 
-	root = open_fs(argv[optind], tree_location, super_mirror);
+	root = open_fs(argv[optind], tree_location, super_mirror, list_roots);
 	if (root == NULL)
 		return 1;
 
+	if (list_roots)
+		goto out;
+
 	if (fs_location != 0) {
 		free_extent_buffer(root->node);
 		root->node = read_tree_block(root, fs_location, 4096, 0);
-- 
1.7.7.6

[PATCH 10/10] Btrfs-progs: make restore deal with really broken file systems

[Josef Bacik]

All we need for restore to work is the chunk root, the tree root and the fs root
we want to restore from.  So to do this we need to make a few adjustments

1) Make open_ctree_fs_info fail completely if it can't read the chunk tree.
There is no sense in continuing if we can't read the chunk tree since we won't
be able to translate logical to physical blocks.

2) Use open_ctree_fs_info in restore, and if we didn't load a tree root or
fs root go ahead and try to set those up manually ourselves.

This is related to work I did last year on restore, but it uses the
open_ctree_fs_info instead of my open coded open_ctree.  Thanks,

Signed-off-by: Josef Bacik <jbacik@fusionio.com>
---
 cmds-check.c   |    2 +-
 cmds-restore.c |   52 ++++++++++++++++++++++++++++++++++++++++++++++------
 debug-tree.c   |    2 +-
 disk-io.c      |   37 ++++++++++---------------------------
 disk-io.h      |    6 ++----
 5 files changed, 60 insertions(+), 39 deletions(-)

diff --git a/cmds-check.c b/cmds-check.c
index 12192fa..bdf74ba 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -3609,7 +3609,7 @@ int cmd_check(int argc, char **argv)
 		return -EBUSY;
 	}
 
-	info = open_ctree_fs_info(argv[optind], bytenr, rw, 1);
+	info = open_ctree_fs_info(argv[optind], bytenr, 0, rw, 1);
 	if (info == NULL)
 		return 1;
 
diff --git a/cmds-restore.c b/cmds-restore.c
index f4e75cf..4760672 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -839,27 +839,67 @@ static int do_list_roots(struct btrfs_root *root)
 static struct btrfs_root *open_fs(const char *dev, u64 root_location,
 				  int super_mirror, int list_roots)
 {
+	struct btrfs_fs_info *fs_info = NULL;
 	struct btrfs_root *root = NULL;
 	u64 bytenr;
 	int i;
 
 	for (i = super_mirror; i < BTRFS_SUPER_MIRROR_MAX; i++) {
 		bytenr = btrfs_sb_offset(i);
-		root = open_ctree_recovery(dev, bytenr, root_location);
-		if (root)
+		fs_info = open_ctree_fs_info(dev, bytenr, root_location, 0, 1);
+		if (fs_info)
 			break;
 		fprintf(stderr, "Could not open root, trying backup super\n");
 	}
 
-	if (root && list_roots) {
-		int ret = do_list_roots(root);
-		if (ret) {
+	if (!fs_info)
+		return NULL;
+
+	/*
+	 * All we really need to succeed is reading the chunk tree, everything
+	 * else we can do by hand, since we only need to read the tree root and
+	 * the fs_root.
+	 */
+	if (!extent_buffer_uptodate(fs_info->tree_root->node)) {
+		u64 generation;
+
+		root = fs_info->tree_root;
+		if (!root_location)
+			root_location = btrfs_super_root(fs_info->super_copy);
+		generation = btrfs_super_generation(fs_info->super_copy);
+		root->node = read_tree_block(root, root_location,
+					     root->leafsize, generation);
+		if (!extent_buffer_uptodate(root->node)) {
+			fprintf(stderr, "Error opening tree root\n");
 			close_ctree(root);
+			return NULL;
+		}
+	}
+
+	if (!list_roots && !fs_info->fs_root) {
+		struct btrfs_key key;
+
+		key.objectid = BTRFS_FS_TREE_OBJECTID;
+		key.type = BTRFS_ROOT_ITEM_KEY;
+		key.offset = (u64)-1;
+		fs_info->fs_root = btrfs_read_fs_root_no_cache(fs_info, &key);
+		if (IS_ERR(fs_info->fs_root)) {
+			fprintf(stderr, "Couldn't read fs root: %ld\n",
+				PTR_ERR(fs_info->fs_root));
+			close_ctree(fs_info->tree_root);
+			return NULL;
+		}
+	}
+
+	if (list_roots) {
+		int ret = do_list_roots(fs_info->tree_root);
+		if (ret) {
+			close_ctree(fs_info->tree_root);
 			root = NULL;
 		}
 	}
 
-	return root;
+	return fs_info->fs_root;
 }
 
 static int find_first_dir(struct btrfs_root *root, u64 *objectid)
diff --git a/debug-tree.c b/debug-tree.c
index 0fc0ecd..bae7f94 100644
--- a/debug-tree.c
+++ b/debug-tree.c
@@ -166,7 +166,7 @@ int main(int ac, char **av)
 	if (ac != 1)
 		print_usage();
 
-	info = open_ctree_fs_info(av[optind], 0, 0, 1);
+	info = open_ctree_fs_info(av[optind], 0, 0, 0, 1);
 	if (!info) {
 		fprintf(stderr, "unable to open %s\n", av[optind]);
 		exit(1);
diff --git a/disk-io.c b/disk-io.c
index a9fd374..5265c3c 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -944,8 +944,10 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path,
 
 	if (!(btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_METADUMP)) {
 		ret = btrfs_read_chunk_tree(chunk_root);
-		if (ret)
-			goto out_failed;
+		if (ret) {
+			printk("Couldn't read chunk tree\n");
+			goto out_chunk;
+		}
 	}
 
 	blocksize = btrfs_level_size(tree_root,
@@ -1018,6 +1020,7 @@ out_failed:
 		free_extent_buffer(fs_info->extent_root->node);
 	if (fs_info->tree_root)
 		free_extent_buffer(fs_info->tree_root->node);
+out_chunk:
 	if (fs_info->chunk_root)
 		free_extent_buffer(fs_info->chunk_root->node);
 out_devices:
@@ -1040,8 +1043,8 @@ out:
 }
 
 struct btrfs_fs_info *open_ctree_fs_info(const char *filename,
-					 u64 sb_bytenr, int writes,
-					 int partial)
+					 u64 sb_bytenr, u64 root_tree_bytenr,
+					 int writes, int partial)
 {
 	int fp;
 	struct btrfs_fs_info *info;
@@ -1055,7 +1058,8 @@ struct btrfs_fs_info *open_ctree_fs_info(const char *filename,
 		fprintf (stderr, "Could not open %s\n", filename);
 		return NULL;
 	}
-	info = __open_ctree_fd(fp, filename, sb_bytenr, 0, writes, partial);
+	info = __open_ctree_fd(fp, filename, sb_bytenr, root_tree_bytenr,
+			       writes, partial);
 	close(fp);
 	return info;
 }
@@ -1064,28 +1068,7 @@ struct btrfs_root *open_ctree(const char *filename, u64 sb_bytenr, int writes)
 {
 	struct btrfs_fs_info *info;
 
-	info = open_ctree_fs_info(filename, sb_bytenr, writes, 0);
-	if (!info)
-		return NULL;
-	return info->fs_root;
-}
-
-struct btrfs_root *open_ctree_recovery(const char *filename, u64 sb_bytenr,
-				       u64 root_tree_bytenr)
-{
-	int fp;
-	struct btrfs_fs_info *info;
-
-
-	fp = open(filename, O_RDONLY);
-	if (fp < 0) {
-		fprintf (stderr, "Could not open %s\n", filename);
-		return NULL;
-	}
-	info = __open_ctree_fd(fp, filename, sb_bytenr,
-			       root_tree_bytenr, 0, 0);
-	close(fp);
-
+	info = open_ctree_fs_info(filename, sb_bytenr, 0, writes, 0);
 	if (!info)
 		return NULL;
 	return info->fs_root;
diff --git a/disk-io.h b/disk-io.h
index ff87958..c29ee8e 100644
--- a/disk-io.h
+++ b/disk-io.h
@@ -50,11 +50,9 @@ int clean_tree_block(struct btrfs_trans_handle *trans,
 struct btrfs_root *open_ctree(const char *filename, u64 sb_bytenr, int writes);
 struct btrfs_root *open_ctree_fd(int fp, const char *path, u64 sb_bytenr,
 				 int writes);
-struct btrfs_root *open_ctree_recovery(const char *filename, u64 sb_bytenr,
-				       u64 root_tree_bytenr);
 struct btrfs_fs_info *open_ctree_fs_info(const char *filename,
-					 u64 sb_bytenr, int writes,
-					 int partial);
+					 u64 sb_bytenr, u64 root_tree_bytenr,
+					 int writes, int partial);
 int close_ctree(struct btrfs_root *root);
 int write_all_supers(struct btrfs_root *root);
 int write_ctree_super(struct btrfs_trans_handle *trans,
-- 
1.7.7.6

Re: Btrfs-progs: Update btrfs restore so it matches all current development

[Chris Mason]

On Tue, Mar 12, 2013 at 11:38:07AM -0600, Josef Bacik wrote:
> Hello,
> 
> I noticed today that a bunch of work that had been done to restore last year
> wasn't in the normal btrfs-progs restore.  So this work forward ports all of the
> patches that were missing, and redoes a bunch of the work that was done to deal
> with really broken file systems so that it uses more of the generic helpers that
> Chris wrote a while ago.  This is based on the latest integration branch, Dave
> if you want to just pull you can do
> 
> git://github.com/josefbacik/btrfs-progs.git for-kdave

Thanks Josef, this seems like a great time for me to pull integration
again.

-chris

Re: [PATCH 06/10] restore: Add regex matching of paths and files to be restored

[Zach Brown]

> The option -m is used to specify the regex string. -c is used to
> specify case insensitive matching. -i was already taken.

I guess that's cool if someone really wanted to use the full power of
regexes.

> In order to restore only a single folder somewhere in the btrfs
> tree, it is unfortunately neccessary to construct a slightly
> nontrivial regex, e.g.:
> 
> restore -m '^/(|home(|/username(|/Desktop(|/.*))))$' /dev/sdb2 /output

But that example is just bonkers and suggests that we should also
support globs for fnmatch(3). (-g is for glob? :))

> +	const char *match_regstr = NULL;
> +	int match_cflags = REG_EXTENDED | REG_NOSUB | REG_NEWLINE;
> +	regex_t match_reg, *mreg = NULL;
> +	char reg_err[256];


> +	if (match_regstr) {
> +		ret = regcomp(&match_reg, match_regstr, match_cflags);
> +		if (ret) {
> +			regerror(ret, &match_reg, reg_err, sizeof(reg_err));
> +			fprintf(stderr, "Regex compile failed: %s\n", reg_err);
> +			goto out;
> +		}
> +		mreg = &match_reg;
> +	}

And this should probably be in a helper function.  Especially if it also
grows fnmatch() support.

- z

Re: [PATCH 06/10] restore: Add regex matching of paths and files to be restored

[Peter Stuge]

Hi Zach,

Zach Brown wrote:
> > In order to restore only a single folder somewhere in the btrfs
> > tree, it is unfortunately neccessary to construct a slightly
> > nontrivial regex, e.g.:
> > 
> > restore -m '^/(|home(|/username(|/Desktop(|/.*))))$' /dev/sdb2 /output
> 
> But that example is just bonkers

I agree ugly, but why bonkers (I understand that to mean silly) ?


> and suggests that we should also support globs for fnmatch(3).
> (-g is for glob? :))

I like the idea better than my regex. The implementation doing
matching should however iterate over all path components, so that
it's not neccessary to do

-g / -g /home -g /home/username -g /home/username/Desktop -g '/home/username/Desktop/*'

but to make this suffice:

-g '/home/username/Desktop/*'

Chopping up the string is easy enough.


> > +	if (match_regstr) {
> > +		ret = regcomp(&match_reg, match_regstr, match_cflags);
> 
> And this should probably be in a helper function.  Especially if it
> also grows fnmatch() support.

I agree. Maybe I'll take some time to make the changes tomorrow.


//Peter

Re: [PATCH 06/10] restore: Add regex matching of paths and files to be restored

[Zach Brown]

> > > restore -m '^/(|home(|/username(|/Desktop(|/.*))))$' /dev/sdb2 /output
> > 
> > But that example is just bonkers
> 
> I agree ugly, but why bonkers (I understand that to mean silly) ?

Well, I guess I mean that it's so ugly that we can't reasonably expect
people to use it :).

> I like the idea better than my regex. The implementation doing
> matching should however iterate over all path components, so that
> it's not neccessary to do
> 
> -g / -g /home -g /home/username -g /home/username/Desktop -g '/home/username/Desktop/*'
> 
> but to make this suffice:
> 
> -g '/home/username/Desktop/*'
> 
> Chopping up the string is easy enough.

Yeah, agreed.  It's annoying that we have to do this by hand :/.

- z

Re: [PATCH 04/10] btrfs-progs: Fix error handling for failed reads in restore tool when mirrors exist

[David Sterba]

On Tue, Mar 12, 2013 at 01:38:11PM -0400, Josef Bacik wrote:
> From: David Marcin <djmarcin@google.com>

Missing signed-off from David, please let us know if it's ok to add it.

thanks,
david

---
>  cmds-restore.c |   11 +++++++----
>  1 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/cmds-restore.c b/cmds-restore.c
> index 617f507..9781801 100644
> --- a/cmds-restore.c
> +++ b/cmds-restore.c
> @@ -198,7 +198,7 @@ static int copy_one_extent(struct btrfs_root *root, int fd,
>  	int compress;
>  	int ret;
>  	int dev_fd;
> -	int mirror_num = 0;
> +	int mirror_num = 1;
>  	int num_copies;
>  
>  	compress = btrfs_file_extent_compression(leaf, fi);
> @@ -241,14 +241,15 @@ again:
>  
>  	if (size_left < length)
>  		length = size_left;
> -	size_left -= length;
>  
>  	done = pread(dev_fd, inbuf+count, length, dev_bytenr);
> -	if (done < length) {
> +	/* Need both checks, or we miss negative values due to u64 conversion */
> +	if (done < 0 || done < length) {
>  		num_copies = btrfs_num_copies(&root->fs_info->mapping_tree,
>  					      bytenr, length);
>  		mirror_num++;
> -		if (mirror_num >= num_copies) {
> +		/* mirror_num is 1-indexed, so num_copies is a valid mirror. */
> +		if (mirror_num > num_copies) {
>  			ret = -1;
>  			fprintf(stderr, "Exhausted mirrors trying to read\n");
>  			goto out;
> @@ -257,6 +258,8 @@ again:
>  		goto again;
>  	}
>  
> +	mirror_num = 1;
> +	size_left -= length;
>  	count += length;
>  	bytenr += length;
>  	if (size_left)
> -- 

Re: [PATCH 05/10] restore: Split output directory and btrfs-local path search_dir() parameters

[David Sterba]

On Tue, Mar 12, 2013 at 01:38:12PM -0400, Josef Bacik wrote:
> --- a/cmds-restore.c
> +++ b/cmds-restore.c
> @@ -39,6 +39,7 @@
>  #include "utils.h"
>  #include "commands.h"
>  
> +static char fs_name[4096];

Path handling should use PATH_MAX, but I'm ok with a separate patch to
fix all of them in one go.

>  static char path_name[4096];

david

Re: [PATCH 04/10] btrfs-progs: Fix error handling for failed reads in restore tool when mirrors exist

[David Marcin]

On Thu, Mar 14, 2013 at 4:10 PM, David Sterba <dsterba@suse.cz> wrote:
>
> On Tue, Mar 12, 2013 at 01:38:11PM -0400, Josef Bacik wrote:
> > From: David Marcin <djmarcin@google.com>
>
> Missing signed-off from David, please let us know if it's ok to add it.
>
> thanks,
> david
>

Yes, OK to add.  Sorry for missing that the first time around.

David

Re: [PATCH 10/10] Btrfs-progs: make restore deal with really broken file systems

[David Sterba]

On Tue, Mar 12, 2013 at 01:38:17PM -0400, Josef Bacik wrote:
> --- a/cmds-restore.c
> +++ b/cmds-restore.c
> @@ -839,27 +839,67 @@ static int do_list_roots(struct btrfs_root *root)
>  static struct btrfs_root *open_fs(const char *dev, u64 root_location,
>  				  int super_mirror, int list_roots)
>  {
> +	struct btrfs_fs_info *fs_info = NULL;
>  	struct btrfs_root *root = NULL;
>  	u64 bytenr;
>  	int i;
>  
>  	for (i = super_mirror; i < BTRFS_SUPER_MIRROR_MAX; i++) {
>  		bytenr = btrfs_sb_offset(i);
> -		root = open_ctree_recovery(dev, bytenr, root_location);
> -		if (root)
> +		fs_info = open_ctree_fs_info(dev, bytenr, root_location, 0, 1);
> +		if (fs_info)
>  			break;
>  		fprintf(stderr, "Could not open root, trying backup super\n");
>  	}
>  
> -	if (root && list_roots) {
> -		int ret = do_list_roots(root);
> -		if (ret) {
> +	if (!fs_info)
> +		return NULL;
> +
> +	/*
> +	 * All we really need to succeed is reading the chunk tree, everything
> +	 * else we can do by hand, since we only need to read the tree root and
> +	 * the fs_root.
> +	 */
> +	if (!extent_buffer_uptodate(fs_info->tree_root->node)) {
> +		u64 generation;
> +
> +		root = fs_info->tree_root;
> +		if (!root_location)
> +			root_location = btrfs_super_root(fs_info->super_copy);
> +		generation = btrfs_super_generation(fs_info->super_copy);
> +		root->node = read_tree_block(root, root_location,
> +					     root->leafsize, generation);
> +		if (!extent_buffer_uptodate(root->node)) {
> +			fprintf(stderr, "Error opening tree root\n");
>  			close_ctree(root);
> +			return NULL;
> +		}
> +	}
> +
> +	if (!list_roots && !fs_info->fs_root) {
> +		struct btrfs_key key;
> +
> +		key.objectid = BTRFS_FS_TREE_OBJECTID;
> +		key.type = BTRFS_ROOT_ITEM_KEY;
> +		key.offset = (u64)-1;
> +		fs_info->fs_root = btrfs_read_fs_root_no_cache(fs_info, &key);
> +		if (IS_ERR(fs_info->fs_root)) {
> +			fprintf(stderr, "Couldn't read fs root: %ld\n",
> +				PTR_ERR(fs_info->fs_root));
> +			close_ctree(fs_info->tree_root);
> +			return NULL;
> +		}
> +	}
> +
> +	if (list_roots) {
> +		int ret = do_list_roots(fs_info->tree_root);
> +		if (ret) {
> +			close_ctree(fs_info->tree_root);
>  			root = NULL;

Setting root to NULL here has no effect, I wonder if this should be
'return NULL' instead, or if it's really ok to return fs_info->fs_root
even after close_ctree() which calls free_fs_roots and other cleanups.


>  		}
>  	}
>  
> -	return root;
> +	return fs_info->fs_root;
>  }

Re: Btrfs-progs: Update btrfs restore so it matches all current development

[David Sterba]

On Tue, Mar 12, 2013 at 01:38:07PM -0400, Josef Bacik wrote:
> I noticed today that a bunch of work that had been done to restore last year
> wasn't in the normal btrfs-progs restore.  So this work forward ports all of the
> patches that were missing, and redoes a bunch of the work that was done to deal
> with really broken file systems so that it uses more of the generic helpers that
> Chris wrote a while ago.  This is based on the latest integration branch, Dave
> if you want to just pull you can do

I've pulled 1-5,7,9 (ie. left out 6,8 -- regex, and 10 which I need to
look at again).

david

Re: [PATCH 10/10] Btrfs-progs: make restore deal with really broken file systems

[David Sterba]

On Fri, Mar 15, 2013 at 12:37:05AM +0100, David Sterba wrote:
> On Tue, Mar 12, 2013 at 01:38:17PM -0400, Josef Bacik wrote:
> > --- a/cmds-restore.c
> > +++ b/cmds-restore.c
> > +	if (list_roots) {
> > +		int ret = do_list_roots(fs_info->tree_root);
> > +		if (ret) {
> > +			close_ctree(fs_info->tree_root);
> >  			root = NULL;
> 
> Setting root to NULL here has no effect, I wonder if this should be
> 'return NULL' instead, or if it's really ok to return fs_info->fs_root
> even after close_ctree() which calls free_fs_roots and other cleanups.

It's no ok, close_tree does free(fs_info), so it's returning garbage,
the caller jumps to (another) close_ctree if it's in list_root mode (and
trying to free the fs_info again).

> 
> >  		}
> >  	}
> >  
> > -	return root;
> > +	return fs_info->fs_root;