Or day 4, depending on how you want to count... The set of changes just committed finish off the reorganization I had in mind before the reimplementation of support for erasing crypto volumes. All LVM-specific code has now been moved out of partman-auto and into partman-lvm. Expanding on some of Jérémy's ideas in the patch he proposed in #396023, some code duplication when creating a new disk label has been removed and we now "clean up" existing LVM data also when a new disk label is created during manual partitioning. I've tested the changes again and have not seen any regressions. As current SVN has only minor changes in functionality when compared to the versions now in unstable, I'd suggest we do another round of uploads for at least the major components before we start on reimplementation of support for erasing crypto volumes. If anybody wants to do some additional testing before that, that would be most welcome. The main changes are: - dynamic loading of LVM and crypto support based on memory available - also removing LVM data for manual disk label creation - one template was revised (see: partman-lvm/device_remove_lvm_span) I have done some work on somewhat more invasive patches, but in the end decided to keep it simple. Main reason is that I do not yet clearly see how erasing crypto volumes can best be implemented. Some more reorganization will definitely be needed. The attached patch (which was intended _instead_ of r50396) should give some idea of the direction I've been thinking in. But that was without really looking at the previous work by David and Jérémy for crypto. Here are some of my thoughts: - lvm/crypto specific code should be kept out of base components as much as possible - the only case where we need to remove crypto volumes is when they are _already activated_ and thus when we already have full crypto support loaded - when we don't have crypto volumes active, just formatting the parent device will make them invalid (I've just tested this for Luks), so they cannot affect future installations (like LVM _can_ do) - we should add a friendly option to scan for and activate existing LVM and crypto volumes sometime, but that is a separate issue - this means that during normal guided partitioning erasing encrypted volumes will never be an issue; the only time it can happen is when a user first does encrypted partitioning and later, in the same session, decides to undo everything; this is also the use case in the open BRs - this also basically means that the user will already have seen the current layout in the manual partitioning screen; this makes me wonder how elaborate the erase function for crypto needs to be - a better display of what exactly will be erased for LVM would be very useful, maybe this should be the first step in the reimplementation - should we also support targeted removal of a specific encrypted volume, e.g. when the parent device is selected? seems worth doing and it should be possible to use largely the same code for that, but it is only useful if we can unlock the parent device after doing so - preferably, the option to remove LVM data should not need dmsetup and its code should not be duplicated for combined crypto/LVM erasing - the code should dynamically detect whether support for LVM and/or crypto is available; if neither is available, we just assume we can proceed by just creating a new disk label - preferably, we should first check (recursively) if all is clear for erasing data before we actually erase anything and we should not ask for multiple confirmations The last point is probably the one that could make the reimplementation complex and is basically where I got stuck. Cheers, FJP
commit fc4cd62e2bafb83ae32dcb7617ec4598cf208450
Author: Frans Pop <fjp@debian.org>
Date: Sat Dec 8 19:41:58 2007 +0100
More generic support for removing device mapper volumes
diff --git a/packages/partman/partman-auto/lib/auto-shared.sh b/packages/partman/partman-auto/lib/auto-shared.sh
index f7f8955..559a709 100644
--- a/packages/partman/partman-auto/lib/auto-shared.sh
+++ b/packages/partman/partman-auto/lib/auto-shared.sh
@@ -4,13 +4,10 @@ auto_init_disk() {
local dev
dev="$1"
- if [ -e /lib/partman/lib/lvm-remove.sh ]; then
- . /lib/partman/lib/lvm-remove.sh
- device_remove_lvm "$dev" || return 1
- fi
-
- # Create new disk label; don't prompt for label
. /lib/partman/lib/disk-label.sh
+ # First remove any device mapper volumes present on the device
+ device_dm_remove "$dev" || return 1
+ # Create new disk label; don't prompt for label
create_new_label "$dev" no || return 1
cd $dev
diff --git a/packages/partman/partman-lvm/lib/lvm-remove.sh b/packages/partman/partman-lvm/lib/lvm-remove.sh
index 19e2ef9..bd8402c 100644
--- a/packages/partman/partman-lvm/lib/lvm-remove.sh
+++ b/packages/partman/partman-lvm/lib/lvm-remove.sh
@@ -10,7 +10,7 @@ device_remove_lvm() {
# Check if the device already contains any physical volumes
realdev=$(mapdevfs "$(cat $dev/device)")
if ! pv_on_device "$realdev"; then
- return 0
+ return 9
fi
# Ask for permission to erase LVM volumes
@@ -55,26 +55,5 @@ device_remove_lvm() {
done
done
- # Make sure that parted has no stale LVM info
- restart=""
- for tmpdev in $DEVICES/*; do
- [ -d "$tmpdev" ] || continue
-
- realdev=$(cat $tmpdev/device)
-
- if [ -b "$realdev" ] || \
- ! $(echo "$realdev" | grep -q "/dev/mapper/"); then
- continue
- fi
-
- rm -rf $tmpdev
- restart=1
- done
-
- if [ "$restart" ]; then
- stop_parted_server
- restart_partman || return 1
- fi
-
return 0
}
diff --git a/packages/partman/partman-partitioning/lib/disk-label.sh b/packages/partman/partman-partitioning/lib/disk-label.sh
index 6475e78..6bdd948 100644
--- a/packages/partman/partman-partitioning/lib/disk-label.sh
+++ b/packages/partman/partman-partitioning/lib/disk-label.sh
@@ -152,6 +152,65 @@ default_disk_label () {
esac
}
+# Unset when sourced
+unset DM_HAVE_LVM
+
+# Allow subfunctions to call LVM removal without needing to source
+# lvm-remove.sh themselves
+lvm_supported() {
+ if [ -n "$DM_HAVE_LVM" ]; then
+ if [ -e /lib/partman/lib/lvm-remove.sh ]; then
+ . /lib/partman/lib/lvm-remove.sh
+ DM_HAVE_LVM=1
+ else
+ return 1
+ fi
+ fi
+ return 0
+}
+
+device_dm_remove() {
+ local dev ret dm_changed restart tmpdev
+ dev="$1"
+
+ dm_changed=""
+ if lvm_supported; then
+ ret=0
+ device_lvm_remove $dev || ret=$?
+ case $ret in
+ 0) dm_changed=1 ;;
+ 9) : ;;
+ *) return 1 ;;
+ esac
+ fi
+
+ if [ -z "$dm_changed" ]; then
+ return 0
+ fi
+
+ # Make sure that parted has no stale device mapper info
+ restart=""
+ for tmpdev in $DEVICES/*; do
+ [ -d "$tmpdev" ] || continue
+
+ realdev=$(cat $tmpdev/device)
+ if [ -b "$realdev" ] || \
+ ! $(echo "$realdev" | grep -q "/dev/mapper/"); then
+ continue
+ fi
+
+ rm -rf $tmpdev
+ restart=1
+ done
+
+ if [ "$restart" ]; then
+ stop_parted_server
+ restart_partman || return 1
+ fi
+
+ return 0
+}
+
create_new_label() {
local dev default_type chosen_type types
dev="$1"
diff --git a/packages/partman/partman-partitioning/storage_device/label/do_option b/packages/partman/partman-partitioning/storage_device/label/do_option
index eb6d5cf..d508583 100755
--- a/packages/partman/partman-partitioning/storage_device/label/do_option
+++ b/packages/partman/partman-partitioning/storage_device/label/do_option
@@ -17,6 +17,8 @@ if [ "$RET" = false ]; then
fi
db_reset partman-partitioning/confirm_new_label
+# First remove any device mapper volumes present on the device
+device_dm_remove "$dev" || exit 1
create_new_label "$dev" ""
partitions=''
Attachment:
signature.asc
Description: This is a digitally signed message part.