Bug#701814: Re: Bug#701814: os-prober: damages XFS exported via iSCSI but not mounted locally; potential data loss
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.dedecker@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.dedecker@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.dedecker@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.dedecker@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
Reply to: