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

[Jim Meyering]

address@hidden wrote:
> ----- Mail original -----
>> De: "Jim Meyering" <address@hidden>
>> À: "Gilles Espinasse" <address@hidden>
>> Cc: address@hidden, "petr uzel" <address@hidden>
>> Envoyé: Samedi 6 Octobre 2012 20:35:08
>> Objet: Re: [PATCH] parted-3.1, fix t8001 not to fail if modprobe fail
>>
> ...
>>
>> 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.
>>
> With your patch, this case work
> make check -C tests TESTS="t8000-loop.sh t8001-loop-blkpg.sh"
> ...
> t8000-loop.sh: skipped test: your system does not support loop partitioning
> SKIP: t8000-loop.sh
> t8001-loop-blkpg.sh: skipped test: your system does not support loop 
> partitioning
> SKIP: t8001-loop-blkpg.sh
>
> But not that corner case
> rmmod loop; make check -C tests TESTS="t8001-loop-blkpg.sh"
> FAIL: t8001-loop-blkpg.sh
>
> as t8000-loop.sh somehow use losetup differently.
> t8000 test load loop module but t8001 "loopdev=$(losetup -f --show 
> backing_file" do not.
>
> So I would propose instead this change that result in
> rmmod loop; make check -C tests TESTS="t8001-loop-blkpg.sh"
> with
> t8001-loop-blkpg.sh: skipped test: your system does not support loop 
> partitioning
> SKIP: t8001-loop-blkpg.sh
>
> or when partition is enabled on loop give
> make check -C tests TESTS="t8001-loop-blkpg.sh"
> PASS: t8001-loop-blkpg.sh
>
> What do you think to change the skip_ message to
> -      0|1) skip_ your system does not support loop partitioning;;
> +      0|1) skip_ your system is not configured with loop partitioning 
> support;;
>
> That was unclear to me just looking at parted log result that I could have 
> enabled that support (on 2.6.32 kernel)

Thanks.  You're right: that can be improved:

diff --git a/tests/init.cfg b/tests/init.cfg
index 24b10bc..dc8b2bc 100644
--- a/tests/init.cfg
+++ b/tests/init.cfg
@@ -120,8 +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
-    0|1) skip_ your system does not support loop partitioning;;
+  local f=/sys/devices/virtual/block/$(basename $1)/ext_range
+  case $(cat "$f") in
+    ''|0|1) skip_ your system is not configured to partition loop devices;;
   esac
 }

> Anyway, here is the patch
>
> 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 t-lvm loop_setup_ to load the loop module, if required.

Very nice.  Thanks for the improvement.
I've made minor log tweaks:

>From 16c0a1aaca7283cd845de90862c911fa60e3f6f6 Mon Sep 17 00:00:00 2001
From: Gilles Espinasse <address@hidden>
Date: Sun, 7 Oct 2012 15:40:23 +0200
Subject: [PATCH 2/2] 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 t-lvm loop_setup_ to load the loop module, if required.
---
 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..9afde4a 100755
--- a/tests/t8001-loop-blkpg.sh
+++ b/tests/t8001-loop-blkpg.sh
@@ -20,6 +20,7 @@

 require_root_
 require_udevadm_settle_
+lvm_init_root_dir_

 cleanup_fn_()
 {
@@ -27,21 +28,14 @@ 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

 # Set up loop device on top of backing file
-loopdev=$(losetup -f --show backing_file)
+loopdev=$(loop_setup_ 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.rc1