Hi Colin, On Sun, May 19, 2013 at 12:11:30AM +0100, Colin Watson wrote: > > When grub-mount fails, os-prober sets the device read-only and tries to > > mount > > it. It looks like the error reported in this bug was caused by setting the > > device read-only, not by mounting it. The mount didn't change anything, > > because the device was read-only. The mount probably would have changed the > > filesystem (by replaying the journal) if the device was not read-only. > > > > A fix for this could be to create a read-only linear device with > > device-mapper > > and mounting that, instead of setting the device read-only. That way, the > > device itself doesn't have to be changed. This can only work if device > > mapper > > is available. > > Indeed. If you can figure out how to make this work reliably, this > would be a nice general fix. I hadn't realised that blockdev had the > potential to trip this kind of breakage when I wrote that code.
I created a first patch to do this (and add some more debug info). To test this patch, you need to make sure that grub-mount is unavailable (otherwise grub-mount will be used instead of the new code). If dmsetup is not available, os-prober will fall back to the old code. Maybe this shouldn't happen by default. Also, os-prober could recommend dmsetup, to make sure it is available most of the time. Cheers, Ivo
>From 99a2fd2241b06a7ae282cc91118f9d418dc27c20 Mon Sep 17 00:00:00 2001 From: Ivo De Decker <ivo.dedec...@ugent.be> Date: Sat, 13 Jul 2013 21:17:54 +0200 Subject: [PATCH 1/4] log when partition set to ro/rw --- common.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/common.sh b/common.sh index 30e245e..13eca64 100644 --- a/common.sh +++ b/common.sh @@ -8,6 +8,7 @@ cleanup () { local partition for partition in $cleanup_ro_partitions; do blockdev --setrw "$partition" + debug "partition $partition set to read-write" done if $cleanup_tmpdir; then rm -rf "$OS_PROBER_TMP" @@ -150,8 +151,11 @@ ro_partition () { if type blockdev >/dev/null 2>&1 && \ [ "$(blockdev --getro "$1")" = 0 ] && \ blockdev --setro "$1"; then + debug "partition $1 set to read-only" cleanup_ro_partitions="${cleanup_ro_partitions:+$cleanup_ro_partitions }$1" trap cleanup EXIT HUP INT QUIT TERM + else + warn "setting partition $1 to read-only failed" fi } -- 1.8.3.2
>From 3088c2a713c87a057554d2716aa700381b1b6b48 Mon Sep 17 00:00:00 2001 From: Ivo De Decker <ivo.dedec...@ugent.be> Date: Sat, 13 Jul 2013 21:13:05 +0200 Subject: [PATCH 2/4] cleanup in separate function do_unmount --- os-probes/common/50mounted-tests | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/os-probes/common/50mounted-tests b/os-probes/common/50mounted-tests index 561163b..c69566a 100755 --- a/os-probes/common/50mounted-tests +++ b/os-probes/common/50mounted-tests @@ -5,6 +5,16 @@ partition="$1" . /usr/share/os-prober/common.sh +do_unmount() { + if [ "$mounted" ] + then + if ! umount "$tmpmnt"; then + warn "failed to umount $tmpmnt" + fi + fi + rmdir "$tmpmnt" || true +} + types="$(fs_type "$partition")" || types=NOT-DETECTED if [ "$types" = NOT-DETECTED ]; then debug "$1 type not recognised; skipping" @@ -74,20 +84,13 @@ if [ "$mounted" ]; then if [ -f "$test" ] && [ -x "$test" ]; then if "$test" "$partition" "$tmpmnt" "$type"; then debug "os found by subtest $test" - if ! umount "$tmpmnt"; then - warn "failed to umount $tmpmnt" - fi - rmdir "$tmpmnt" || true + do_unmount exit 0 fi fi done - if ! umount "$tmpmnt"; then - warn "failed to umount $tmpmnt" - fi fi - -rmdir "$tmpmnt" || true +do_unmount # No tests found anything. exit 1 -- 1.8.3.2
>From fe4d6e5dedc48b02ed0724ec71c80954a8f468a5 Mon Sep 17 00:00:00 2001 From: Ivo De Decker <ivo.dedec...@ugent.be> Date: Sun, 14 Jul 2013 13:26:43 +0200 Subject: [PATCH 3/4] log result from mount command --- os-probes/common/50mounted-tests | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/os-probes/common/50mounted-tests b/os-probes/common/50mounted-tests index c69566a..9a0d23d 100755 --- a/os-probes/common/50mounted-tests +++ b/os-probes/common/50mounted-tests @@ -70,10 +70,12 @@ if type grub-mount >/dev/null 2>&1 && \ else ro_partition "$partition" for type in $types $delaytypes; do - if mount -o ro -t "$type" "$partition" "$tmpmnt" 2>/dev/null; then + if mountinfo=`mount -o ro -t "$type" "$partition" "$tmpmnt" 2>&1`; then debug "mounted as $type filesystem" mounted=1 break + else + debug "mounting $partition as $type failed: $mountinfo" fi done fi -- 1.8.3.2
>From a3769674b7820b30eb6f7ca81abda058918816ce Mon Sep 17 00:00:00 2001 From: Ivo De Decker <ivo.dedec...@ugent.be> Date: Sat, 13 Jul 2013 21:15:41 +0200 Subject: [PATCH 4/4] using read-only device mapper entry --- os-probes/common/50mounted-tests | 46 +++++++++++++++++++++++++++++++++------- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/os-probes/common/50mounted-tests b/os-probes/common/50mounted-tests index 9a0d23d..33d9255 100755 --- a/os-probes/common/50mounted-tests +++ b/os-probes/common/50mounted-tests @@ -12,6 +12,11 @@ do_unmount() { warn "failed to umount $tmpmnt" fi fi + if [ -e "$dm_device" ] + then + debug remove device mapper device $dm_device + dmsetup remove $dm_device + fi rmdir "$tmpmnt" || true } @@ -68,16 +73,41 @@ if type grub-mount >/dev/null 2>&1 && \ type=fuseblk fi else - ro_partition "$partition" - for type in $types $delaytypes; do - if mountinfo=`mount -o ro -t "$type" "$partition" "$tmpmnt" 2>&1`; then - debug "mounted as $type filesystem" - mounted=1 - break + if type dmsetup >/dev/null 2>&1 && \ + type blockdev >/dev/null 2>&1 + then + partition_name="osprober-${partition##*/}" + dm_device="/dev/mapper/$partition_name" + size_p=$(blockdev --getsize $partition ) + if [ -e "$dm_device" ] + then + error "$dm_device already exists" + dm_device= else - debug "mounting $partition as $type failed: $mountinfo" + debug creating device mapper device $dm_device + echo "0 $size_p linear $partition 0" | dmsetup create -r $partition_name + for type in $types $delaytypes; do + if mountinfo=`mount -o ro -t "$type" "$dm_device" "$tmpmnt" 2>&1`; then + debug "mounted as $type filesystem" + mounted=1 + break + else + debug "mounting $dm_device ($partition) as $type failed: $mountinfo" + fi + done fi - done + else + ro_partition "$partition" + for type in $types $delaytypes; do + if mountinfo=`mount -o ro -t "$type" "$partition" "$tmpmnt" 2>&1`; then + debug "mounted as $type filesystem" + mounted=1 + break + else + debug "mounting $partition as $type failed: $mountinfo" + fi + done + fi fi if [ "$mounted" ]; then -- 1.8.3.2