Josef Bacik
2013-Mar-12 17:38 UTC
Btrfs-progs: Update btrfs restore so it matches all current development
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 -- 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
Josef Bacik
2013-Mar-12 17:38 UTC
[PATCH 01/10] Btrfs-progs: add an option for specifying the root to restore
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 -- 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
Josef Bacik
2013-Mar-12 17:38 UTC
[PATCH 02/10] Btrfs-progs: try other mirrors if decomression fails
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 -- 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
Josef Bacik
2013-Mar-12 17:38 UTC
[PATCH 03/10] Btrfs-progs: try other mirrors on read failure
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 -- 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
Josef Bacik
2013-Mar-12 17:38 UTC
[PATCH 04/10] btrfs-progs: Fix error handling for failed reads in restore tool when mirrors exist
From: 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 -- 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
Josef Bacik
2013-Mar-12 17:38 UTC
[PATCH 05/10] restore: Split output directory and btrfs-local path search_dir() parameters
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 -- 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
Josef Bacik
2013-Mar-12 17:38 UTC
[PATCH 06/10] restore: Add regex matching of paths and files to be restored
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 -- 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
Josef Bacik
2013-Mar-12 17:38 UTC
[PATCH 07/10] btrfs-progs: add lzo compression support to restore
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 -- 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
Josef Bacik
2013-Mar-12 17:38 UTC
[PATCH 08/10] btrfs-progs: fix regexec to only work if we actually have a regexec
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 -- 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
Josef Bacik
2013-Mar-12 17:38 UTC
[PATCH 09/10] Btrfs-progs: give restore a list roots option
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 -- 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
Josef Bacik
2013-Mar-12 17:38 UTC
[PATCH 10/10] Btrfs-progs: make restore deal with really broken file systems
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 -- 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
Chris Mason
2013-Mar-12 19:26 UTC
Re: Btrfs-progs: Update btrfs restore so it matches all current development
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-kdaveThanks Josef, this seems like a great time for me to pull integration again. -chris -- 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
Zach Brown
2013-Mar-13 21:31 UTC
Re: [PATCH 06/10] restore: Add regex matching of paths and files to be restored
> 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 /outputBut 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 -- 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
Peter Stuge
2013-Mar-13 23:34 UTC
Re: [PATCH 06/10] restore: Add regex matching of paths and files to be restored
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 bonkersI 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 -- 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
Zach Brown
2013-Mar-14 00:04 UTC
Re: [PATCH 06/10] restore: Add regex matching of paths and files to be restored
> > > 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 -- 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
David Sterba
2013-Mar-14 23:10 UTC
Re: [PATCH 04/10] btrfs-progs: Fix error handling for failed reads in restore tool when mirrors exist
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) > ---- 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
David Sterba
2013-Mar-14 23:18 UTC
Re: [PATCH 05/10] restore: Split output directory and btrfs-local path search_dir() parameters
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 -- 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
David Marcin
2013-Mar-14 23:36 UTC
Re: [PATCH 04/10] btrfs-progs: Fix error handling for failed reads in restore tool when mirrors exist
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 -- 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
David Sterba
2013-Mar-14 23:37 UTC
Re: [PATCH 10/10] Btrfs-progs: make restore deal with really broken file systems
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; > }-- 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
David Sterba
2013-Mar-18 17:27 UTC
Re: Btrfs-progs: Update btrfs restore so it matches all current development
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 doI''ve pulled 1-5,7,9 (ie. left out 6,8 -- regex, and 10 which I need to look at again). david -- 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
David Sterba
2013-Mar-21 17:45 UTC
Re: [PATCH 10/10] Btrfs-progs: make restore deal with really broken file systems
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;-- 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