[PATCH 1/3] btrfs-progs: fix len of read_extent_buffer for inline extent in restore

[PATCH 1/3] btrfs-progs: fix len of read_extent_buffer for inline extent in restore

[Gui Hecheng]

Steps to reproduce:
	# mkfs.btrfs -f <dev>
	# mount -o compress-force=lzo <dev> <mnt>
	# for ((i=0;i<4000;i++)); do
	  	echo -n 'A' >> <mnt>/inline_data
	  done
	# umount <mnt>
	# valgrind --tool=memcheck --leak-check=full	\
	  btrfs restore <dev> <dest_dir>
output:
	==32118== Invalid read of size 1
	==32118==    at 0x4A0A4E4: memcpy@@GLIBC_2.14
	==32118==    by 0x43DC91: read_extent_buffer
	==32118==    by 0x421401: search_dir (cmds-restore.c:240)
	==32118==    by 0x422CBB: cmd_restore (cmds-restore.c:1317)
	==32118==    by 0x404709: main (btrfs.c:248)
	==32118==  Address 0x4c4f4ac is not stack'd, malloc'd or...

It is because when deal with inline extent, the read_extent_buffer
is now reading a len of @ram_bytes which is the len of the uncompressed
data. But actually here we want the len of the inline item.
So in the compressed situation, use the len of the inline item.

Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
---
 cmds-restore.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/cmds-restore.c b/cmds-restore.c
index bb72311..e94592c 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -231,13 +231,15 @@ static int copy_one_inline(int fd, struct btrfs_path *path, u64 pos)
 	unsigned long ptr;
 	int ret;
 	int len;
+	int inline_item_len;
 	int compress;
 
 	fi = btrfs_item_ptr(leaf, path->slots[0],
 			    struct btrfs_file_extent_item);
 	ptr = btrfs_file_extent_inline_start(fi);
 	len = btrfs_file_extent_inline_len(leaf, path->slots[0], fi);
-	read_extent_buffer(leaf, buf, ptr, len);
+	inline_item_len = btrfs_file_extent_inline_item_len(leaf, btrfs_item_nr(path->slots[0]));
+	read_extent_buffer(leaf, buf, ptr, inline_item_len);
 
 	compress = btrfs_file_extent_compression(leaf, fi);
 	if (compress == BTRFS_COMPRESS_NONE) {
-- 
1.8.1.4

[PATCH 2/3] btrfs-progs: fix next_leaf in restore as it improperly skips some slots

[Gui Hecheng]

When entering the next level node, the @next_leaf in restore forgets to
start at the first slot. Just reset it to the first one.

Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
---
 cmds-restore.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/cmds-restore.c b/cmds-restore.c
index e94592c..918f1fb 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -191,6 +191,7 @@ again:
 			level++;
 			if (level == BTRFS_MAX_LEVEL)
 				return 1;
+			offset = 1;
 			continue;
 		}
 
-- 
1.8.1.4

[PATCH 3/3] btrfs-progs: remove meaningless debug info for restore

[Gui Hecheng]

The printf of @offset enlightens the user little.
And the restore cmd is not a debugging tool, so just
remove the debug-info-like printf.

Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
---
 cmds-restore.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/cmds-restore.c b/cmds-restore.c
index 918f1fb..648fbc9 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -310,8 +310,6 @@ static int copy_one_extent(struct btrfs_root *root, int fd,
 	if (compress == BTRFS_COMPRESS_NONE)
 		bytenr += offset;
 
-	if (offset)
-		printf("offset is %Lu\n", offset);
 	/* we found a hole */
 	if (disk_size == 0)
 		return 0;
-- 
1.8.1.4

Re: [PATCH 3/3] btrfs-progs: remove meaningless debug info for restore

[David Sterba]

On Thu, Aug 28, 2014 at 10:25:55AM +0800, Gui Hecheng wrote:
> The printf of @offset enlightens the user little.
> And the restore cmd is not a debugging tool, so just
> remove the debug-info-like printf.

I'd like to let the restore command be more verbose as it's potentially
working with a broken filesystem and getting details can help. There's
-v option for increasing verbosity, please move the printf there.

Thaks.

Re: [PATCH 3/3] btrfs-progs: remove meaningless debug info for restore

[Gui Hecheng]

On Fri, 2014-08-29 at 16:49 +0200, David Sterba wrote:
> On Thu, Aug 28, 2014 at 10:25:55AM +0800, Gui Hecheng wrote:
> > The printf of @offset enlightens the user little.
> > And the restore cmd is not a debugging tool, so just
> > remove the debug-info-like printf.
> 
> I'd like to let the restore command be more verbose as it's potentially
> working with a broken filesystem and getting details can help. There's
> -v option for increasing verbosity, please move the printf there.

OK，since the -v option seems to serve as a debug helper.

> Thaks.

[PATCH] btrfs-progs: move debug info to verbose mode for restore

[Gui Hecheng]

The restore tool should only print info of the restoring process
in verbose mode with -v option specified.

Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
---
 cmds-restore.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmds-restore.c b/cmds-restore.c
index e9ec472..f909429 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -310,7 +310,7 @@ static int copy_one_extent(struct btrfs_root *root, int fd,
 	if (compress == BTRFS_COMPRESS_NONE)
 		bytenr += offset;
 
-	if (offset)
+	if (verbose && offset)
 		printf("offset is %Lu\n", offset);
 	/* we found a hole */
 	if (disk_size == 0)
-- 
1.8.1.4