g....@free.fr wrote:
> ----- Mail original -----
>> De: "Jim Meyering" <j...@meyering.net>
>> À: "Gilles Espinasse" <g....@free.fr>
>> Cc: bug-parted@gnu.org, "petr uzel" <petr.u...@suse.cz>
>> 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 <g....@free.fr>
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
