Re: [PATCH] parted-3.1, fix t8001 not to fail if modprobe fail

[Jim Meyering]

Gilles Espinasse wrote:
> Remove 'rmmod loop' and 'modprobe loop max_part=7' instructions
> The second command may fail after the first command have run, leaving the 
> machine with no loop support.
>
> This happen on my chroot as
> - rmmod do not depend if the loop module is available,
> - modprobe does not work as the kernel compiled inside the chroot is 
> different from the running kernel.
>
> Instead detect earlier if the system support loop with partitions assuming 
> all loop dev have the same number of allowed partitions

Hi Gilles,

Thanks for the patch.
I've removed your Signed-off-by (considered redundant by me,
when the name is identical to that of the author) and adjusted
the log message.  If was going to ask you to ACK the following,
but then tested it on a debian unstable system, where I had first
verified that the command

  cat /sys/devices/virtual/block/loop0/ext_range

would fail, because there was no such file.
(loop module was not loaded)
That meant that the require_partitionable_loop_device_ /dev/loop0
test was passing even though the above cat printed
only a file-not-found diagnostic.  Oops.

I'm about to fix that test with this patch:

diff --git a/tests/init.cfg b/tests/init.cfg
index 24b10bc..3590364 100644
--- a/tests/init.cfg
+++ b/tests/init.cfg
@@ -120,7 +120,9 @@ require_erasable_()
 # At least Fedora 16 (kernel 3.1.6-1.fc16.x86_64) fails this test.
 require_partitionable_loop_device_()
 {
-  case $(cat /sys/devices/virtual/block/$(basename $1)/ext_range) in
+  local f=/sys/devices/virtual/block/$(basename $1)/ext_range
+  case $(cat "$f") in
+    '') skip_ "no such file? $f";;
     0|1) skip_ your system does not support loop partitioning;;
   esac
 }

Here's my initially modified patch:

>From ae674024aa91ccf131222f340bd1dd93ff7c6178 Mon Sep 17 00:00:00 2001
From: Gilles Espinasse <address@hidden>
Date: Sat, 6 Oct 2012 10:57:20 +0200
Subject: [PATCH] tests: t8001: do not rely on "modprobe loop"

Remove 'rmmod loop' and 'modprobe loop max_part=7' commands.
The latter command may fail after the first command has run,
leaving the machine with no loop support.

This happen on my chroot as
- rmmod does not depend on the availability of the loop module,
- modprobe does not work as the kernel compiled inside the chroot
  is different from the running kernel.

Instead detect earlier whether the system supports loop partitions,
assuming all loop dev have the same number of allowed partitions.
---
 tests/t8001-loop-blkpg.sh | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/tests/t8001-loop-blkpg.sh b/tests/t8001-loop-blkpg.sh
index deef18b..e33d9ae 100755
--- a/tests/t8001-loop-blkpg.sh
+++ b/tests/t8001-loop-blkpg.sh
@@ -27,13 +27,9 @@ cleanup_fn_()
     && { udevadm settle --timeout=3; losetup -d "$loopdev"; }
 }

-# If the loop module is loaded, unload it first
-if lsmod | grep '^loop[[:space:]]'; then
-    rmmod loop || fail=1
-fi
-
-# Insert loop module with max_part > 1
-modprobe loop max_part=7 || fail=1
+# Check on the first loop for the max_part supported on all loop
+# loop module with partition support require using modprobe loop max_part=7
+require_partitionable_loop_device_ /dev/loop0

 # Create backing file
 dd if=/dev/zero of=backing_file bs=1M count=4 >/dev/null 2>&1 || fail=1
@@ -42,8 +38,6 @@ dd if=/dev/zero of=backing_file bs=1M count=4 >/dev/null 2>&1 
|| fail=1
 loopdev=$(losetup -f --show backing_file)
 test -z "$loopdev" && fail=1

-require_partitionable_loop_device_ $loopdev
-
 # Expect this to succeed
 parted -s "$loopdev" mklabel msdos > err 2>&1 || fail=1
 compare /dev/null err || fail=1     # expect no output
--
1.8.0.rc0.18.gf84667d
---------------------------------------------------------------------------

Here's a better version of your patch, which leaves
the require_partitionable_loop_device_ call after losetup.
Please review it.
I'll wait for your ACK, since it's still in your name.

>From 03f6391dc4cf869fe9177deac75c060132f20a81 Mon Sep 17 00:00:00 2001
From: Gilles Espinasse <address@hidden>
Date: Sat, 6 Oct 2012 10:57:20 +0200
Subject: [PATCH] tests: t8001: do not rely on "modprobe loop"

Remove 'rmmod loop' and 'modprobe loop max_part=7' commands.
The latter command may fail after the first command has run,
leaving the machine with no loop support.

This happen on my chroot as
- rmmod does not depend on the availability of the loop module,
- modprobe does not work as the kernel compiled inside the chroot
  is different from the running kernel.

Instead, rely on losetup to load the loop module, if required.
---
 tests/t8001-loop-blkpg.sh | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/tests/t8001-loop-blkpg.sh b/tests/t8001-loop-blkpg.sh
index deef18b..23256b1 100755
--- a/tests/t8001-loop-blkpg.sh
+++ b/tests/t8001-loop-blkpg.sh
@@ -27,14 +27,6 @@ cleanup_fn_()
     && { udevadm settle --timeout=3; losetup -d "$loopdev"; }
 }

-# If the loop module is loaded, unload it first
-if lsmod | grep '^loop[[:space:]]'; then
-    rmmod loop || fail=1
-fi
-
-# Insert loop module with max_part > 1
-modprobe loop max_part=7 || fail=1
-
 # Create backing file
 dd if=/dev/zero of=backing_file bs=1M count=4 >/dev/null 2>&1 || fail=1

@@ -42,6 +34,7 @@ dd if=/dev/zero of=backing_file bs=1M count=4 >/dev/null 2>&1 
|| fail=1
 loopdev=$(losetup -f --show backing_file)
 test -z "$loopdev" && fail=1

+# Skip this test if loop devices are not partitionable.
 require_partitionable_loop_device_ $loopdev

 # Expect this to succeed
--
1.8.0.rc0.18.gf84667d