Re: locking devices and partitions in partman
I've attached a new version of the same patch, with a number of changes
as described below...
On Wed, Jun 07, 2006 at 01:00:22AM +0200, Frans Pop wrote:
One problem I see at first glance is one of translation: you cannot
hardcode a string like "In use by LVM VG $vg" for $MESSAGE, unless you
really want all translators (led by Christian) coming after you with a
heavy cluebat...
Translatable strings included
...
The function names look potentially confusing too as (un)lock_device can
be used for both devices and partitions.
Functions renamed to partman_(un)lock_unit
Would it not be cleaner to also pass the partition as a parameter instead
of using $id? Are you sure that will always be set correctly?
Looking at the likely use cases, I've instead kept it the way it is now.
The reason is that the device node (e.g. /dev/sda2) is more readily
available in the parts where we might want to do the locking (like when
cryptsetup /dev/something or pvcreate + vgcreate /dev/something has been
executed).
The new patch is attached.
Regards,
David
Index: packages/partman/partman-base/debian/partman-base.templates
===================================================================
--- packages/partman/partman-base/debian/partman-base.templates (revision 37887)
+++ packages/partman/partman-base/debian/partman-base.templates (working copy)
@@ -14,6 +14,22 @@
Type: text
_Description: Detecting file systems...
+Template: partman-base/devicelocked
+Type: error
+_Description: The device is currently in use
+ No modifications can be made to the device ${DEVICE} for the
+ following reasons:
+ .
+ ${MESSAGE}
+
+Template: partman-base/partlocked
+Type: error
+_Description: The partition is currently in use
+ No modifications can be made to partition #${PARTITION} of
+ device ${DEVICE} for the following reasons:
+ .
+ ${MESSAGE}
+
Template: partman/exception_handler
Type: select
Choices: ${CHOICES}
Index: packages/partman/partman-base/definitions.sh
===================================================================
--- packages/partman/partman-base/definitions.sh (revision 37887)
+++ packages/partman/partman-base/definitions.sh (working copy)
@@ -866,6 +866,67 @@
esac
}
+# Lock a device or partition against further modifications
+partman_lock_unit() {
+ local device message dev testdev
+ device="$1"
+ message="$2"
+
+ for dev in $DEVICES/*; do
+ [ -d "$dev" ] || continue
+ cd $dev
+
+ # First check if we should lock a device
+ if [ -e "device" ]; then
+ testdev=$(mapdevfs $(cat device))
+ if [ "$device" = "$testdev" ]; then
+ echo "$message" > locked
+ return 0
+ fi
+ fi
+
+ # Second check if we should lock a partition
+ open_dialog PARTITIONS
+ while { read_line num id size type fs path name; [ "$id" ]; }; do
+ testdev=$(mapdevfs $path)
+ if [ "$device" = "$testdev" ]; then
+ echo "$message" > $id/locked
+ fi
+ done
+ close_dialog
+ done
+}
+
+# Unlock a device or partition to allow further modifications
+partman_unlock_unit() {
+ local device dev testdev
+ device="$1"
+
+ for dev in $DEVICES/*; do
+ [ -d "$dev" ] || continue
+ cd $dev
+
+ # First check if we should unlock a device
+ if [ -e "device" ]; then
+ testdev=$(mapdevfs $(cat device))
+ if [ "$device" = "$testdev" ]; then
+ rm -f locked
+ return 0
+ fi
+ fi
+
+ # Second check if we should unlock a partition
+ open_dialog PARTITIONS
+ while { read_line num id size type fs path name; [ "$id" ]; }; do
+ testdev=$(mapdevfs $path)
+ if [ "$device" = "$testdev" ]; then
+ rm -f $id/locked
+ fi
+ done
+ close_dialog
+ done
+}
+
log '*******************************************************'
# Local Variables:
Index: packages/partman/partman-base/choose_partition/partition_tree/do_option
===================================================================
--- packages/partman/partman-base/choose_partition/partition_tree/do_option (revision 37887)
+++ packages/partman/partman-base/choose_partition/partition_tree/do_option (working copy)
@@ -8,7 +8,44 @@
id=${1#*//}
cd $dev
+device=$(humandev $(cat device))
+# If the user wants to modify a device or partition
+# the device may not be locked
+if [ -e "$dev/locked" ]; then
+ locked=$(cat "$dev/locked")
+ db_subst partman-base/devicelocked DEVICE "$device"
+ db_subst partman-base/devicelocked MESSAGE "$locked"
+ db_set partman-base/devicelocked "false"
+ db_input critical partman-base/devicelocked
+ db_go
+ exit 0
+fi
+
+# Two scenarios to check for here:
+# 1) If the user wants to modify a partition - it may not be locked
+# 2) If the user wants to modify a device - none of its partitions may be locked
+open_dialog PARTITIONS
+while { read_line num tmpid size type fs path name; [ "$tmpid" ]; }; do
+ if [ -n "$id" ]; then
+ [ "$id" = "$tmpid" ] || continue
+ fi
+
+ if [ -e "$dev/$tmpid/locked" ]; then
+ locked=$(cat "$dev/$tmpid/locked")
+ db_subst partman-base/partlocked DEVICE "$device"
+ db_subst partman-base/partlocked PARTITION "$num"
+ db_subst partman-base/partlocked MESSAGE "$locked"
+ db_set partman-base/partlocked "false"
+ db_input critical partman-base/partlocked
+ db_go
+ close_dialog
+ exit 0
+ fi
+done
+close_dialog
+
+
if [ -z "$id" ]; then
# ask_user /lib/partman/storage_device "$dev" "$id" || true
open_dialog GET_DISK_TYPE
Index: packages/partman/partman-lvm/debian/partman-lvm.templates
===================================================================
--- packages/partman/partman-lvm/debian/partman-lvm.templates (revision 37887)
+++ packages/partman/partman-lvm/debian/partman-lvm.templates (working copy)
@@ -27,6 +27,10 @@
# Translators: use the acronym for "Physical Volume" in your language here
_Description: PV
+Template: partman-lvm/text/in_use
+Type: text
+_Description: In use by LVM volume group ${VG}
+
Template: partman-lvm/menu/display
Type: text
# Menu entry
Index: packages/partman/partman-lvm/choose_partition/lvm/do_option
===================================================================
--- packages/partman/partman-lvm/choose_partition/lvm/do_option (revision 37887)
+++ packages/partman/partman-lvm/choose_partition/lvm/do_option (working copy)
@@ -65,6 +65,7 @@
db_go
db_get partman-lvm/activevg
[ "$RET" = "true" ] && log-output -t partman-lvm vgchange -a y
+ # TODO: We need to update the devices that LVM just claimed
fi
# ask only the first time
mkdir -p /var/cache/partman-lvm && touch /var/cache/partman-lvm/first
@@ -245,11 +246,17 @@
db_set partman-lvm/vgcreate_error "false"
db_input high partman-lvm/vgcreate_error
db_go
+ else
+ db_subst partman-lvm/text/in_use VG "$vg"
+ db_metaget partman-lvm/text/in_use description
+ for pv in $pvs; do
+ partman_lock_unit "$pv" "$RET"
+ done
fi
}
do_vg_delete() {
- local vgs vg output
+ local vgs vg output pvs
vgs=""
# Look for VGs with no LVs
@@ -287,10 +294,15 @@
db_get partman-lvm/vgdelete_confirm
[ "$RET" = "true" ] || return
+ pvs=$(vg_list_pvs "$vg")
if ! vg_delete "$vg"; then
db_set partman-lvm/vgdelete_error "false"
db_input high partman-lvm/vgdelete_error
db_go
+ else
+ for pv in $pvs; do
+ partman_unlock_unit "$pv"
+ done
fi
}
@@ -368,6 +380,10 @@
db_input high partman-lvm/vgextend_error
db_go
return
+ else
+ db_subst partman-lvm/text/in_use VG "$vg"
+ db_metaget partman-lvm/text/in_use description
+ partman_lock_unit "$pv" "$RET"
fi
done
}
@@ -454,6 +470,8 @@
db_input high partman-lvm/vgreduce_error
db_go
return
+ else
+ partman_unlock_unit "$pv"
fi
done
}
Reply to: