linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH blktests 1/2] loop/001: fix typo 'paritition' to 'partition'
@ 2019-03-14 11:45 Dongli Zhang
  2019-03-14 11:45 ` [PATCH blktests 2/2] loop/001: verify all partitions are removed Dongli Zhang
  0 siblings, 1 reply; 7+ messages in thread
From: Dongli Zhang @ 2019-03-14 11:45 UTC (permalink / raw)
  To: linux-block, osandov; +Cc: jack

Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
---
 tests/loop/001 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/loop/001 b/tests/loop/001
index b70e8c0..47f760a 100755
--- a/tests/loop/001
+++ b/tests/loop/001
@@ -2,7 +2,7 @@
 # SPDX-License-Identifier: GPL-3.0+
 # Copyright (C) 2017 Omar Sandoval
 #
-# Test loop device paritition scanning. Regression test for commit e02898b42380
+# Test loop device partition scanning. Regression test for commit e02898b42380
 # ("loop: fix LO_FLAGS_PARTSCAN hang").
 
 . tests/loop/rc
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH blktests 2/2] loop/001: verify all partitions are removed
  2019-03-14 11:45 [PATCH blktests 1/2] loop/001: fix typo 'paritition' to 'partition' Dongli Zhang
@ 2019-03-14 11:45 ` Dongli Zhang
  2019-03-14 17:55   ` Omar Sandoval
  0 siblings, 1 reply; 7+ messages in thread
From: Dongli Zhang @ 2019-03-14 11:45 UTC (permalink / raw)
  To: linux-block, osandov; +Cc: jack

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.

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()").
 
 . 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
 	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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH blktests 2/2] loop/001: verify all partitions are removed
  2019-03-14 11:45 ` [PATCH blktests 2/2] loop/001: verify all partitions are removed Dongli Zhang
@ 2019-03-14 17:55   ` Omar Sandoval
  2019-03-15  2:00     ` Dongli Zhang
  0 siblings, 1 reply; 7+ messages in thread
From: Omar Sandoval @ 2019-03-14 17:55 UTC (permalink / raw)
  To: Dongli Zhang; +Cc: linux-block, osandov, jack

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
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH blktests 2/2] loop/001: verify all partitions are removed
  2019-03-14 17:55   ` Omar Sandoval
@ 2019-03-15  2:00     ` Dongli Zhang
  2019-03-21 23:26       ` Omar Sandoval
  0 siblings, 1 reply; 7+ messages in thread
From: Dongli Zhang @ 2019-03-15  2:00 UTC (permalink / raw)
  To: Omar Sandoval; +Cc: linux-block, osandov, jack



On 3/15/19 1:55 AM, Omar Sandoval wrote:
> 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.

Seems 'lsblk' does not work here.

step1: truncate -s 100M /tmp/tmp.raw
step2: parted /tmp/tmp.raw --script mklabel msdos \
       mkpart primary 0% 50% mkpart primary 50% 100%
step3: losetup -P -f --show /tmp/tmp.raw

Now we are able to see two loop partitions from 'lsblk'

# lsblk -ln /dev/loop0
loop0     7:0    0  100M  0 loop
loop0p1 259:0    0   50M  0 loop
loop0p2 259:1    0   50M  0 loop


step4: # losetup -d /dev/loop0

There is below syslog as  partscan is failed.

[  261.181049] __loop_clr_fd: partition scan of loop0 failed (rc=-22)


There are 2 partitions left:

# ls /dev | grep loop0
loop0
loop0p1
loop0p2

# ls /sys/block/loop0 | grep loop
loop0p1
loop0p2


However, 'lsblk -ln' does not report the orphan paritions:

# lsblk -ln
sr0   11:0    1 1024M  0 rom
sda    8:0    0   20G  0 disk
sda2   8:2    0    1K  0 part
sda5   8:5    0  4.1G  0 part [SWAP]
sda1   8:1    0 15.9G  0 part /


Therefore, we would not be able to use 'lsblk' here.

Dongli Zhang

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH blktests 2/2] loop/001: verify all partitions are removed
  2019-03-15  2:00     ` Dongli Zhang
@ 2019-03-21 23:26       ` Omar Sandoval
  2019-03-22  2:23         ` Dongli Zhang
  0 siblings, 1 reply; 7+ messages in thread
From: Omar Sandoval @ 2019-03-21 23:26 UTC (permalink / raw)
  To: Dongli Zhang; +Cc: linux-block, osandov, jack

On Fri, Mar 15, 2019 at 10:00:27AM +0800, Dongli Zhang wrote:
> 
> 
> On 3/15/19 1:55 AM, Omar Sandoval wrote:
> > 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.
> 
> Seems 'lsblk' does not work here.
> 
> step1: truncate -s 100M /tmp/tmp.raw
> step2: parted /tmp/tmp.raw --script mklabel msdos \
>        mkpart primary 0% 50% mkpart primary 50% 100%
> step3: losetup -P -f --show /tmp/tmp.raw
> 
> Now we are able to see two loop partitions from 'lsblk'
> 
> # lsblk -ln /dev/loop0
> loop0     7:0    0  100M  0 loop
> loop0p1 259:0    0   50M  0 loop
> loop0p2 259:1    0   50M  0 loop
> 
> 
> step4: # losetup -d /dev/loop0
> 
> There is below syslog as  partscan is failed.
> 
> [  261.181049] __loop_clr_fd: partition scan of loop0 failed (rc=-22)
> 
> 
> There are 2 partitions left:
> 
> # ls /dev | grep loop0
> loop0
> loop0p1
> loop0p2
> 
> # ls /sys/block/loop0 | grep loop
> loop0p1
> loop0p2
> 
> 
> However, 'lsblk -ln' does not report the orphan paritions:
> 
> # lsblk -ln
> sr0   11:0    1 1024M  0 rom
> sda    8:0    0   20G  0 disk
> sda2   8:2    0    1K  0 part
> sda5   8:5    0  4.1G  0 part [SWAP]
> sda1   8:1    0 15.9G  0 part /
> 
> 
> Therefore, we would not be able to use 'lsblk' here.

I see. I think we should check both lsblk and sysfs here. How about
something like
https://github.com/osandov/blktests/commit/6c1237cd358008024ece90bd915a67c23add8a2a?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH blktests 2/2] loop/001: verify all partitions are removed
  2019-03-21 23:26       ` Omar Sandoval
@ 2019-03-22  2:23         ` Dongli Zhang
  2019-03-25 17:32           ` Omar Sandoval
  0 siblings, 1 reply; 7+ messages in thread
From: Dongli Zhang @ 2019-03-22  2:23 UTC (permalink / raw)
  To: Omar Sandoval; +Cc: linux-block, osandov, jack



On 3/22/19 7:26 AM, Omar Sandoval wrote:
> On Fri, Mar 15, 2019 at 10:00:27AM +0800, Dongli Zhang wrote:
>>
>>
>> On 3/15/19 1:55 AM, Omar Sandoval wrote:
>>> 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.
>>
>> Seems 'lsblk' does not work here.
>>
>> step1: truncate -s 100M /tmp/tmp.raw
>> step2: parted /tmp/tmp.raw --script mklabel msdos \
>>        mkpart primary 0% 50% mkpart primary 50% 100%
>> step3: losetup -P -f --show /tmp/tmp.raw
>>
>> Now we are able to see two loop partitions from 'lsblk'
>>
>> # lsblk -ln /dev/loop0
>> loop0     7:0    0  100M  0 loop
>> loop0p1 259:0    0   50M  0 loop
>> loop0p2 259:1    0   50M  0 loop
>>
>>
>> step4: # losetup -d /dev/loop0
>>
>> There is below syslog as  partscan is failed.
>>
>> [  261.181049] __loop_clr_fd: partition scan of loop0 failed (rc=-22)
>>
>>
>> There are 2 partitions left:
>>
>> # ls /dev | grep loop0
>> loop0
>> loop0p1
>> loop0p2
>>
>> # ls /sys/block/loop0 | grep loop
>> loop0p1
>> loop0p2
>>
>>
>> However, 'lsblk -ln' does not report the orphan paritions:
>>
>> # lsblk -ln
>> sr0   11:0    1 1024M  0 rom
>> sda    8:0    0   20G  0 disk
>> sda2   8:2    0    1K  0 part
>> sda5   8:5    0  4.1G  0 part [SWAP]
>> sda1   8:1    0 15.9G  0 part /
>>
>>
>> Therefore, we would not be able to use 'lsblk' here.
> 
> I see. I think we should check both lsblk and sysfs here. How about
> something like
> https://github.com/osandov/blktests/commit/6c1237cd358008024ece90bd915a67c23add8a2a?
> 

It is good to me. Thank you very much for improve the patchset! I have tested it
would pass with commit 758a58d0bc67 and not pass w/o 758a58d0bc67.

To check both lsblk and sysfs may test the loop much more thoroughly.


Just one programming question about loop_partition_sysfs():

+find_loop_partition_sysfs() {
+	find "$sysfs/" -mindepth 1 -maxdepth 1 -name "$1"'p*' -printf '%f\n' |
+		sed -n "s/^${1}p//p" | sort
+}

Why not something like:

ls "$sysfs/" | grep $1 | sed -n "s/^${1}p//p" | sort

Is that because it is better to use 'find' than 'ls' in bash programming?

Thank you very much!

Dongli Zhang

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH blktests 2/2] loop/001: verify all partitions are removed
  2019-03-22  2:23         ` Dongli Zhang
@ 2019-03-25 17:32           ` Omar Sandoval
  0 siblings, 0 replies; 7+ messages in thread
From: Omar Sandoval @ 2019-03-25 17:32 UTC (permalink / raw)
  To: Dongli Zhang; +Cc: linux-block, osandov, jack

On Fri, Mar 22, 2019 at 10:23:01AM +0800, Dongli Zhang wrote:
> 
> 
> On 3/22/19 7:26 AM, Omar Sandoval wrote:
> > On Fri, Mar 15, 2019 at 10:00:27AM +0800, Dongli Zhang wrote:
> >>
> >>
> >> On 3/15/19 1:55 AM, Omar Sandoval wrote:
> >>> 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.
> >>
> >> Seems 'lsblk' does not work here.
> >>
> >> step1: truncate -s 100M /tmp/tmp.raw
> >> step2: parted /tmp/tmp.raw --script mklabel msdos \
> >>        mkpart primary 0% 50% mkpart primary 50% 100%
> >> step3: losetup -P -f --show /tmp/tmp.raw
> >>
> >> Now we are able to see two loop partitions from 'lsblk'
> >>
> >> # lsblk -ln /dev/loop0
> >> loop0     7:0    0  100M  0 loop
> >> loop0p1 259:0    0   50M  0 loop
> >> loop0p2 259:1    0   50M  0 loop
> >>
> >>
> >> step4: # losetup -d /dev/loop0
> >>
> >> There is below syslog as  partscan is failed.
> >>
> >> [  261.181049] __loop_clr_fd: partition scan of loop0 failed (rc=-22)
> >>
> >>
> >> There are 2 partitions left:
> >>
> >> # ls /dev | grep loop0
> >> loop0
> >> loop0p1
> >> loop0p2
> >>
> >> # ls /sys/block/loop0 | grep loop
> >> loop0p1
> >> loop0p2
> >>
> >>
> >> However, 'lsblk -ln' does not report the orphan paritions:
> >>
> >> # lsblk -ln
> >> sr0   11:0    1 1024M  0 rom
> >> sda    8:0    0   20G  0 disk
> >> sda2   8:2    0    1K  0 part
> >> sda5   8:5    0  4.1G  0 part [SWAP]
> >> sda1   8:1    0 15.9G  0 part /
> >>
> >>
> >> Therefore, we would not be able to use 'lsblk' here.
> > 
> > I see. I think we should check both lsblk and sysfs here. How about
> > something like
> > https://github.com/osandov/blktests/commit/6c1237cd358008024ece90bd915a67c23add8a2a?
> > 
> 
> It is good to me. Thank you very much for improve the patchset! I have tested it
> would pass with commit 758a58d0bc67 and not pass w/o 758a58d0bc67.
> 
> To check both lsblk and sysfs may test the loop much more thoroughly.

Thanks, applied.

> Just one programming question about loop_partition_sysfs():
> 
> +find_loop_partition_sysfs() {
> +	find "$sysfs/" -mindepth 1 -maxdepth 1 -name "$1"'p*' -printf '%f\n' |
> +		sed -n "s/^${1}p//p" | sort
> +}
> 
> Why not something like:
> 
> ls "$sysfs/" | grep $1 | sed -n "s/^${1}p//p" | sort
> 
> Is that because it is better to use 'find' than 'ls' in bash programming?

I usually prefer find because you can control the output more exactly.
ls probably would've been fine in this case.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-03-25 17:33 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-14 11:45 [PATCH blktests 1/2] loop/001: fix typo 'paritition' to 'partition' Dongli Zhang
2019-03-14 11:45 ` [PATCH blktests 2/2] loop/001: verify all partitions are removed Dongli Zhang
2019-03-14 17:55   ` Omar Sandoval
2019-03-15  2:00     ` Dongli Zhang
2019-03-21 23:26       ` Omar Sandoval
2019-03-22  2:23         ` Dongli Zhang
2019-03-25 17:32           ` Omar Sandoval

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).