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 <[email protected]>
> >> ---
> >> 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?
