Re: [PATCH blktests 2/2] loop/001: verify all partitions are removed

[Omar Sandoval <osandov@osandov.com>]

On Thu, Mar 14, 2019 at 07:45:17PM +0800, Dongli Zhang wrote:
> loop/001 does not test whether all partitions are removed successfully
> during loop device partition scanning. As a result, the regression
> introduced by 0da03cab87e6 ("loop: Fix deadlock when calling
> blkdev_reread_part()") can not be detected.
> 
> The regression will generate below message in dmesg:
> 
> [  464.414043] __loop_clr_fd: partition scan of loop0 failed (rc=-22)
> 
> and leave orphan partitions like below:
> 
> - /dev/loop0p1
> - /sys/block/loop0/loop0p1
> 
> This patch verifies all partitions are removed by checking if there is
> /sys/block/loopX/loopXpY left. The expected number of partitions left is 0.

Thanks for the test! A couple of comments below.

> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
> ---
>  tests/loop/001     | 5 +++++
>  tests/loop/001.out | 1 +
>  2 files changed, 6 insertions(+)
> 
> diff --git a/tests/loop/001 b/tests/loop/001
> index 47f760a..a0326b7 100755
> --- a/tests/loop/001
> +++ b/tests/loop/001
> @@ -4,6 +4,9 @@
>  #
>  # Test loop device partition scanning. Regression test for commit e02898b42380
>  # ("loop: fix LO_FLAGS_PARTSCAN hang").
> +#
> +# Test loop device paritition scanning. Regression test for commit 758a58d0bc67
> +# ("loop: set GENHD_FL_NO_PART_SCAN after blkdev_reread_part()").

These can just be combined to

# Test loop device partition scanning. Regression test for commits e02898b42380
# ("loop: fix LO_FLAGS_PARTSCAN hang") and 758a58d0bc67 ("loop: set
# GENHD_FL_NO_PART_SCAN after blkdev_reread_part()").

>  . tests/loop/rc
>  
> @@ -24,9 +27,11 @@ test() {
>  		mkpart primary 50% 100%
>  
>  	loop_device="$(losetup -P -f --show "$TMPDIR/img")"
> +	loop_name=${loop_device:5}
>  	lsblk -ln "$loop_device" | wc -l
>  
>  	losetup -d "$loop_device"
> +	ls /sys/block/$loop_name | grep loop | wc -l

We can just repeat the same `lsblk -ln "$loop_device" | wc -l` from
earlier, right? That's a bit cleaner than the hardcoded string slicing
and ls.

>  	rm "$TMPDIR/img"
>  	echo "Test complete"
>  }
> diff --git a/tests/loop/001.out b/tests/loop/001.out
> index 75979f0..8c4917f 100644
> --- a/tests/loop/001.out
> +++ b/tests/loop/001.out
> @@ -1,3 +1,4 @@
>  Running loop/001
>  3
> +0
>  Test complete
> -- 
> 2.7.4
>