[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#254727: partman-auto: reusing existing partitions with same method



I've been looking at http://bugs.debian.org/254727 ("Creates swap even
when there is already one"), and also at
https://bugs.launchpad.net/bugs/746313 ("partman should reuse existing
BIOS Boot Partition") which has the same cause but has more serious
consequences.

In general, I'm aware that partman doesn't have very good support for
reusing existing partitions.  Much of the problem is in defining good
recipe syntax for it: how do you name the partition you want to reuse?
Partition numbers may be too specific for some users, not every
partition has a UUID, etc.  However, in this case, I think we could
define a simple abstraction: "if there's already a partition with this
method, I want to reuse it", and encode that as a $reusemethod{ }
internal specifier in partman recipes.

The implementation is a little tricky.  You have to exclude reused
partitions from size calculations, but you also have to perform an
action upon applying the recipe.  I found that it was easiest to
transform $reusemethod{ } into $reuse{ ID } when decoding the recipe,
and then deal with that when performing the recipe.

I've pushed an initial implementation of this here:

  http://git.debian.org/?p=d-i/partman-auto.git;a=shortlog;h=refs/heads/people/cjwatson/reusemethod

and the diff against master is attached.  (Of course, we'd also need to
add $reusemethod{ } to a bunch of recipe stanzas.)  What do people
think of this?

-- 
Colin Watson                                       [cjwatson@ubuntu.com]
diff --git a/debian/changelog b/debian/changelog
index ecc8560..9500786 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,6 +1,10 @@
 partman-auto (98) UNRELEASED; urgency=low
 
   * Update old comment about partman-auto-lvm and $iflabel.
+  * Refactor some of decode_recipe to make it easier to have multiple ways
+    to ignore recipe stanzas.
+  * Add $reusemethod internal specifier, which excludes the partition if a
+    partition with the same method already exists.
 
  -- Colin Watson <cjwatson@debian.org>  Wed, 20 Apr 2011 11:09:22 +0100
 
diff --git a/lib/auto-shared.sh b/lib/auto-shared.sh
index 3d9e66a..72573b5 100644
--- a/lib/auto-shared.sh
+++ b/lib/auto-shared.sh
@@ -92,9 +92,22 @@ ensure_primary() {
 	)"
 }
 
-create_primary_partitions() {
+reuse_partitions() {
 	cd $dev
+	local scheme
+
+	scheme="$scheme_reused"
+	foreach_partition '
+		id="$(echo " $*" | sed -n '\''s/.* \$reuse{ \([^}]*\) }.*/\1/p'\'')"
+		if [ -z "$id" ] || [ ! -d "$id" ]; then
+			db_progress STOP
+			autopartitioning_failed
+		fi
+		setup_partition $id $*'
+}
 
+create_primary_partitions() {
+	cd $dev
 	while [ "$free_type" = pri/log ] && \
 	      echo $scheme | grep -q '\$primary{'; do
 		pull_primary
diff --git a/lib/recipes.sh b/lib/recipes.sh
index 396f6d9..016eb58 100644
--- a/lib/recipes.sh
+++ b/lib/recipes.sh
@@ -24,10 +24,26 @@ autopartitioning_failed () {
 	exit 1
 }
 
+find_method () {
+	local num id size type fs path name method found
+	found=
+	open_dialog PARTITIONS
+	while { read_line num id size type fs path name; [ "$id" ]; }; do
+		[ -f $id/method-old ] || continue
+		method="$(cat $id/method-old)"
+		if [ "$method" = "$1" ]; then
+			found="$id"
+		fi
+	done
+	close_dialog
+	echo "$found"
+}
+
 unnamed=0
 
 decode_recipe () {
 	local ignore ram line word min factor max fs iflabel label -
+	local reusemethod method id
 	ignore="${2:+${2}ignore}"
 	unnamed=$(($unnamed + 1))
 	ram=$(grep ^Mem: /proc/meminfo | { read x y z; echo $y; }) # in bytes
@@ -109,30 +125,45 @@ decode_recipe () {
 
 			# Exclude partitions that have ...ignore set
 			if [ "$ignore" ] && [ "$(echo $line | grep "$ignore")" ]; then
-				:
-			else
-				# Exclude partitions that are only for a
-				# different disk label.  The $PWD check
-				# avoids problems when running from older
-				# versions of partman-auto-lvm, where we
-				# weren't in a subdirectory of $DEVICES
-				# while decoding the recipe; we preserve it
-				# in case of custom code with the same
-				# problem.
-				iflabel="$(echo $line | sed -n 's/.*\$iflabel{ \([^}]*\) }.*/\1/p')"
-				if [ "$iflabel" ]; then
-					if [ "${PWD#$DEVICES/}" != "$PWD" ]; then
-						open_dialog GET_LABEL_TYPE
-						read_line label
-						close_dialog
-						if [ "$iflabel" = "$label" ]; then
-							scheme="${scheme:+$scheme$NL}$line"
-						fi
+				line=
+				continue
+			fi
+
+			# Exclude partitions that are only for a different
+			# disk label.  The $PWD check avoids problems when
+			# running from older versions of partman-auto-lvm,
+			# where we weren't in a subdirectory of $DEVICES
+			# while decoding the recipe; we preserve it in case
+			# of custom code with the same problem.
+			iflabel="$(echo $line | sed -n 's/.*\$iflabel{ \([^}]*\) }.*/\1/p')"
+			if [ "$iflabel" ]; then
+				if [ "${PWD#$DEVICES/}" = "$PWD" ]; then
+					line=''
+					continue
+				fi
+
+				open_dialog GET_LABEL_TYPE
+				read_line label
+				close_dialog
+				if [ "$iflabel" != "$label" ]; then
+					line=''
+					continue
+				fi
+			fi
+
+			# Exclude partitions where we can reuse an existing
+			# partition instead.
+			if echo "$line" | grep -q '\$reusemethod{'; then
+				if [ "${PWD#$DEVICES/}" != "$PWD" ]; then
+					method="$(echo "$line" | sed -n 's/.* method{ \([^}]*\) }.*/\1/p')"
+					id="$(find_method "$method")"
+					if [ "$id" ]; then
+						line="$(echo "$line" | sed 's/\$reusemethod{[^}]*}/$reuse{ '"$id"' }/')"
 					fi
-				else
-					scheme="${scheme:+$scheme$NL}$line"
 				fi
 			fi
+
+			scheme="${scheme:+$scheme$NL}$line"
 			line=''
 			;;
 		    *)
@@ -366,6 +397,21 @@ choose_recipe () {
 }
 
 expand_scheme() {
+	# Filter out reused partitions first, as we don't want to take
+	# account of their size.
+	scheme_reused=$(
+	    foreach_partition '
+		if echo "$*" | grep -q '\''\$reuse{'\''; then
+			echo "$*"
+		fi'
+	)
+	scheme=$(
+	    foreach_partition '
+		if ! echo "$*" | grep -q '\''\$reuse{'\''; then
+			echo "$*"
+		fi'
+	)
+
 	# Make factors small numbers so we can multiply on them.
 	# Also ensure that fact, max and fs are valid
 	# (Ofcourse in valid recipes they must be valid.)
@@ -432,7 +478,8 @@ clean_method() {
 		cd $device
 		open_dialog PARTITIONS
 		while { read_line num id size type fs path name; [ "$id" ]; }; do
-			rm -f $id/method
+			[ -e $id/method ] || continue
+			mv $id/method $id/method-old
 		done
 		close_dialog
 	done
diff --git a/perform_recipe b/perform_recipe
index b48c739..89b09e1 100755
--- a/perform_recipe
+++ b/perform_recipe
@@ -35,6 +35,8 @@ expand_scheme
 
 ensure_primary
 
+reuse_partitions
+
 db_progress STEP 1
 
 create_primary_partitions

Reply to: