linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] btrfs-progs: restore: Do proper mirror iteration in copy_one_extent()
@ 2019-12-04  2:15 Qu Wenruo
  0 siblings, 0 replies; only message in thread
From: Qu Wenruo @ 2019-12-04  2:15 UTC (permalink / raw)
  To: linux-btrfs

The old code of copy_one_extent() is a mess:
- The main loop is implemented using goto
- @mirror_num is reset to 1 for each loop
- @mirror num check against @num_copies is wrong for decompression error

This patch will fix this mess by:
- Use read_extent_data()
  read_extent_data() has all the good wrapping of btrfs_map_block()
  and length check.
  This removes a lot of complexity.

- Add extra file extent offset check
  To prevent underflow for memory allocation

- Do proper mirror_num check for decompression error

Issue: #221
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 cmds/restore.c | 87 ++++++++++++++++++++++----------------------------
 1 file changed, 38 insertions(+), 49 deletions(-)

diff --git a/cmds/restore.c b/cmds/restore.c
index c104b01aef69..ee5c42f1a542 100644
--- a/cmds/restore.c
+++ b/cmds/restore.c
@@ -346,8 +346,6 @@ static int copy_one_extent(struct btrfs_root *root, int fd,
 			   struct extent_buffer *leaf,
 			   struct btrfs_file_extent_item *fi, u64 pos)
 {
-	struct btrfs_multi_bio *multi = NULL;
-	struct btrfs_device *device;
 	char *inbuf, *outbuf = NULL;
 	ssize_t done, total = 0;
 	u64 bytenr;
@@ -356,12 +354,10 @@ static int copy_one_extent(struct btrfs_root *root, int fd,
 	u64 num_bytes;
 	u64 length;
 	u64 size_left;
-	u64 dev_bytenr;
 	u64 offset;
-	u64 count = 0;
+	u64 cur;
 	int compress;
 	int ret;
-	int dev_fd;
 	int mirror_num = 1;
 	int num_copies;
 
@@ -372,14 +368,26 @@ static int copy_one_extent(struct btrfs_root *root, int fd,
 	offset = btrfs_file_extent_offset(leaf, fi);
 	num_bytes = btrfs_file_extent_num_bytes(leaf, fi);
 	size_left = disk_size;
-	if (compress == BTRFS_COMPRESS_NONE)
+	/* Hole, early exit */
+	if (disk_size == 0)
+		return 0;
+
+	/* Invalid file extent */
+	if ((compress == BTRFS_COMPRESS_NONE && offset >= disk_size) ||
+	    offset > ram_size) {
+		error(
+	"invalid data extent offset, offset=%llu disk_size=%llu ram_size=%llu",
+		      offset, disk_size, ram_size);
+		return -EUCLEAN;
+	}
+
+	if (compress == BTRFS_COMPRESS_NONE && offset < disk_size) {
 		bytenr += offset;
+		size_left -= offset;
+	}
 
 	if (verbose && offset)
 		printf("offset is %Lu\n", offset);
-	/* we found a hole */
-	if (disk_size == 0)
-		return 0;
 
 	inbuf = malloc(size_left);
 	if (!inbuf) {
@@ -395,48 +403,29 @@ static int copy_one_extent(struct btrfs_root *root, int fd,
 			return -ENOMEM;
 		}
 	}
+
+	num_copies = btrfs_num_copies(root->fs_info, bytenr,
+				      disk_size - offset);
 again:
-	length = size_left;
-	ret = btrfs_map_block(root->fs_info, READ, bytenr, &length, &multi,
-			      mirror_num, NULL);
-	if (ret) {
-		error("cannot map block logical %llu length %llu: %d",
-				(unsigned long long)bytenr,
-				(unsigned long long)length, ret);
-		goto out;
-	}
-	device = multi->stripes[0].dev;
-	dev_fd = device->fd;
-	device->total_ios++;
-	dev_bytenr = multi->stripes[0].physical;
-	free(multi);
-
-	if (size_left < length)
-		length = size_left;
-
-	done = pread(dev_fd, inbuf+count, length, dev_bytenr);
-	/* 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, bytenr, length);
-		mirror_num++;
-		/* mirror_num is 1-indexed, so num_copies is a valid mirror. */
-		if (mirror_num > num_copies) {
-			ret = -1;
-			error("exhausted mirrors trying to read (%d > %d)",
+	cur = bytenr;
+	while (cur < bytenr + size_left) {
+		length = bytenr + size_left - cur;
+		ret = read_extent_data(root->fs_info, inbuf + cur - bytenr, cur,
+				       &length, mirror_num);
+		if (ret < 0) {
+			mirror_num++;
+			if (mirror_num > num_copies) {
+				ret = -1;
+				error("exhausted mirros trying to read (%d > %d)",
 					mirror_num, num_copies);
-			goto out;
+				goto out;
+			}
+			fprintf(stderr, "Trying another mirror\n");
+			continue;
 		}
-		fprintf(stderr, "Trying another mirror\n");
-		goto again;
+		cur += length;
 	}
 
-	mirror_num = 1;
-	size_left -= length;
-	count += length;
-	bytenr += length;
-	if (size_left)
-		goto again;
-
 	if (compress == BTRFS_COMPRESS_NONE) {
 		while (total < num_bytes) {
 			done = pwrite(fd, inbuf+total, num_bytes-total,
@@ -454,13 +443,13 @@ again:
 
 	ret = decompress(root, inbuf, outbuf, disk_size, &ram_size, compress);
 	if (ret) {
-		num_copies = btrfs_num_copies(root->fs_info, bytenr, length);
 		mirror_num++;
-		if (mirror_num >= num_copies) {
+		if (mirror_num > num_copies) {
 			ret = -1;
 			goto out;
 		}
-		fprintf(stderr, "Trying another mirror\n");
+		fprintf(stderr,
+			"Trying another mirror due to decompression error\n");
 		goto again;
 	}
 
-- 
2.24.0


^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2019-12-04  2:15 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-04  2:15 [PATCH] btrfs-progs: restore: Do proper mirror iteration in copy_one_extent() Qu Wenruo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).