[PATCH v2 0/6] Lowmem mode check check fix for mulit-device

[PATCH v2 0/6] Lowmem mode check check fix for mulit-device

[Qu Wenruo]

This patchset will fix a false alert in lowmem mode, which doesn't handle
RAID0/5/6/10 chunk well. (the 5th patch)

Along the lowmem fix, also enhance and cleanup some chunk verification code,
as lowmem mode and original mode are using different chunk verification.
(the 1st~4th patch)

Finally, cleanup the loop device setup and move it to test/common other
than handling in each test case. (the 6th patch)

changelog:
v2:
  Fix a bug that single chunk is always treated as invalid, since its
  profile is not power of 2.
  Add the 6th patch to move the loop device setup to test/common.

Qu Wenruo (6):
  btrfs-progs: Cleanup open-coded btrfs_chunk_item_size
  btrfs-progs: Enhance chunk item validation check
  btrfs-progs: check: Reuse btrfs_check_chunk_valid in lowmem mode
  btrfs-progs: Introduce function to get correct stripe length
  btrfs-progs: lowmem check: Fix false alert on missing chunk or dev
    extent
  btrfs-progs: test: Introduce functions to prepare and cleanup loop
    device

 cmds-check.c                                       | 35 +++++------
 tests/common                                       | 38 ++++++++++++
 .../misc-tests/006-image-on-missing-device/test.sh | 12 +---
 tests/misc-tests/011-delete-missing-device/test.sh | 12 +---
 tests/mkfs-tests/001-basic-profiles/test.sh        | 12 +---
 .../005-long-device-name-for-ssd/test.sh           | 10 +---
 tests/mkfs-tests/006-partitioned-loopdev/test.sh   | 10 +---
 volumes.c                                          | 70 ++++++++++++++++++++--
 volumes.h                                          |  3 +
 9 files changed, 136 insertions(+), 66 deletions(-)

-- 
2.13.0

[PATCH v2 1/6] btrfs-progs: Cleanup open-coded btrfs_chunk_item_size

[Qu Wenruo]

In btrfs_check_chunk_valid() we calculates chunk item using open code.

use btrfs_chunk_item_size() to replace them.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 volumes.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/volumes.c b/volumes.c
index b350e259..62e23aee 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1685,6 +1685,7 @@ int btrfs_check_chunk_valid(struct btrfs_root *root,
 	u16 num_stripes;
 	u16 sub_stripes;
 	u64 type;
+	u32 chunk_ondisk_size;
 
 	length = btrfs_chunk_length(leaf, chunk);
 	stripe_len = btrfs_chunk_stripe_len(leaf, chunk);
@@ -1724,16 +1725,16 @@ int btrfs_check_chunk_valid(struct btrfs_root *root,
 			BTRFS_BLOCK_GROUP_PROFILE_MASK) & type);
 		return -EIO;
 	}
+
+	chunk_ondisk_size = btrfs_chunk_item_size(num_stripes);
 	/*
 	 * Btrfs_chunk contains at least one stripe, and for sys_chunk
 	 * it can't exceed the system chunk array size
 	 * For normal chunk, it should match its chunk item size.
 	 */
 	if (num_stripes < 1 ||
-	    (slot == -1 && sizeof(struct btrfs_stripe) * num_stripes >
-	     BTRFS_SYSTEM_CHUNK_ARRAY_SIZE) ||
-	    (slot >= 0 && sizeof(struct btrfs_stripe) * (num_stripes - 1) >
-	     btrfs_item_size_nr(leaf, slot))) {
+	    (slot == -1 && chunk_ondisk_size > BTRFS_SYSTEM_CHUNK_ARRAY_SIZE) ||
+	    (slot >= 0 && chunk_ondisk_size > btrfs_item_size_nr(leaf, slot))) {
 		error("invalid num_stripes: %u", num_stripes);
 		return -EIO;
 	}
-- 
2.13.0

[PATCH v2 2/6] btrfs-progs: Enhance chunk item validation check

[Qu Wenruo]

btrfs_check_chunk_valid() doesn't check if
1) chunk flag has conflicting flags
   For example chunk type DATA|METADATA|RAID1|RAID10 is completely
   invalid, while current check_chunk_valid() can't detect it.
2) num_stripes is invalid for RAID10
   Num_stripes 5 is not valid for RAID10.

This patch will enhance btrfs_check_chunk_valid() to handle above cases.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 volumes.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/volumes.c b/volumes.c
index 62e23aee..57534314 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1725,6 +1725,20 @@ int btrfs_check_chunk_valid(struct btrfs_root *root,
 			BTRFS_BLOCK_GROUP_PROFILE_MASK) & type);
 		return -EIO;
 	}
+	if (!(type & BTRFS_BLOCK_GROUP_TYPE_MASK)) {
+		error("missing chunk type flag: %llu", type);
+		return -EIO;
+	}
+	if (!(is_power_of_2(type & BTRFS_BLOCK_GROUP_PROFILE_MASK) ||
+	      (type & BTRFS_BLOCK_GROUP_PROFILE_MASK) == 0)) {
+		error("conflicting chunk type detected: %llu", type);
+		return -EIO;
+	}
+	if ((type & BTRFS_BLOCK_GROUP_PROFILE_MASK) &&
+	    !is_power_of_2(type & BTRFS_BLOCK_GROUP_PROFILE_MASK)) {
+		error("conflicting chunk profile detected: %llu", type);
+		return -EIO;
+	}
 
 	chunk_ondisk_size = btrfs_chunk_item_size(num_stripes);
 	/*
@@ -1741,7 +1755,8 @@ int btrfs_check_chunk_valid(struct btrfs_root *root,
 	/*
 	 * Device number check against profile
 	 */
-	if ((type & BTRFS_BLOCK_GROUP_RAID10 && sub_stripes == 0) ||
+	if ((type & BTRFS_BLOCK_GROUP_RAID10 && (sub_stripes != 2 ||
+		  !IS_ALIGNED(num_stripes, sub_stripes))) ||
 	    (type & BTRFS_BLOCK_GROUP_RAID1 && num_stripes < 1) ||
 	    (type & BTRFS_BLOCK_GROUP_RAID5 && num_stripes < 2) ||
 	    (type & BTRFS_BLOCK_GROUP_RAID6 && num_stripes < 3) ||
-- 
2.13.0

[PATCH v2 3/6] btrfs-progs: check: Reuse btrfs_check_chunk_valid in lowmem mode

[Qu Wenruo]

Before this patch, btrfs check lowmem mode manually checks found chunk
item, even we already have the generic chunk validation checker,
btrfs_check_chunk_valid().

This patch will use btrfs_check_chunk_valid() to replace open-coded
chunk validation checker in check_chunk_item().

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 cmds-check.c | 24 ++++++------------------
 1 file changed, 6 insertions(+), 18 deletions(-)

diff --git a/cmds-check.c b/cmds-check.c
index ad7c81b2..b19d5a28 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -11158,11 +11158,9 @@ static int check_chunk_item(struct btrfs_fs_info *fs_info,
 	struct btrfs_block_group_item *bi;
 	struct btrfs_block_group_item bg_item;
 	struct btrfs_dev_extent *ptr;
-	u32 sectorsize = btrfs_super_sectorsize(fs_info->super_copy);
 	u64 length;
 	u64 chunk_end;
 	u64 type;
-	u64 profile;
 	int num_stripes;
 	u64 offset;
 	u64 objectid;
@@ -11174,25 +11172,15 @@ static int check_chunk_item(struct btrfs_fs_info *fs_info,
 	chunk = btrfs_item_ptr(eb, slot, struct btrfs_chunk);
 	length = btrfs_chunk_length(eb, chunk);
 	chunk_end = chunk_key.offset + length;
-	if (!IS_ALIGNED(length, sectorsize)) {
-		error("chunk[%llu %llu) not aligned to %u",
-			chunk_key.offset, chunk_end, sectorsize);
-		err |= BYTES_UNALIGNED;
+	ret = btrfs_check_chunk_valid(extent_root, eb, chunk, slot,
+				      chunk_key.offset);
+	if (ret < 0) {
+		error("chunk[%llu %llu) is invalid", chunk_key.offset,
+			chunk_end);
+		err |= BYTES_UNALIGNED | UNKNOWN_TYPE;
 		goto out;
 	}
-
 	type = btrfs_chunk_type(eb, chunk);
-	profile = type & BTRFS_BLOCK_GROUP_PROFILE_MASK;
-	if (!(type & BTRFS_BLOCK_GROUP_TYPE_MASK)) {
-		error("chunk[%llu %llu) has no chunk type",
-			chunk_key.offset, chunk_end);
-		err |= UNKNOWN_TYPE;
-	}
-	if (profile && (profile & (profile - 1))) {
-		error("chunk[%llu %llu) multiple profiles detected: %llx",
-			chunk_key.offset, chunk_end, profile);
-		err |= UNKNOWN_TYPE;
-	}
 
 	bg_key.objectid = chunk_key.offset;
 	bg_key.type = BTRFS_BLOCK_GROUP_ITEM_KEY;
-- 
2.13.0

[PATCH v2 4/6] btrfs-progs: Introduce function to get correct stripe length

[Qu Wenruo]

Introduce a new function, btrfs_get_chunk_stripe_len() to get correct
stripe length.
This is very handy for lowmem mode, which checks the mapping between
device extent and chunk item.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 volumes.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 volumes.h |  3 +++
 2 files changed, 47 insertions(+)

diff --git a/volumes.c b/volumes.c
index 57534314..1c291edb 100644
--- a/volumes.c
+++ b/volumes.c
@@ -2285,3 +2285,47 @@ out:
 
 	return ret;
 }
+
+/*
+ * Get stripe length from chunk item and its stripe items
+ *
+ * Caller should only call this function after validating the chunk item
+ * by using btrfs_check_chunk_valid().
+ */
+u64 btrfs_stripe_length(struct btrfs_fs_info *fs_info,
+			struct extent_buffer *leaf,
+			struct btrfs_chunk *chunk)
+{
+	u64 stripe_len;
+	u64 chunk_len;
+	u32 num_stripes = btrfs_chunk_num_stripes(leaf, chunk);
+	u64 profile = btrfs_chunk_type(leaf, chunk) &
+		      BTRFS_BLOCK_GROUP_PROFILE_MASK;
+
+	chunk_len = btrfs_chunk_length(leaf, chunk);
+
+	switch (profile) {
+	case 0: /* Single profile */
+	case BTRFS_BLOCK_GROUP_RAID1:
+	case BTRFS_BLOCK_GROUP_DUP:
+		stripe_len = chunk_len;
+		break;
+	case BTRFS_BLOCK_GROUP_RAID0:
+		stripe_len = chunk_len / num_stripes;
+		break;
+	case BTRFS_BLOCK_GROUP_RAID5:
+		stripe_len = chunk_len / (num_stripes - 1);
+		break;
+	case BTRFS_BLOCK_GROUP_RAID6:
+		stripe_len = chunk_len / (num_stripes - 2);
+		break;
+	case BTRFS_BLOCK_GROUP_RAID10:
+		stripe_len = chunk_len / (num_stripes /
+				btrfs_chunk_sub_stripes(leaf, chunk));
+		break;
+	default:
+		/* Invalid chunk profile found */
+		BUG_ON(1);
+	}
+	return stripe_len;
+}
diff --git a/volumes.h b/volumes.h
index 699b0bae..fc0a775b 100644
--- a/volumes.h
+++ b/volumes.h
@@ -246,4 +246,7 @@ int btrfs_check_chunk_valid(struct btrfs_root *root,
 			    struct extent_buffer *leaf,
 			    struct btrfs_chunk *chunk,
 			    int slot, u64 logical);
+u64 btrfs_stripe_length(struct btrfs_fs_info *fs_info,
+			struct extent_buffer *leaf,
+			struct btrfs_chunk *chunk);
 #endif
-- 
2.13.0

[PATCH v2 5/6] btrfs-progs: lowmem check: Fix false alert on missing chunk or dev extent

[Qu Wenruo]

When checking chunk or dev extent, lowmem mode uses chunk length as dev
extent length, and if they mismatch, report missing chunk or dev extent
like:
------
ERROR: chunk[256 4324327424) stripe 0 did not find the related dev extent
ERROR: chunk[256 4324327424) stripe 1 did not find the related dev extent
ERROR: chunk[256 4324327424) stripe 2 did not find the related dev extent
------

However, only for Single/DUP/RAID1 profiles chunk length is the same as
dev extent length.
For other profiles, this will cause tons of false alert.

Fix it by using correct stripe length when checking chunk and dev extent
items.

This fixes the mkfs test failure when using lowmem mode check.

Reported-by: Kai Krakow <hurikhan77@gmail.com>
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 cmds-check.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/cmds-check.c b/cmds-check.c
index b19d5a28..52091325 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -10920,7 +10920,12 @@ static int check_dev_extent_item(struct btrfs_fs_info *fs_info,
 
 	l = path.nodes[0];
 	chunk = btrfs_item_ptr(l, path.slots[0], struct btrfs_chunk);
-	if (btrfs_chunk_length(l, chunk) != length)
+	ret = btrfs_check_chunk_valid(chunk_root, l, chunk, path.slots[0],
+				      chunk_key.offset);
+	if (ret < 0)
+		goto out;
+
+	if (btrfs_stripe_length(fs_info, l, chunk) != length)
 		goto out;
 
 	num_stripes = btrfs_chunk_num_stripes(l, chunk);
@@ -11160,6 +11165,7 @@ static int check_chunk_item(struct btrfs_fs_info *fs_info,
 	struct btrfs_dev_extent *ptr;
 	u64 length;
 	u64 chunk_end;
+	u64 stripe_len;
 	u64 type;
 	int num_stripes;
 	u64 offset;
@@ -11209,6 +11215,7 @@ static int check_chunk_item(struct btrfs_fs_info *fs_info,
 	}
 
 	num_stripes = btrfs_chunk_num_stripes(eb, chunk);
+	stripe_len = btrfs_stripe_length(fs_info, eb, chunk);
 	for (i = 0; i < num_stripes; i++) {
 		btrfs_release_path(&path);
 		btrfs_init_path(&path);
@@ -11228,7 +11235,7 @@ static int check_chunk_item(struct btrfs_fs_info *fs_info,
 		offset = btrfs_dev_extent_chunk_offset(leaf, ptr);
 		if (objectid != chunk_key.objectid ||
 		    offset != chunk_key.offset ||
-		    btrfs_dev_extent_length(leaf, ptr) != length)
+		    btrfs_dev_extent_length(leaf, ptr) != stripe_len)
 			goto not_match_dev;
 		continue;
 not_match_dev:
-- 
2.13.0

[PATCH v2 6/6] btrfs-progs: test: Introduce functions to prepare and cleanup loop device

[Qu Wenruo]

Introduce new helpers, prepare_loop_dev() and cleanup_loop_dev() to
prepare and cleanup loop device.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 tests/common                                       | 38 ++++++++++++++++++++++
 .../misc-tests/006-image-on-missing-device/test.sh | 12 ++-----
 tests/misc-tests/011-delete-missing-device/test.sh | 12 ++-----
 tests/mkfs-tests/001-basic-profiles/test.sh        | 12 ++-----
 .../005-long-device-name-for-ssd/test.sh           | 10 ++----
 tests/mkfs-tests/006-partitioned-loopdev/test.sh   | 10 ++----
 6 files changed, 53 insertions(+), 41 deletions(-)

diff --git a/tests/common b/tests/common
index 7ad436e3..3808cfc5 100644
--- a/tests/common
+++ b/tests/common
@@ -355,6 +355,44 @@ setup_root_helper()
 	SUDO_HELPER=root_helper
 }
 
+# prepare loop device using specified size and path
+# $1: path of the file
+# $2: size of the device, optional, default value is '2G'
+prepare_loop_dev()
+{
+	local path="$1"
+	local size="$2"
+
+	[[ "$path" ]] || _fail "path must be specified for prepare_loop_dev()"
+	[[ "$size" ]] || size='2G'
+
+	# Cleanup if it's already mounted or set up as loop device
+	cleanup_loop_dev $path
+
+	run_check touch $path
+	chmod a+rw $path
+	run_check truncate -s $size $path
+
+	run_check_stdout $SUDO_HELPER losetup --find --show $path
+}
+
+# cleanup loop device based on its backend file
+# $1: the path of the backend file
+#
+# We don't want to populate result in cleanup, so any error will only be
+# caught manually, don't call run_check here.
+cleanup_loop_dev()
+{
+	local path="$1"
+
+	loop_dev=$(losetup -l | tail -n +2 | grep $path | cut -f1 -d\ )
+	if [ ! -z "$loop_dev" ]; then
+		umount $loop_dev &> /dev/null
+		$SUDO_HELPER losetup -d $loop_dev || \
+			_fail "failed to detach $path"
+	fi
+}
+
 prepare_test_dev()
 {
 	# num[K/M/G/T...]
diff --git a/tests/misc-tests/006-image-on-missing-device/test.sh b/tests/misc-tests/006-image-on-missing-device/test.sh
index 5b6fe065..8249c0e9 100755
--- a/tests/misc-tests/006-image-on-missing-device/test.sh
+++ b/tests/misc-tests/006-image-on-missing-device/test.sh
@@ -23,21 +23,15 @@ setup_root_helper
 prepare_devices()
 {
 	for i in `seq $ndevs`; do
-		touch img$i
-		chmod a+rw img$i
-		truncate -s0 img$i
-		truncate -s2g img$i
-		devs[$i]=`run_check_stdout $SUDO_HELPER losetup --find --show img$i`
+		devs[$i]=$(prepare_loop_dev img$i)
 	done
 }
 
 cleanup_devices()
 {
-	for dev in ${devs[@]}; do
-		run_mayfail $SUDO_HELPER losetup -d $dev
-	done
 	for i in `seq $ndevs`; do
-		truncate -s0 img$i
+		cleanup_loop_dev img$i
+		rm img$i
 	done
 	run_check $SUDO_HELPER losetup --all
 }
diff --git a/tests/misc-tests/011-delete-missing-device/test.sh b/tests/misc-tests/011-delete-missing-device/test.sh
index 5b5f9786..ad4b7d45 100755
--- a/tests/misc-tests/011-delete-missing-device/test.sh
+++ b/tests/misc-tests/011-delete-missing-device/test.sh
@@ -16,21 +16,15 @@ setup_root_helper
 prepare_devices()
 {
 	for i in `seq $ndevs`; do
-		touch img$i
-		chmod a+rw img$i
-		truncate -s0 img$i
-		truncate -s2g img$i
-		devs[$i]=`run_check_stdout $SUDO_HELPER losetup --find --show img$i`
+		devs[$i]=$(prepare_loop_dev img$i)
 	done
 }
 
 cleanup_devices()
 {
-	for dev in ${devs[@]}; do
-		run_mayfail $SUDO_HELPER losetup -d $dev
-	done
 	for i in `seq $ndevs`; do
-		truncate -s0 img$i
+		cleanup_loop_dev img$i
+		rm img$i
 	done
 	run_check $SUDO_HELPER losetup --all
 }
diff --git a/tests/mkfs-tests/001-basic-profiles/test.sh b/tests/mkfs-tests/001-basic-profiles/test.sh
index 0dc9a2bd..98bd9e6b 100755
--- a/tests/mkfs-tests/001-basic-profiles/test.sh
+++ b/tests/mkfs-tests/001-basic-profiles/test.sh
@@ -16,21 +16,15 @@ setup_root_helper
 prepare_devices()
 {
 	for i in `seq $ndevs`; do
-		touch img$i
-		chmod a+rw img$i
-		truncate -s0 img$i
-		truncate -s2g img$i
-		devs[$i]=`run_check_stdout $SUDO_HELPER losetup --find --show img$i`
+		devs[$i]=$(prepare_loop_dev img$i)
 	done
 }
 
 cleanup_devices()
 {
-	for dev in ${devs[@]}; do
-		run_check $SUDO_HELPER losetup -d $dev
-	done
 	for i in `seq $ndevs`; do
-		truncate -s0 img$i
+		cleanup_loop_dev img$i
+		rm img$i
 	done
 	run_check $SUDO_HELPER losetup --all
 }
diff --git a/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh b/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh
index 63fb1785..aacff6ee 100755
--- a/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh
+++ b/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh
@@ -13,11 +13,7 @@ dmname=\
 btrfs-test-with-very-long-name-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
 dmdev=/dev/mapper/$dmname
 
-run_check truncate -s0 img
-chmod a+w img
-run_check truncate -s2g img
-
-loopdev=`run_check_stdout $SUDO_HELPER losetup --find --show img`
+loopdev=$(prepare_loop_dev img)
 run_check $SUDO_HELPER dmsetup create $dmname --table "0 1048576 linear $loopdev 0"
 
 dmbase=`readlink -f $dmdev`
@@ -36,5 +32,5 @@ run_check $SUDO_HELPER $TOP/btrfs inspect-internal dump-super $dmdev
 
 # cleanup
 run_check $SUDO_HELPER dmsetup remove $dmname
-run_mayfail $SUDO_HELPER losetup -d $loopdev
-run_check truncate -s0 img
+cleanup_loop_dev img
+rm img
diff --git a/tests/mkfs-tests/006-partitioned-loopdev/test.sh b/tests/mkfs-tests/006-partitioned-loopdev/test.sh
index 12f37842..b005ef3d 100755
--- a/tests/mkfs-tests/006-partitioned-loopdev/test.sh
+++ b/tests/mkfs-tests/006-partitioned-loopdev/test.sh
@@ -12,12 +12,8 @@ check_prereq mkfs.btrfs
 
 setup_root_helper
 
-run_check truncate -s0 img
-chmod a+w img
 cp partition-1g-1g img
-run_check truncate -s2g img
-
-loopdev=$(run_check_stdout $SUDO_HELPER losetup --partscan --find --show img)
+loopdev=$(prepare_loop_dev img)
 base=$(basename $loopdev)
 
 # expect partitions named like loop0p1 etc
@@ -27,5 +23,5 @@ for looppart in $(ls /dev/$base?*); do
 done
 
 # cleanup
-run_check $SUDO_HELPER losetup -d $loopdev
-run_check truncate -s0 img
+cleanup_loop_dev img
+rm img
-- 
2.13.0

Re: [PATCH v2 0/6] Lowmem mode check check fix for mulit-device

[David Sterba]

On Wed, May 31, 2017 at 01:56:04PM +0800, Qu Wenruo wrote:
> This patchset will fix a false alert in lowmem mode, which doesn't handle
> RAID0/5/6/10 chunk well. (the 5th patch)
> 
> Along the lowmem fix, also enhance and cleanup some chunk verification code,
> as lowmem mode and original mode are using different chunk verification.
> (the 1st~4th patch)

Patches 1-5 applied, thanks.

> Finally, cleanup the loop device setup and move it to test/common other
> than handling in each test case. (the 6th patch)

That's a good cleanup, but the implementation has to be improved. I'll
comment in the patch.

Re: [PATCH v2 6/6] btrfs-progs: test: Introduce functions to prepare and cleanup loop device

[David Sterba]

On Wed, May 31, 2017 at 01:56:10PM +0800, Qu Wenruo wrote:
> +# prepare loop device using specified size and path
> +# $1: path of the file
> +# $2: size of the device, optional, default value is '2G'
> +prepare_loop_dev()
> +{
> +	local path="$1"
> +	local size="$2"
> +
> +	[[ "$path" ]] || _fail "path must be specified for prepare_loop_dev()"

There's _assert_path helper, for this purpose.

> +	[[ "$size" ]] || size='2G'
> +
> +	# Cleanup if it's already mounted or set up as loop device
> +	cleanup_loop_dev $path

Any reference to paths must be quoted.

> +	run_check touch $path
> +	chmod a+rw $path
> +	run_check truncate -s $size $path
> +
> +	run_check_stdout $SUDO_HELPER losetup --find --show $path
> +}
> +
> +# cleanup loop device based on its backend file
> +# $1: the path of the backend file
> +#
> +# We don't want to populate result in cleanup, so any error will only be
> +# caught manually, don't call run_check here.
> +cleanup_loop_dev()
> +{
> +	local path="$1"
> +
> +	loop_dev=$(losetup -l | tail -n +2 | grep $path | cut -f1 -d\ )

So, this lists existing loop devices, skips the heading, looks for some
random unquoted path, and extracts a filename. Also known as 'losetup -j $path'.

> +	if [ ! -z "$loop_dev" ]; then
> +		umount $loop_dev &> /dev/null

I'd like to preserve the output of commands, even if it fills the log.
It's usually helpful. The umount call is unconditional, so if you want
to discard erros when the device is not mounted, I suggest to do
'findmnt', umount and catch any errors. Eg. we want to know when the
umount fails, so we should not try to delete the loop device, as below.
Unexpectedly failing umount can be something worth investigating.

> +		$SUDO_HELPER losetup -d $loop_dev || \
> +			_fail "failed to detach $path"
> +	fi
> +}
> +
>  prepare_test_dev()
>  {
>  	# num[K/M/G/T...]
> diff --git a/tests/misc-tests/006-image-on-missing-device/test.sh b/tests/misc-tests/006-image-on-missing-device/test.sh
> index 5b6fe065..8249c0e9 100755
> --- a/tests/misc-tests/006-image-on-missing-device/test.sh
> +++ b/tests/misc-tests/006-image-on-missing-device/test.sh
> @@ -23,21 +23,15 @@ setup_root_helper
>  prepare_devices()
>  {
>  	for i in `seq $ndevs`; do
> -		touch img$i
> -		chmod a+rw img$i
> -		truncate -s0 img$i
> -		truncate -s2g img$i
> -		devs[$i]=`run_check_stdout $SUDO_HELPER losetup --find --show img$i`
> +		devs[$i]=$(prepare_loop_dev img$i)
>  	done
>  }
>  
>  cleanup_devices()
>  {
> -	for dev in ${devs[@]}; do
> -		run_mayfail $SUDO_HELPER losetup -d $dev
> -	done
>  	for i in `seq $ndevs`; do
> -		truncate -s0 img$i
> +		cleanup_loop_dev img$i
> +		rm img$i
>  	done
>  	run_check $SUDO_HELPER losetup --all
>  }
> diff --git a/tests/misc-tests/011-delete-missing-device/test.sh b/tests/misc-tests/011-delete-missing-device/test.sh
> index 5b5f9786..ad4b7d45 100755
> --- a/tests/misc-tests/011-delete-missing-device/test.sh
> +++ b/tests/misc-tests/011-delete-missing-device/test.sh
> @@ -16,21 +16,15 @@ setup_root_helper
>  prepare_devices()
>  {
>  	for i in `seq $ndevs`; do
> -		touch img$i
> -		chmod a+rw img$i
> -		truncate -s0 img$i
> -		truncate -s2g img$i
> -		devs[$i]=`run_check_stdout $SUDO_HELPER losetup --find --show img$i`
> +		devs[$i]=$(prepare_loop_dev img$i)
>  	done
>  }
>  
>  cleanup_devices()
>  {
> -	for dev in ${devs[@]}; do
> -		run_mayfail $SUDO_HELPER losetup -d $dev
> -	done
>  	for i in `seq $ndevs`; do
> -		truncate -s0 img$i
> +		cleanup_loop_dev img$i
> +		rm img$i
>  	done
>  	run_check $SUDO_HELPER losetup --all
>  }
> diff --git a/tests/mkfs-tests/001-basic-profiles/test.sh b/tests/mkfs-tests/001-basic-profiles/test.sh
> index 0dc9a2bd..98bd9e6b 100755
> --- a/tests/mkfs-tests/001-basic-profiles/test.sh
> +++ b/tests/mkfs-tests/001-basic-profiles/test.sh
> @@ -16,21 +16,15 @@ setup_root_helper
>  prepare_devices()
>  {
>  	for i in `seq $ndevs`; do
> -		touch img$i
> -		chmod a+rw img$i
> -		truncate -s0 img$i
> -		truncate -s2g img$i
> -		devs[$i]=`run_check_stdout $SUDO_HELPER losetup --find --show img$i`
> +		devs[$i]=$(prepare_loop_dev img$i)
>  	done
>  }
>  
>  cleanup_devices()
>  {
> -	for dev in ${devs[@]}; do
> -		run_check $SUDO_HELPER losetup -d $dev
> -	done
>  	for i in `seq $ndevs`; do
> -		truncate -s0 img$i
> +		cleanup_loop_dev img$i
> +		rm img$i
>  	done
>  	run_check $SUDO_HELPER losetup --all
>  }
> diff --git a/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh b/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh
> index 63fb1785..aacff6ee 100755
> --- a/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh
> +++ b/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh
> @@ -13,11 +13,7 @@ dmname=\
>  btrfs-test-with-very-long-name-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
>  dmdev=/dev/mapper/$dmname
>  
> -run_check truncate -s0 img
> -chmod a+w img
> -run_check truncate -s2g img
> -
> -loopdev=`run_check_stdout $SUDO_HELPER losetup --find --show img`
> +loopdev=$(prepare_loop_dev img)
>  run_check $SUDO_HELPER dmsetup create $dmname --table "0 1048576 linear $loopdev 0"
>  
>  dmbase=`readlink -f $dmdev`
> @@ -36,5 +32,5 @@ run_check $SUDO_HELPER $TOP/btrfs inspect-internal dump-super $dmdev
>  
>  # cleanup
>  run_check $SUDO_HELPER dmsetup remove $dmname
> -run_mayfail $SUDO_HELPER losetup -d $loopdev
> -run_check truncate -s0 img
> +cleanup_loop_dev img
> +rm img
> diff --git a/tests/mkfs-tests/006-partitioned-loopdev/test.sh b/tests/mkfs-tests/006-partitioned-loopdev/test.sh
> index 12f37842..b005ef3d 100755
> --- a/tests/mkfs-tests/006-partitioned-loopdev/test.sh
> +++ b/tests/mkfs-tests/006-partitioned-loopdev/test.sh
> @@ -12,12 +12,8 @@ check_prereq mkfs.btrfs
>  
>  setup_root_helper
>  
> -run_check truncate -s0 img
> -chmod a+w img
>  cp partition-1g-1g img
> -run_check truncate -s2g img
> -
> -loopdev=$(run_check_stdout $SUDO_HELPER losetup --partscan --find --show img)
> +loopdev=$(prepare_loop_dev img)
>  base=$(basename $loopdev)
>  
>  # expect partitions named like loop0p1 etc
> @@ -27,5 +23,5 @@ for looppart in $(ls /dev/$base?*); do
>  done
>  
>  # cleanup
> -run_check $SUDO_HELPER losetup -d $loopdev
> -run_check truncate -s0 img
> +cleanup_loop_dev img
> +rm img
> -- 
> 2.13.0
> 
> 
>