Skip to contentSkip to conversation

Enhanced error messages for btrfs-convert

PATCH In review Qu Wenruo 3 participants 6 messages Sep 14, 2018 – Oct 24, 2018 Btrfs file system

lore
Share & tools
lore

Qu Wenruo

author
#0 lore raw

cover Enhanced error messages for btrfs-convert

This patchset can be fetched from github:
https://github.com/adam900710/btrfs-progs/tree/convert_error_messages
As usual, it's based on latest stable tag (v4.17.1).

There is one error report of btrfs-convert, the error message looks pretty meaningless:

create btrfs filesystem:

      blocksize: 4096
      nodesize:  16384
      features:  extref, skinny-metadata (default)

creating ext2 image file
ERROR: failed to create ext2_saved/image: -1
WARNING: an error occurred during conversion, filesystem is partially created but not finalized and not mountable

After some investigation, the problem turns out to be read failure. But the error number is intermediate number (-1) returned from read_disk_extent().

This patchset will first fix the intermediate return number of read_disk_extent(), then add more error messages for btrfs-convert (at least convert part) to makes it easier to identify the problem.

In this particular case, it should output things like:

create btrfs filesystem:

      blocksize: 4096
      nodesize:  16384
      features:  extref, skinny-metadata (default)

creating ext2 image file
ERROR: failed to calculate csum for bytenr 2732765184 len 4096, Input/output error
ERROR: failed to create ext2_saved/image: -5
WARNING: an error occurred during conversion, filesystem is partially created but not finalized and not mountable

Qu Wenruo (2):
btrfs: convert: Make read_disk_extent() return more meaningful -EIO
other -1
btrfs-progs: convert: Output meaningful error messages for
create_image()

 convert/main.c      | 37 ++++++++++++++++++++++++++++++-------
 convert/source-fs.c |  2 +-
 2 files changed, 31 insertions(+), 8 deletions(-)
Signature · 1 line
2.19.0
author
#1 lore raw

PATCH 1/2 btrfs: convert: Make read_disk_extent() return more meaningful -EIO other -1

When pread64() returns value smaller than expected, it normally means EIO, so just return -EIO to replace the intermediate number. So when IO fails, we should be able to get more meaningful error number of than EPERM.

---
 convert/source-fs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
convert/source-fs.c +1 -1
index b6d08370428a..5660a22cc652 100644
@@ -201,7 +201,7 @@ int read_disk_extent(struct btrfs_root *root, u64 bytenr,
 	ret = 0;
 fail:
 	if (ret > 0)
-		ret = -1;
+		ret = -EIO;
 	return ret;
 }
 
Signature · 1 line
2.19.0
Reviewed-by on v1 1/2
#2 lore raw

Re: [PATCH 1/2] btrfs: convert: Make read_disk_extent() return more meaningful -EIO other -1

Quoted 6 lines · Qu Wenruo

When pread64() returns value smaller than expected, it normally means EIO, so just return -EIO to replace the intermediate number. So when IO fails, we should be able to get more meaningful error number of than EPERM.

Quoted 18 lines
---
 convert/source-fs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
convert/source-fs.c +1 -1
index b6d08370428a..5660a22cc652 100644
@@ -201,7 +201,7 @@ int read_disk_extent(struct btrfs_root *root, u64 bytenr,
 	ret = 0;
 fail:
 	if (ret > 0)
-		ret = -1;
+		ret = -EIO;
 	return ret;
 }
 
author
#3 lore raw

PATCH 2/2 btrfs-progs: convert: Output meaningful error messages for create_image()

In reply to Qu Wenruo

When convert failed, the error messsage would look like:

create btrfs filesystem:

      blocksize: 4096
      nodesize:  16384
      features:  extref, skinny-metadata (default)

creating ext2 image file
ERROR: failed to create ext2_saved/image: -1
WARNING: an error occurred during conversion, filesystem is partially created but not finalized and not mountable

We can only know something wrong happened during "ext2_saved/image" file creation, but unable to know what exactly went wrong.

This patch will add the following error messages for create_image() and its callee:

1) Sanity test error
2) Csum calculation error
3) Free ino number allocation error
4) Inode creation error
5) Inode mode change error
6) Inode link error

With all these error messages, we should be pretty easy to locate the error without extra debugging.

---
 convert/main.c | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)
convert/main.c +30 -7
index 3736a14955d1..f100699b652c 100644
@@ -290,10 +290,16 @@ static int create_image_file_range(struct btrfs_trans_handle *trans,
 	if (disk_bytenr) {
 		/* Check if the range is in a data block group */
 		bg_cache = btrfs_lookup_block_group(root->fs_info, bytenr);
-		if (!bg_cache)
+		if (!bg_cache) {
+			error("missing data block for bytenr %llu", bytenr);
 			return -ENOENT;
-		if (!(bg_cache->flags & BTRFS_BLOCK_GROUP_DATA))
+		}
+		if (!(bg_cache->flags & BTRFS_BLOCK_GROUP_DATA)) {
+			error(
+	"data bytenr %llu is covered by non-data block group %llu flags 0x%llu",
+			      bytenr, bg_cache->key.objectid, bg_cache->flags);
 			return -EINVAL;
+		}
 
 		/* The extent should never cross block group boundary */
 		len = min_t(u64, len, bg_cache->key.objectid +
@@ -310,8 +316,13 @@ static int create_image_file_range(struct btrfs_trans_handle *trans,
 	if (ret < 0)
 		return ret;
 
-	if (datacsum)
+	if (datacsum) {
 		ret = csum_disk_extent(trans, root, bytenr, len);
+		if (ret < 0)
+			error(
+		"failed to calculate csum for bytenr %llu len %llu, %s\n",
+			      bytenr, len, strerror(-ret));
+	}
 	*ret_len = len;
 	return ret;
 }
@@ -759,18 +770,30 @@ static int create_image(struct btrfs_root *root,
 
 	ret = btrfs_find_free_objectid(trans, root, BTRFS_FIRST_FREE_OBJECTID,
 				       &ino);
-	if (ret < 0)
+	if (ret < 0) {
+		error("failed to find free objectid for root %llu, %s",
+			root->root_key.objectid, strerror(-ret));
 		goto out;
+	}
 	ret = btrfs_new_inode(trans, root, ino, 0400 | S_IFREG);
-	if (ret < 0)
+	if (ret < 0) {
+		error("failed to create new inode for root %llu, %s",
+			root->root_key.objectid, strerror(-ret));
 		goto out;
+	}
 	ret = btrfs_change_inode_flags(trans, root, ino, flags);
-	if (ret < 0)
+	if (ret < 0) {
+		error("failed to change inode flag for ino %llu root %llu, %s",
+			ino, root->root_key.objectid, strerror(-ret));
 		goto out;
+	}
 	ret = btrfs_add_link(trans, root, ino, BTRFS_FIRST_FREE_OBJECTID, name,
 			     strlen(name), BTRFS_FT_REG_FILE, NULL, 1, 0);
-	if (ret < 0)
+	if (ret < 0) {
+		error("failed to link ino %llu to '/%s' in root %llu, %s",
+			ino, name, root->root_key.objectid, strerror(-ret));
 		goto out;
+	}
 
 	key.objectid = ino;
 	key.type = BTRFS_INODE_ITEM_KEY;
Signature · 1 line
2.19.0
Reviewed-by on v1 2/2
#4 lore raw

Re: [PATCH 2/2] btrfs-progs: convert: Output meaningful error messages for create_image()

Quoted 24 lines · Qu Wenruo

When convert failed, the error messsage would look like:

create btrfs filesystem:

      blocksize: 4096
      nodesize:  16384
      features:  extref, skinny-metadata (default)

creating ext2 image file
ERROR: failed to create ext2_saved/image: -1
WARNING: an error occurred during conversion, filesystem is partially created but not finalized and not mountable

We can only know something wrong happened during "ext2_saved/image" file creation, but unable to know what exactly went wrong.

This patch will add the following error messages for create_image() and its callee:

1) Sanity test error
2) Csum calculation error
3) Free ino number allocation error
4) Inode creation error
5) Inode mode change error
6) Inode link error

Quoted 5 lines

With all these error messages, we should be pretty easy to locate the error without extra debugging.

Quoted 78 lines
---
 convert/main.c | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)
convert/main.c +30 -7
index 3736a14955d1..f100699b652c 100644
@@ -290,10 +290,16 @@ static int create_image_file_range(struct btrfs_trans_handle *trans,
 	if (disk_bytenr) {
 		/* Check if the range is in a data block group */
 		bg_cache = btrfs_lookup_block_group(root->fs_info, bytenr);
-		if (!bg_cache)
+		if (!bg_cache) {
+			error("missing data block for bytenr %llu", bytenr);
 			return -ENOENT;
-		if (!(bg_cache->flags & BTRFS_BLOCK_GROUP_DATA))
+		}
+		if (!(bg_cache->flags & BTRFS_BLOCK_GROUP_DATA)) {
+			error(
+	"data bytenr %llu is covered by non-data block group %llu flags 0x%llu",
+			      bytenr, bg_cache->key.objectid, bg_cache->flags);
 			return -EINVAL;
+		}
 
 		/* The extent should never cross block group boundary */
 		len = min_t(u64, len, bg_cache->key.objectid +
@@ -310,8 +316,13 @@ static int create_image_file_range(struct btrfs_trans_handle *trans,
 	if (ret < 0)
 		return ret;
 
-	if (datacsum)
+	if (datacsum) {
 		ret = csum_disk_extent(trans, root, bytenr, len);
+		if (ret < 0)
+			error(
+		"failed to calculate csum for bytenr %llu len %llu, %s\n",
+			      bytenr, len, strerror(-ret));
+	}
 	*ret_len = len;
 	return ret;
 }
@@ -759,18 +770,30 @@ static int create_image(struct btrfs_root *root,
 
 	ret = btrfs_find_free_objectid(trans, root, BTRFS_FIRST_FREE_OBJECTID,
 				       &ino);
-	if (ret < 0)
+	if (ret < 0) {
+		error("failed to find free objectid for root %llu, %s",
+			root->root_key.objectid, strerror(-ret));
 		goto out;
+	}
 	ret = btrfs_new_inode(trans, root, ino, 0400 | S_IFREG);
-	if (ret < 0)
+	if (ret < 0) {
+		error("failed to create new inode for root %llu, %s",
+			root->root_key.objectid, strerror(-ret));
 		goto out;
+	}
 	ret = btrfs_change_inode_flags(trans, root, ino, flags);
-	if (ret < 0)
+	if (ret < 0) {
+		error("failed to change inode flag for ino %llu root %llu, %s",
+			ino, root->root_key.objectid, strerror(-ret));
 		goto out;
+	}
 	ret = btrfs_add_link(trans, root, ino, BTRFS_FIRST_FREE_OBJECTID, name,
 			     strlen(name), BTRFS_FT_REG_FILE, NULL, 1, 0);
-	if (ret < 0)
+	if (ret < 0) {
+		error("failed to link ino %llu to '/%s' in root %llu, %s",
+			ino, name, root->root_key.objectid, strerror(-ret));
 		goto out;
+	}
 
 	key.objectid = ino;
 	key.type = BTRFS_INODE_ITEM_KEY;
#5 lore raw
In reply to Qu Wenruo
Quoted 37 lines · Qu Wenruo

This patchset can be fetched from github:
https://github.com/adam900710/btrfs-progs/tree/convert_error_messages
As usual, it's based on latest stable tag (v4.17.1).

There is one error report of btrfs-convert, the error message looks pretty meaningless:

create btrfs filesystem:

      blocksize: 4096
      nodesize:  16384
      features:  extref, skinny-metadata (default)

creating ext2 image file
ERROR: failed to create ext2_saved/image: -1
WARNING: an error occurred during conversion, filesystem is partially created but not finalized and not mountable

After some investigation, the problem turns out to be read failure. But the error number is intermediate number (-1) returned from read_disk_extent().

This patchset will first fix the intermediate return number of read_disk_extent(), then add more error messages for btrfs-convert (at least convert part) to makes it easier to identify the problem.

In this particular case, it should output things like:

create btrfs filesystem:

      blocksize: 4096
      nodesize:  16384
      features:  extref, skinny-metadata (default)

creating ext2 image file
ERROR: failed to calculate csum for bytenr 2732765184 len 4096, Input/output error
ERROR: failed to create ext2_saved/image: -5
WARNING: an error occurred during conversion, filesystem is partially created but not finalized and not mountable

Quoted 5 lines

Qu Wenruo (2):
btrfs: convert: Make read_disk_extent() return more meaningful -EIO
other -1
btrfs-progs: convert: Output meaningful error messages for
create_image()

Added to devel, thanks.