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

[RFC] Some love for partman-md



Hi!

After loosing 2 hours yesterday fighting against the RAIDvsCrypto
issue [1], I decided that I should not let this happen to anyone else.

Attached you will find an attempt to do so.  The changeset is only broke
down in two patches:
 * The first is a bit invasive (partman-md, partman-base, mdcfg) and
   improve globally the way MD devices are initialized and from my tests
   fix 10 bugs at ounce (counting merged ones).
 * The second is in partman-partitioning and prevent deletion and
   resizing for MD, LVM and crypto devices as they do not work correctly
   for any of these devices.

I have done quite some testing inside partman and I think they should be
right.  Unfortunately, my offline mirror was broken and I was not able
to do any full installations.  As I'll be very happy to have such fixes
in Lenny, I would very much welcome reviews and tests.

[1] http://wiki.debian.org/DebianInstaller/RAIDvsCrypto

Cheers,
-- 
Jérémy Bobbio                        .''`. 
lunar@debian.org                    : :Ⓐ  :  # apt-get install anarchism
                                    `. `'` 
                                      `-   
commit e5f830e53c38b4d214326c8a33db556ac3ede504
Author: Jérémy Bobbio <lunar@debian.org>
Date:   Thu May 29 16:12:16 2008 +0000

    Clean up the initialization of MD devices
    
    The initialization of MD devices in partman previously diverged from what
    others partman components are doing, resulting in quite few bugs and
    really bad interactions with crypto setup.
    
     * Scanning for existing MD devices
    
       The loading of kernel modules and the initial scan for MD devices has
       been factor out in the md-init script, shipped in both mdcfg and
       mdcfg-utils packages.
    
       This script is called by both the init.d/md-devices in partman-md and
       by mdcfg itself.  init.d/md-devices is now called before init.d/parted
       to have existing RAID partitions listed on the first partitioner run.
    
       Existing MD devices will now be activated at the begining of the
       partionner.  (Closes: #391474)
    
     * Keep configurations of RAID partitions accross partman restarts
    
       MD devices were previously skipped in init.d/parted, resulting in the
       loss of the previous configuration.  We now only skip deactivated RAID
       devices, in a similar way than sataraid and multipath devices are
       skipped.
    
       Most of the job of init.d/md and init.d/md-devices have been cleaned up
       and merged into init.d/md to cope with the previous change. The
       initialization scheme is now closer to what is done for LVM.
    
       (Closes: #391479, #391483, #393728, #398668)

diff --git a/packages/mdcfg/debian/changelog b/packages/mdcfg/debian/changelog
index 2543ba2..0df294d 100644
--- a/packages/mdcfg/debian/changelog
+++ b/packages/mdcfg/debian/changelog
@@ -1,3 +1,12 @@
+mdcfg (1.27) UNRELEASED; urgency=low
+
+  [ Jérémy Bobbio ]
+  * Add an md-init script to mdcfg-utils, responsible for loading modules and
+    doing the initial scan for MD devices.  This script is run by both mdcfg
+    and partman-md.
+
+ -- Jérémy Bobbio <lunar@debian.org>  Thu, 29 May 2008 17:46:31 +0000
+
 mdcfg (1.26) unstable; urgency=low
 
   [ Updated translations ]
diff --git a/packages/mdcfg/debian/dirs b/packages/mdcfg/debian/dirs
index 452a963..1e73c8b 100644
--- a/packages/mdcfg/debian/dirs
+++ b/packages/mdcfg/debian/dirs
@@ -1 +1,2 @@
+bin
 var/lib/partconf/block.d
diff --git a/packages/mdcfg/debian/rules b/packages/mdcfg/debian/rules
index 0712257..c57a981 100755
--- a/packages/mdcfg/debian/rules
+++ b/packages/mdcfg/debian/rules
@@ -27,6 +27,8 @@ install: build
 	dh_installdirs
 	install -m755 mdcfg.sh debian/mdcfg.postinst
 	install -m755 mdcfg.sh debian/mdcfg-utils/bin/mdcfg
+	install -m755 md-init.sh debian/mdcfg-utils/bin/md-init
+	install -m755 md-init.sh debian/mdcfg/bin/md-init
 	install -m755 partconf-hook.sh debian/mdcfg/var/lib/partconf/block.d/mdcfg.sh
 	install -m755 finish-install debian/mdcfg-utils/usr/lib/finish-install.d/65mdcfg
 
diff --git a/packages/mdcfg/md-init.sh b/packages/mdcfg/md-init.sh
new file mode 100755
index 0000000..53645f8
--- /dev/null
+++ b/packages/mdcfg/md-init.sh
@@ -0,0 +1,24 @@
+#!/bin/sh
+
+if [ -e /proc/mdstat ]; then
+	exit 0
+fi
+
+# Try to load the necesarry modules.
+# Supported schemes: RAID 0, RAID 1, RAID 5
+depmod -a >/dev/null 2>&1
+modprobe md >/dev/null 2>&1 || modprobe md-mod >/dev/null 2>&1
+modprobe raid0 >/dev/null 2>&1
+modprobe raid1 >/dev/null 2>&1
+# kernels >=2.6.18 have raid456
+modprobe raid456 >/dev/null 2>&1 || modprobe raid5 >/dev/null 2>&1
+
+# Try to detect MD devices, and start them
+# mdadm will fail if /dev/md does not already exist
+mkdir -p /dev/md
+
+log-output -t md-init --pass-stdout \
+	mdadm --examine --scan --config=partitions >/tmp/mdadm.conf
+
+log-output -t md-init \
+	mdadm --assemble --scan --run --config=/tmp/mdadm.conf --auto=yes
diff --git a/packages/mdcfg/mdcfg.sh b/packages/mdcfg/mdcfg.sh
index 37a1f09..51daf9c 100755
--- a/packages/mdcfg/mdcfg.sh
+++ b/packages/mdcfg/mdcfg.sh
@@ -502,24 +502,8 @@ md_mainmenu() {
 
 ### Main of script ###
 
-# Try to load the necesarry modules.
-# Supported schemes: RAID 0, RAID 1, RAID 5
-depmod -a >/dev/null 2>&1
-modprobe md >/dev/null 2>&1 || modprobe md-mod >/dev/null 2>&1
-modprobe raid0 >/dev/null 2>&1
-modprobe raid1 >/dev/null 2>&1
-# kernels >=2.6.18 have raid456
-modprobe raid456 >/dev/null 2>&1 || modprobe raid5 >/dev/null 2>&1
-
-# Try to detect MD devices, and start them
-# mdadm will fail if /dev/md does not already exist
-mkdir -p /dev/md
-
-log-output -t mdcfg --pass-stdout \
-	mdadm --examine --scan --config=partitions >/tmp/mdadm.conf
-
-log-output -t mdcfg \
-	mdadm --assemble --scan --run --config=/tmp/mdadm.conf --auto=yes
+# Load the modules and scan for MD devices if needed
+md-init
 
 # Make sure that we have md-support
 if [ ! -e /proc/mdstat ]; then
diff --git a/packages/partman/partman-base/debian/changelog b/packages/partman/partman-base/debian/changelog
index 2d63730..31b93af 100644
--- a/packages/partman/partman-base/debian/changelog
+++ b/packages/partman/partman-base/debian/changelog
@@ -1,3 +1,11 @@
+partman-base (121) UNRELEASED; urgency=low
+
+  [ Jérémy Bobbio ]
+  * Instead of skipping every MD devices during partman initialization, we
+    know only skip the ones that are deactivated.
+
+ -- Jérémy Bobbio <lunar@debian.org>  Thu, 29 May 2008 16:07:37 +0000
+
 partman-base (120) unstable; urgency=high
 
   [ Jérémy Bobbio ]
diff --git a/packages/partman/partman-base/init.d/parted b/packages/partman/partman-base/init.d/parted
index 346170a..f7cfe30 100755
--- a/packages/partman/partman-base/init.d/parted
+++ b/packages/partman/partman-base/init.d/parted
@@ -6,6 +6,15 @@ set -e
 
 ORIG_IFS="$IFS"
 
+is_deactivated_md() {
+	local number
+	number=$(echo "$1" | sed -n -e 's,/dev/md/\?,,p')
+	if [ "$number" ] && ! grep -q "^md$number :" /proc/mdstat; then
+		return 0
+	fi
+	return 1
+}
+
 part_of_sataraid () {
 	local raiddev
 	for raiddev in $(dmraid -r -c); do
@@ -52,8 +61,7 @@ if [ ! -f /var/run/parted_server.pid ]; then
 
 	IFS="$NL"
 	for partdev in $(parted_devices |
-		grep -v '^/dev/md' |
-		sed 's,^/dev/\(ide\|scsi\|[hs]d\),!/dev/\1,' |
+		sed 's,^/dev/\(ide\|scsi\|[hsm]d\),!/dev/\1,' |
 		sort |
 		sed 's,^!,,' ); do
 
@@ -65,6 +73,13 @@ if [ ! -f /var/run/parted_server.pid ]; then
 		size=$2
 		model=$3
 
+		# Skip deactivated MD devices
+		if [ -e /proc/mdstat ]; then
+			if is_deactivated_md $device; then
+				continue
+			fi
+		fi
+
 		# Skip devices that are part of a dmraid device
 		if type dmraid >/dev/null 2>&1; then
 			if part_of_sataraid $device; then
diff --git a/packages/partman/partman-md/debian/changelog b/packages/partman/partman-md/debian/changelog
index e93fc37..384d64a 100644
--- a/packages/partman/partman-md/debian/changelog
+++ b/packages/partman/partman-md/debian/changelog
@@ -1,3 +1,16 @@
+partman-md (42) UNRELEASED; urgency=low
+
+  [ Jérémy Bobbio ]
+  * Clean up the initialization of MD devices.  Together with the changes
+    introduced in partman-base (>> 120), setup of RAID devices won't be lost
+    accross partman restarts anymore.
+    (Closes: #391479, #391483, #393728, #398668)
+  * Load the necassary modules and scan RAID arrays during partman
+    initialization.  (Closes: #391474)
+    Requires mdcfg-utils (>> 1.26).
+
+ -- Jérémy Bobbio <lunar@debian.org>  Thu, 29 May 2008 16:02:39 +0000
+
 partman-md (41) unstable; urgency=low
 
   [ Frans Pop ]
diff --git a/packages/partman/partman-md/debian/control b/packages/partman/partman-md/debian/control
index 08c74b5..77a906c 100644
--- a/packages/partman/partman-md/debian/control
+++ b/packages/partman/partman-md/debian/control
@@ -9,5 +9,5 @@ Vcs-Svn: svn://svn.debian.org/d-i/trunk/packages/partman/partman-md
 Package: partman-md
 XC-Package-Type: udeb
 Architecture: all
-Depends: ${misc:Depends}, mdadm-udeb, mdcfg-utils, partman-base (>= 114)
+Depends: ${misc:Depends}, mdadm-udeb, mdcfg-utils (>> 1.26), partman-base (>> 120)
 Description: Add to partman support for MD
diff --git a/packages/partman/partman-md/init.d/_numbers b/packages/partman/partman-md/init.d/_numbers
index 75ae141..b5155f7 100644
--- a/packages/partman/partman-md/init.d/_numbers
+++ b/packages/partman/partman-md/init.d/_numbers
@@ -1,2 +1,2 @@
-31 md-devices
+25 md-devices
 51 md
diff --git a/packages/partman/partman-md/init.d/md b/packages/partman/partman-md/init.d/md
index e5024f2..96b0b52 100755
--- a/packages/partman/partman-md/init.d/md
+++ b/packages/partman/partman-md/init.d/md
@@ -2,16 +2,25 @@
 
 . /lib/partman/lib/base.sh
 
-# Load modules
-#depmod -a 1>/dev/null 2>&1
-#modprobe md 1>/dev/null 2>&1 || modprobe md-mod 1>/dev/null 2>&1
-#
-## Load supported personalities
-#modprobe raid1 1>/dev/null 2>&1
-#
-## Detect and start MD devices
-#/sbin/mdadm --examine --scan --config=partitions >/tmp/mdadm.conf
-#/sbin/mdadm --assemble --scan --run --config=/tmp/mdadm.conf --auto=yes
+# Check if we have RAID
+if [ ! -f /proc/mdstat ]; then
+	exit 0
+fi
+
+# Obtain the size of an MD device
+get_size () {
+	while [ -z "$(echo "$line" | grep "^md$NUMBER :")" ]; do
+		read line
+		[ $? -eq 1 ] && break  # EOF
+	done
+	read line
+	size=$(echo "$line" | sed -e 's/ blocks.*//')
+	# If the sed failed, the line didn't contain the size; just
+	# return 0 in that case.
+	if [ "$size" = "$line" ]; then
+		size=0
+	fi
+}
 
 # Mark all RAID partitions as being MD
 for dev in $DEVICES/*; do
@@ -28,6 +37,7 @@ for dev in $DEVICES/*; do
 	done
 	close_dialog
 
+	# Check if device/partitions are used for software RAID
 	for id in $partitions; do
 		md=no
 		open_dialog GET_FLAGS $id
@@ -43,38 +53,57 @@ for dev in $DEVICES/*; do
 		fi
 	done
 
-	# Next, check if the device is a part of an MD setup
-#	if [ -f device ]; then
-#		DEVICE=`sed -e "s/\/dev\///" device`
-#		grep -q "${DEVICE}" /proc/mdstat
-#		if [ $? -eq 0 ]; then
-#			open_dialog NEW_LABEL loop
-#			close_dialog
-#		fi
-#	fi
-done
+	# Check if the device is a part of an MD setup
+	if [ -f device ]; then
+		NUMBER=$(sed -e 's,/dev/md/\?,,' device)
+		if ! grep -q "^md$NUMBER :" /proc/mdstat; then
+			continue
+		fi
 
-# Create the raid devices
-# NOTE: /dev/md/X does not necessarily exist; /dev/mdX can exist alone
-#       (see #470374 for reference)
-#for i in $(grep md /proc/mdstat | \
-#	    sed -e 's/^\(md.*\) : active \([[:alnum:]]*\).*/\1/'); do
-#	NUMBER=$(echo $i | sed -e "s/^md//")
-#	DEVICE="$DEVICES/=dev=md$NUMBER"
-#	mkdir -p $DEVICE
-#	cd $DEVICE
-#	echo "/dev/md$NUMBER" > $DEVICE/device
-#	echo Unknown > $DEVICE/model
-#	echo "0" > $DEVICE/size
-#	open_dialog OPEN "/dev/md$NUMBER"
-#	read_line response
-#	close_dialog
-#	if [ "$response" = failed ]; then
-#		rm -rf $DEVICE
-#	fi
-#
-#	open_dialog NEW_LABEL loop
-#	close_dialog
-#done
+		# replace "Unknown" model by a more appropriate string
+		db_metaget partman-md/text/device description
+		echo "${RET}" > model
+
+		# fix size
+		get_size < /proc/mdstat
+		sector_size=$(grep " sectors" /proc/mdstat | sed -e 's/[^0-9]\+//g')
+		if [ -z "$sector_size" ]; then
+			sector_size=1024
+		fi
+		size=$(($size * $sector_size))
+		echo "$size" > size
+
+		# create a label
+		open_dialog NEW_LABEL loop
+		close_dialog
+		# find the free space
+		open_dialog PARTITIONS
+		free_space=''
+		while { read_line num id size type fs path name; [ "$id" ]; }; do
+			if [ "$fs" = free ]; then
+				free_space=$id
+				free_size=$size
+			fi
+		done
+		close_dialog
+		# create partition in the free space
+		if [ "$free_space" ]; then
+			open_dialog NEW_PARTITION primary ext2 $free_space full $free_size
+			read_line num id size type fs path name
+			close_dialog
+			if [ "$id" ]; then
+				open_dialog GET_FILE_SYSTEM $id
+				read_line filesystem
+				close_dialog
+				if [ "$filesystem" != none ]; then
+					open_dialog CHANGE_FILE_SYSTEM $id $filesystem
+					close_dialog
+				fi
+			fi
+		fi
+		open_dialog DISK_UNCHANGED
+		close_dialog
+	fi
+done
 
 exit 0
diff --git a/packages/partman/partman-md/init.d/md-devices b/packages/partman/partman-md/init.d/md-devices
index df9c785..4c00470 100755
--- a/packages/partman/partman-md/init.d/md-devices
+++ b/packages/partman/partman-md/init.d/md-devices
@@ -2,94 +2,7 @@
 
 . /lib/partman/lib/base.sh
 
-# Check if we have RAID
-if [ ! -f /proc/mdstat ]; then
-	exit 0
-fi
-
-# Obtain the size of an MD device
-get_size () {
-	while [ -z "$(echo "$line" | grep "^md$NUMBER :")" ]; do
-		read line
-		[ $? -eq 1 ] && break  # EOF
-	done
-	read line
-	size=$(echo "$line" | sed -e 's/ blocks.*//')
-	# If the sed failed, the line didn't contain the size; just
-	# return 0 in that case.
-	if [ "$size" = "$line" ]; then
-		size=0
-	fi
-}
-
-# Create the raid devices
-for i in $(grep ^md /proc/mdstat | grep -v inactive | \
-	   sed -e 's/^\(md.*\) : active \([[:alnum:]]*\).*/\1/'); do
-	NUMBER="${i#md}"
-	if [ -e "/dev/md/${NUMBER}" ]; then
-		DEVICE="$DEVICES/=dev=md=${NUMBER}"
-		DEVNODE="/dev/md/${NUMBER}"
-	else
-		DEVICE="$DEVICES/=dev=md${NUMBER}"
-		DEVNODE="/dev/md${NUMBER}"
-	fi
-	mkdir -p ${DEVICE}
-	cd ${DEVICE}
-	echo "$DEVNODE" > ${DEVICE}/device
-
-	db_metaget partman-md/text/device description
-	echo "${RET}" > ${DEVICE}/model
-
-	get_size < /proc/mdstat
-	sector_size=$(grep " sectors" /proc/mdstat | sed -e 's/[^0-9]\+//g')
-	if [ -z "$sector_size" ]; then
-		sector_size=1024
-	fi
-	size=$(($size * $sector_size))
-	echo "$size" > ${DEVICE}/size
-
-	open_dialog OPENED "$DEVNODE"
-	read_line response
-	close_dialog
-	if [ "$response" = no ]; then
-		open_dialog OPEN "$DEVNODE"
-		read_line response
-		close_dialog
-		if [ "$response" = failed ]; then
-			rm -rf ${DEVICE}
-		fi
-	fi
-
-	open_dialog NEW_LABEL loop
-	close_dialog
-
-	open_dialog PARTITIONS
-	free_space=''
-	while { read_line num id size type fs path name; [ "$id" ]; }; do
-		if [ "$fs" = free ]; then
-			free_space=$id
-			free_size=$size
-		fi
-	done
-	close_dialog
-
-	if [ "$free_space" ]; then
-		open_dialog NEW_PARTITION primary ext2 $free_space full $free_size
-		read_line num id size type fs path name
-		close_dialog
-		if [ "$id" ]; then
-			open_dialog GET_FILE_SYSTEM $id
-			read_line filesystem
-			close_dialog
-			if [ "$filesystem" != none ]; then
-				open_dialog CHANGE_FILE_SYSTEM $id $filesystem
-				close_dialog
-			fi
-		fi
-	fi
-
-	open_dialog DISK_UNCHANGED
-	close_dialog
-done
+# Load modules and scan arrays if needed
+md-init
 
 exit 0

commit 922e996d3dfa9f545b2e4426d79a5bdcb588fd64
Author: Jérémy Bobbio <lunar@debian.org>
Date:   Thu May 29 23:08:16 2008 +0000

    Disable "resize" and "delete" for partitions on "loop"
    
    For RAID, crypto and LVM devices only one partition can be handled at a
    time.  As it makes no sense to resize or delete these kind of partitions,
    let's hide the options instead of tricking the users.
    
    As all these devices are using "loop" partition tables, that's our
    filtering criteria here.
    
    (Closes: #318228, #417973)

diff --git a/packages/partman/partman-partitioning/active_partition/delete/choices b/packages/partman/partman-partitioning/active_partition/delete/choices
index 6587145..5935d7c 100755
--- a/packages/partman/partman-partitioning/active_partition/delete/choices
+++ b/packages/partman/partman-partitioning/active_partition/delete/choices
@@ -1,7 +1,19 @@
 #!/bin/sh
 
-. /usr/share/debconf/confmodule
+. /lib/partman/lib/base.sh
+
+dev=$1
+id=$2
+cd $dev
+
+open_dialog GET_LABEL_TYPE
+read_line label
+close_dialog
+
+# Disable on devices where there is no "real" partitioning
+if [ "$label" = loop ]; then
+	exit 0
+fi
 
 db_metaget partman-partitioning/text/delete description
 printf "delete\t${RET}\n"
-
diff --git a/packages/partman/partman-partitioning/active_partition/resize/choices b/packages/partman/partman-partitioning/active_partition/resize/choices
index 3b25d94..ef53072 100755
--- a/packages/partman/partman-partitioning/active_partition/resize/choices
+++ b/packages/partman/partman-partitioning/active_partition/resize/choices
@@ -6,6 +6,15 @@ dev=$1
 id=$2
 cd $dev
 
+open_dialog GET_LABEL_TYPE
+read_line label
+close_dialog
+
+# Disable on devices where there is no "real" partitioning
+if [ "$label" = loop ]; then
+	exit 0
+fi
+
 if [ -f $id/detected_filesystem ]; then
 	fs=$(cat $id/detected_filesystem)
 	case "$fs" in
diff --git a/packages/partman/partman-partitioning/debian/changelog b/packages/partman/partman-partitioning/debian/changelog
index 27ac4b8..7da50d7 100644
--- a/packages/partman/partman-partitioning/debian/changelog
+++ b/packages/partman/partman-partitioning/debian/changelog
@@ -1,5 +1,6 @@
 partman-partitioning (60) UNRELEASED; urgency=low
 
+  [ Colin Watson ]
   * Silence warning in case $id/options already exists.
   * Move 'local' down a line in create_new_partition in order to work
     properly with bash, which initialises local variables without an
@@ -9,6 +10,11 @@ partman-partitioning (60) UNRELEASED; urgency=low
   * Use explicit dummy variables in create_new_partition rather than
     overriding $type, $size, et al.
 
+  [ Jérémy Bobbio ]
+  * Disable "resize" and "delete" for partitions on "loop" partition tables.
+    This will prevent unusable behaviour on RAID, crypto and LVM partitions.
+    (Closes: #318228, #417973)
+
  -- Colin Watson <cjwatson@debian.org>  Tue, 27 May 2008 10:17:15 +0100
 
 partman-partitioning (59) unstable; urgency=low

Attachment: signature.asc
Description: Digital signature


Reply to: