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

2011-04-20 Thread Colin Watson
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   [cjwat...@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 cjwat...@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 

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

2011-04-20 Thread Lennart Sorensen
On Wed, Apr 20, 2011 at 05:18:52PM +0100, Colin Watson wrote:
 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?

What if the other OS using a swap partition is currently suspended
to disk?  Can you share the swap partition in that case?  Sounds rather
unsafe in some cases.

Of course this doesn't mean you can't let people share them, but doing
it by default would seem hazardous.

-- 
Len Sorensen


-- 
To UNSUBSCRIBE, email to debian-boot-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: 
http://lists.debian.org/20110420171215.gr...@caffeine.csclub.uwaterloo.ca