Bug#462396: Multiple disks support for partman-auto-lvm

2008-08-22 Thread Otavio Salvador
Jérémy Bobbio <[EMAIL PROTECTED]> writes:

> The work on this got little bit out of hand as I fixed all issues along
> the way, and I am a little bit uncomfortable in introducing so much
> changes at this stage of the release process.

I believe you both are doing very well regarting this patchset and
this is a very important feature that will be used by many people. 

> On the other hand, preseeded features are easy and quick to test
> (especially with KVM, virtio and a local mirror) and I am attaching all
> the preseeds that I have used thouroughly through the development
> of this patchset.  They all work through successful installations and
> can be called repeatedly in any order with success.

This could be added to the digress to allow us to track it in future
(Joey?) but being testable automatically also makes it easier to get
fixed fastly if a regression is found.

> I also confirm that manual installations continue to work as they

This is very important.

I'd not object about this to be merged NOW but I'd like to hear Frans
opinion on that before we do a final decision.

However I think we need to decide about it ASAP since this needs to
get some time on dailies to try to get regressions sorted out ASAP.

-- 
O T A V I OS A L V A D O R
-
 E-mail: [EMAIL PROTECTED]  UIN: 5906116
 GNU/Linux User: 239058 GPG ID: 49A5F855
 Home Page: http://otavio.ossystems.com.br
-
"Microsoft sells you Windows ... Linux gives
 you the whole house."



--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]



Bug#462396: Multiple disks support for partman-auto-lvm

2008-08-22 Thread Jérémy Bobbio
On Thu, Aug 21, 2008 at 08:44:17PM +0200, Grégory Oestreicher wrote:
> If there are no more comments on this patch, what's the next step ? I'll be 
> off in vacation from tomorrow for next week and won't be able to read my 
> mails, so if something must be done to integrate it in Lenny someone else 
> will have to take care of this.

Glad that you ask.  I have been working on this for the past three days
straight! :)

Attached is a pretty dense patchset integrating (a modified version of)
your patch and other changes in order to support it properly.

Main differences from your last patch:

 * Split refactoring, bug fixes and features integration

   The patchset should be fairly easy to follow. The biggest patch being
   the one introducing support for multiple disks in partman-auto-lvm,
   though.
 
 * Removal of partman-auto-lvm/extra_devices

   All disks are specified in partman-auto/disk.  The first one being
   the default.  This is more consistent to what we already support with
   partman-auto-raid.

 * Mapping of Volume Groups and Physical Volumes done in
   auto_lvm_prepare()

   This one required changes in the way $pv_devices was filled.
 
 * Support proper removal of Volume Groups on multiple disks

   That was the tricky part.  As documented in the commit logs, the way
   partman was previously restarted when removing all PV or RAID
   partitions on a disk was not coping nice with multiple devices at
   all.  This bug currently affects partman-auto-raid, but would affect
   partman-auto-lvm supporting multiple disks.

   Manual installations are currently affected as well as we currently
   allow the creation of new labels on disks with activated RAID
   partitions.  Solving this would require introducing locks as we do
   for LVM, but that requires merging mdcfg and partman-md for good,
   something for Lenny+1, really.

The work on this got little bit out of hand as I fixed all issues along
the way, and I am a little bit uncomfortable in introducing so much
changes at this stage of the release process.

On the other hand, preseeded features are easy and quick to test
(especially with KVM, virtio and a local mirror) and I am attaching all
the preseeds that I have used thouroughly through the development
of this patchset.  They all work through successful installations and
can be called repeatedly in any order with success.

I also confirm that manual installations continue to work as they
should.

I welcome other opinions here.  Grégory also has submitted the first
version of this patch in January, and have reworked the patch several
times with the feedback we gave him.  If these changes don't go in for
Lenny, we at least owe an apology for not finishing this up earlier.

Here's the index of the patchset, nevertheless:
 * Move dev_to_partman() to lib/auto-shared.sh
 * Remove useless dev_to_partman() definition from initial_auto_raid
 * Factor out conversion to megabytes in partman
 * Remove devfs resolving for PV devices
 * Fill $pv_devices earlier when using crypto method
 * Reset partman-lvm/device_remove_lvm after asking
 * Fix preseeding of partman-md/device_remove_md
 * Use 255 as return code when user backs up in device_remove_{md,lvm}()
 * Move the cleanup of LVM and MD devices out of create_new_label()
 * Restart partman once for all devices in prepare_new_labels
 * Move get_last_free_partition_infos() out of auto_init_disk()
 * Rename auto_init_disk() to auto_init_disks()
 * Rename $dev to $main_device in auto_lvm_prepare()
 * Factor out envelope creating in partman-auto-lvm
 * Factor out the creation of Physical Volume partitions
 * Reuse $defvgname in auto_lvm_perform()
 * Support multiple disks in partman-auto-lvm
 * Add support for preseeding logical volume names
 * Allow removal of VG spaning on autopartitioned disks
 * Remove extra $() construct in remove_lvm_find_vgs()
 * Initialize all disks at once when doing RAID autopartitioning

> If what's missing is the documentation, I'm working on it but other projects 
> are requiring my attention and I can't spend as much time as I'd like on it. 
> Maybe you'll can review a first draft after my holidays, if everything goes 
> well.

This would be very welcome.

Cheers,
-- 
Jérémy Bobbio.''`. 
[EMAIL PROTECTED]: :Ⓐ  :  # apt-get install anarchism
`. `'` 
  `-   


partman_lvm_multiple_disks.diff.gz
Description: Binary data
# Standard LVM atomic

d-i debconf/priority select high
d-i debian-installer/locale string fr_FR
d-i console-keymaps-at/keymap select dvorak
d-i netcfg/get_hostname string debian
d-i netcfg/get_domain string 
d-i mirror/country string enter information manually
d-i mirror/http/hostname string 192.168.11.1
d-i mirror/http/directory string /debian-partial
d-i mirror/http/proxy string
d-i mirror/suite string sid
d-i debian-installer/allow_unauthenticated string true
d-i partman-auto/method string lvm
d-

Bug#462396: Multiple disks support for partman-auto-lvm

2008-08-21 Thread Grégory Oestreicher
Hi,

If there are no more comments on this patch, what's the next step ? I'll be 
off in vacation from tomorrow for next week and won't be able to read my 
mails, so if something must be done to integrate it in Lenny someone else 
will have to take care of this.

If what's missing is the documentation, I'm working on it but other projects 
are requiring my attention and I can't spend as much time as I'd like on it. 
Maybe you'll can review a first draft after my holidays, if everything goes 
well.

Cheers,
Grégory



--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]



Bug#462396: Multiple disks support for partman-auto-lvm

2008-08-05 Thread Grégory Oestreicher
Hi,

Here are the patches concerning the functional changes. I also modified the 
error strings to follow Christian Perrier's advices. Please note that this 
patch adds a new string, "Multiple disks", that should be translated.

Cheers,
Grégory
diff -ruN --exclude=.svn --exclude=debian debian-installer.refactor/packages/partman/partman-auto-crypto/autopartition-crypto debian-installer.fchanges/packages/partman/partman-auto-crypto/autopartition-crypto
--- debian-installer.refactor/packages/partman/partman-auto-crypto/autopartition-crypto	2008-07-31 21:11:32.0 +0200
+++ debian-installer.fchanges/packages/partman/partman-auto-crypto/autopartition-crypto	2008-07-31 23:38:21.0 +0200
@@ -3,10 +3,10 @@
 . /lib/partman/lib/auto-lvm.sh
 . /lib/partman/lib/crypto-base.sh
 
-dev="$1"
+targetdev="$1"
 method=crypto
 
-auto_lvm_prepare $dev $method || exit $?
+auto_lvm_prepare $targetdev $method || exit $?
 
 found=no
 for dev in $DEVICES/*; do
@@ -28,15 +28,13 @@
 
 		echo dm-crypt > $id/crypto_type
 		crypto_prepare_method "$dev/$id" dm-crypt || exit 1
-		found=yes
-		break
 	done
-	[ $found = yes ] && break
 done
 
 crypto_check_setup || exit 1
 crypto_setup no || exit 1
 
+pv_devices=''
 # This is a kludge to workaround parted's refusal to allow dm devices
 # to be used for LVM
 for dev in $DEVICES/*; do
@@ -45,7 +43,7 @@
 	[ -f "$dev/device" ] || continue
 
 	# Found it
-	pv_devices=$(cat $dev/device)
+	pv_devices="$pv_devices $(cat $dev/device)"
 
 	# We should have only one partition, but lets be thorough
 	cd $dev
@@ -65,7 +63,7 @@
 	done
 done
 
-auto_lvm_perform || exit 1
+auto_lvm_perform $targetdev || exit 1
 
 # partman likes to believe that the virtual devices have a changed
 # partition table
diff -ruN --exclude=.svn debian-installer.refactor/packages/partman/partman-auto-lvm/autopartition-lvm debian-installer.fchanges/packages/partman/partman-auto-lvm/autopartition-lvm
--- debian-installer.refactor/packages/partman/partman-auto-lvm/autopartition-lvm	2008-07-31 21:11:28.0 +0200
+++ debian-installer.fchanges/packages/partman/partman-auto-lvm/autopartition-lvm	2008-08-02 12:24:30.0 +0200
@@ -13,4 +13,4 @@
 	pv_devices="$pv_devices $realpath"
 done
 
-auto_lvm_perform || exit 1
+auto_lvm_perform $dev || exit 1
diff -ruN --exclude=.svn debian-installer.refactor/packages/partman/partman-auto-lvm/debian/partman-auto-lvm.templates debian-installer.fchanges/packages/partman/partman-auto-lvm/debian/partman-auto-lvm.templates
--- debian-installer.refactor/packages/partman/partman-auto-lvm/debian/partman-auto-lvm.templates	2008-07-31 21:11:28.0 +0200
+++ debian-installer.fchanges/packages/partman/partman-auto-lvm/debian/partman-auto-lvm.templates	2008-08-05 16:15:47.0 +0200
@@ -42,3 +42,28 @@
  the volume group.
  .
  Check /var/log/syslog or see virtual console 4 for the details.
+
+Template: partman-auto-lvm/text/multiple_disks
+Type: text
+# :sl1:
+_Description: Multiple disks (%s)
+
+Template: partman-auto-lvm/no_such_pv
+Type: error
+# :sl3:
+_Description: Non-existing physical volume
+ A volume group definition contains a reference to a non-existing
+ physical volume.
+ .
+ Please check that all devices are properly connected.
+ Alternatively, please check the automatic partitioning recipe.
+
+Template: partman-auto-lvm/no_pv_in_vg
+Type: error
+# :sl3:
+_Description: No physical volume defined in volume group
+ The automatic partitioning recipe contains the definition of a
+ volume group that does not contain any physical volume.
+ .
+ Please check the automatic partitioning recipe.
+
diff -ruN --exclude=.svn debian-installer.refactor/packages/partman/partman-auto-lvm/lib/auto-lvm.sh debian-installer.fchanges/packages/partman/partman-auto-lvm/lib/auto-lvm.sh
--- debian-installer.refactor/packages/partman/partman-auto-lvm/lib/auto-lvm.sh	2008-08-03 18:45:50.0 +0200
+++ debian-installer.fchanges/packages/partman/partman-auto-lvm/lib/auto-lvm.sh	2008-08-03 20:55:46.0 +0200
@@ -50,7 +50,7 @@
 	local dev free_size
 	dev=$1
 	
-	get_disk_infos $dev;
+	get_last_free_partition_infos $dev
 	free_size=$(expr 000"$free_size" : '0*\(..*\)..$') # convert to megabytes
 
 	expand_scheme
@@ -60,13 +60,24 @@
 }
 
 auto_lvm_prepare() {
-	local dev method size free_size normalscheme target
+	local dev method size free_size normalscheme target extra_devices tmpdev tmpdevdir physdev
 	dev=$1
 	method=$2
 
 	[ -f $dev/size ] || return 1
 	size=$(cat $dev/size)
 
+	extra_devices=''
+	if db_get partman-auto-lvm/extra_devices && [ "$RET" ]; then
+		for tmpdev in "$RET"; do
+			tmpdevdir="$(dev_to_partman $tmpdev)"
+			if [ -d "$tmpdevdir" ]; then
+size=$(($size + $(cat $tmpdevdir/size)))
+extra_devices="$extra_devices $tmpdevdir"
+			fi
+		done
+	fi
+
 	# Be sure the modules are loaded
 	modprobe dm-mod >/dev/null 2>&1 || true
 	modprobe lvm-mod >/dev/null 2>&1 || true
@@ -75,21 +86,34 @@
 		log-output -t update-dev update-dev
 	fi
 
-	target="$(humandev

Bug#462396: Multiple disks support for partman-auto-lvm

2008-08-03 Thread Grégory Oestreicher
Le dimanche 03 août 2008 09:06, Jérémy Bobbio a écrit :
> > It puts in global scope some attributes about the last free partition
> > found on the disk. So what about get_last_free_partition_infos() ?
>
> A lot more meaningful, indeed. :)

Here is an updated patch for the refactoring part, which also corrects a 
stupid error (I forgot the parenthesis after auto_lvm_create_envelope()).

Cheers,
Grégory
Index: partman-auto-raid/display.d/initial_auto_raid
===
--- partman-auto-raid/display.d/initial_auto_raid	(révision 54731)
+++ partman-auto-raid/display.d/initial_auto_raid	(copie de travail)
@@ -5,25 +5,8 @@
 
 . /lib/partman/lib/base.sh
 . /lib/partman/lib/commit.sh
+. /lib/partman/lib/auto-shared.sh
 
-dev_to_partman () {
-	local dev_name="$1"
-
-	local mapped_dev_name="$(mapdevfs $dev_name)"
-	if [ -n "$mapped_dev_name" ]; then
-		dev_name="$mapped_dev_name"
-	fi
-
-	for dev in $DEVICES/*; do
-		# mapdevfs both to allow for different ways to refer to the
-		# same device using devfs, and to allow user input in
-		# non-devfs form
-		if [ "$(mapdevfs $(cat $dev/device))" = "$dev_name" ]; then
-			echo $dev
-		fi
-	done
-}
-
 # See if we are supposed to run and only run once
 db_get partman-auto/method
 if [ "$RET" != raid ] || \
Index: partman-auto-lvm/lib/auto-lvm.sh
===
--- partman-auto-lvm/lib/auto-lvm.sh	(révision 54731)
+++ partman-auto-lvm/lib/auto-lvm.sh	(copie de travail)
@@ -10,6 +10,55 @@
 	exit 1
 }
 
+# Creates, if needed, the envelope to hold LVM VGs.
+# 
+# We need to create the envelope only if one is not defined. This is the case 
+# when :
+# 	- the default device is not part of a PV declaration in the scheme (a PV
+#	  is declared when there's a method{ lvm } or method{ crypto } attribute) ;
+#	*AND*
+#	- the recipe contains a PV declaration *without* device.
+#
+# For this case the physical device used will be the default one.
+#
+# First arg : the scheme to add the envelope to
+# Second arg : the physical device (ie /dev/hda)
+# Third arg : the method to use (lvm or crypto)
+# Returns : the scheme with the envelope if needed
+#
+auto_lvm_create_envelope() {
+	local scheme physdev method
+	scheme="$1"
+	physdev=$2
+	method=$3
+
+	if 	! echo "$scheme" | grep -E "method\{ (lvm|crypto) \}" | grep -q "device{ $physdev[[:digit:]]* }" && \
+		! echo "$scheme" | grep -E "method\{ (lvm|crypto) \}" | grep -q -v "device{"; then
+			scheme="$scheme${NL}100 1000 10 ext3 \$primary{ } method{ $method }"
+	fi
+
+	echo "$scheme"
+}
+
+# This function  depends on the existence of $scheme and $devfspv_devices in scope
+#
+# It will create the partitions needed by a recipe / scheme to hold all PVs
+#
+# First arg : the path to the partman directory for the device
+#
+auto_lvm_create_partitions() {
+	local dev free_size
+	dev=$1
+	
+	get_disk_infos $dev;
+	free_size=$(expr 000"$free_size" : '0*\(..*\)..$') # convert to megabytes
+
+	expand_scheme
+
+	create_primary_partitions
+	create_partitions
+}
+
 auto_lvm_prepare() {
 	local dev method size free_size normalscheme target
 	dev=$1
Index: partman-auto/display.d/initial_auto
===
--- partman-auto/display.d/initial_auto	(révision 54731)
+++ partman-auto/display.d/initial_auto	(copie de travail)
@@ -9,26 +9,6 @@
 
 . /lib/partman/lib/auto-shared.sh
 
-dev_to_partman () {
-	local dev_name="$1"
-
-	local mapped_dev_name="$(mapdevfs $dev_name)"
-	if [ -n "$mapped_dev_name" ]; then
-		dev_name="$mapped_dev_name"
-	fi
-
-	for dev in $DEVICES/*; do
-		[ -d "$dev" ] || continue
-
-		# mapdevfs both to allow for different ways to refer to the
-		# same device using devfs, and to allow user input in
-		# non-devfs form
-		if [ "$(mapdevfs $(cat $dev/device))" = "$dev_name" ]; then
-			echo $dev
-		fi
-	done
-}
-
 # Skip if no disks detected and don't run on S/390
 if [ -z "$(get_auto_disks)" ] || \
[ "$(udpkg --print-architecture)" = s390 ]; then
Index: partman-auto/lib/auto-shared.sh
===
--- partman-auto/lib/auto-shared.sh	(révision 54731)
+++ partman-auto/lib/auto-shared.sh	(copie de travail)
@@ -8,6 +8,13 @@
 	. /lib/partman/lib/disk-label.sh
 	create_new_label "$dev" no || return 1
 
+	get_last_free_partition_infos $dev
+}
+
+get_last_free_partition_infos() {
+	local dev
+	dev=$1
+
 	cd $dev
 
 	free_space=''
@@ -213,3 +220,25 @@
 
 # TODO: Add a select_auto_disks() function
 # Note: This needs a debconf_multiselect equiv.
+
+# Maps a devfs name to a partman directory
+dev_to_partman () {
+	local dev_name="$1"
+
+	local mapped_dev_name="$(mapdevfs $dev_name)"
+	if [ -n "$mapped_dev_name" ]; then
+		dev_name="$mapped_dev_name"
+	fi
+
+	for dev in $DEVICES/*; do
+		[ -d "$dev" ] || continue
+
+		# mapdevfs both to allow for different ways to refer to the
+		# same device using devfs, and to all

Bug#462396: Multiple disks support for partman-auto-lvm

2008-08-03 Thread Jérémy Bobbio
On Sat, Aug 02, 2008 at 11:14:48AM +0200, Grégory Oestreicher wrote:
> Le samedi 02 août 2008 01:21, Jérémy Bobbio a écrit :
> > NACK for get_disk_infos().  This name does not reflect at all what the
> > function does, as far as I understand it.
> 
> It puts in global scope some attributes about the last free partition found 
> on 
> the disk. So what about get_last_free_partition_infos() ?

A lot more meaningful, indeed. :)

Cheers,
-- 
Jérémy Bobbio.''`. 
[EMAIL PROTECTED]: :Ⓐ  :  # apt-get install anarchism
`. `'` 
  `-   


signature.asc
Description: Digital signature


Bug#462396: Multiple disks support for partman-auto-lvm

2008-08-02 Thread Grégory Oestreicher
Le samedi 02 août 2008 01:21, Jérémy Bobbio a écrit :
> NACK for get_disk_infos().  This name does not reflect at all what the
> function does, as far as I understand it.

It puts in global scope some attributes about the last free partition found on 
the disk. So what about get_last_free_partition_infos() ?

Cheers,
Grégory



--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]



Bug#462396: Multiple disks support for partman-auto-lvm

2008-08-02 Thread Jérémy Bobbio
On Thu, Jul 31, 2008 at 10:31:11PM +0200, Grégory Oestreicher wrote:
> Le mercredi 30 juillet 2008 01:48, Jérémy Bobbio a écrit :
> > > Some minor refactoring was needed when writing the code, so why not doing
> > > it while it's still hot ? However I can submit two separate patches if
> > > it's clearer.
> >
> > I'm already asking quite some work, but if you have the energy, it would
> > be lovely. 
> 
> Here is the patch concerning the refactoring part. I moved the definition of 
> dev_to_partman outside of partman-auto-raid and replaced it by the 
> appropriate include, but couldn't see the function used anywhere. As I have 
> no time / resources to test this I chose to do it this way, but if someone 
> knows for sure the function is not needed I can remove the inclusion.

As far as I have been able to dig through the history, it has never
actually been used.  initial_auto_raid probably started as a copy of
initial_auto and the function was probably never removed.

This part of your patch is pending on my queue.

> I also added the definition of some function used in the second part of the 
> patch. The envelope creation is now a separate function as it is called 
> twice. I think the functional changes patch will be ready this week end 
> (2008-08-03) for review, after all required tests on my side.

NACK for get_disk_infos().  This name does not reflect at all what the
function does, as far as I understand it.

Cheers,
-- 
Jérémy Bobbio.''`. 
[EMAIL PROTECTED]: :Ⓐ  :  # apt-get install anarchism
`. `'` 
  `-   


signature.asc
Description: Digital signature


Bug#462396: Multiple disks support for partman-auto-lvm

2008-07-31 Thread Otavio Salvador
Grégory Oestreicher <[EMAIL PROTECTED]> writes:

> Le mercredi 30 juillet 2008 01:48, Jérémy Bobbio a écrit :
>> > Some minor refactoring was needed when writing the code, so why not doing
>> > it while it's still hot ? However I can submit two separate patches if
>> > it's clearer.
>>
>> I'm already asking quite some work, but if you have the energy, it would
>> be lovely. 
>
> Here is the patch concerning the refactoring part. I moved the definition of 
> dev_to_partman outside of partman-auto-raid and replaced it by the 
> appropriate include, but couldn't see the function used anywhere. As I have 
> no time / resources to test this I chose to do it this way, but if someone 
> knows for sure the function is not needed I can remove the inclusion.

This part looks very clear and could be commited if Jeremy and Frans thinks it
is OK.

-- 
O T A V I OS A L V A D O R
-
 E-mail: [EMAIL PROTECTED]  UIN: 5906116
 GNU/Linux User: 239058 GPG ID: 49A5F855
 Home Page: http://otavio.ossystems.com.br
-
"Microsoft sells you Windows ... Linux gives
 you the whole house."



--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]



Bug#462396: Multiple disks support for partman-auto-lvm

2008-07-31 Thread Grégory Oestreicher
Le mercredi 30 juillet 2008 01:48, Jérémy Bobbio a écrit :
> > Some minor refactoring was needed when writing the code, so why not doing
> > it while it's still hot ? However I can submit two separate patches if
> > it's clearer.
>
> I'm already asking quite some work, but if you have the energy, it would
> be lovely. 

Here is the patch concerning the refactoring part. I moved the definition of 
dev_to_partman outside of partman-auto-raid and replaced it by the 
appropriate include, but couldn't see the function used anywhere. As I have 
no time / resources to test this I chose to do it this way, but if someone 
knows for sure the function is not needed I can remove the inclusion.

I also added the definition of some function used in the second part of the 
patch. The envelope creation is now a separate function as it is called 
twice. I think the functional changes patch will be ready this week end 
(2008-08-03) for review, after all required tests on my side.

Cheers,
Grégory
Index: partman-auto-raid/display.d/initial_auto_raid
===
--- partman-auto-raid/display.d/initial_auto_raid	(révision 54731)
+++ partman-auto-raid/display.d/initial_auto_raid	(copie de travail)
@@ -5,25 +5,8 @@
 
 . /lib/partman/lib/base.sh
 . /lib/partman/lib/commit.sh
+. /lib/partman/lib/auto-shared.sh
 
-dev_to_partman () {
-	local dev_name="$1"
-
-	local mapped_dev_name="$(mapdevfs $dev_name)"
-	if [ -n "$mapped_dev_name" ]; then
-		dev_name="$mapped_dev_name"
-	fi
-
-	for dev in $DEVICES/*; do
-		# mapdevfs both to allow for different ways to refer to the
-		# same device using devfs, and to allow user input in
-		# non-devfs form
-		if [ "$(mapdevfs $(cat $dev/device))" = "$dev_name" ]; then
-			echo $dev
-		fi
-	done
-}
-
 # See if we are supposed to run and only run once
 db_get partman-auto/method
 if [ "$RET" != raid ] || \
Index: partman-auto-lvm/lib/auto-lvm.sh
===
--- partman-auto-lvm/lib/auto-lvm.sh	(révision 54731)
+++ partman-auto-lvm/lib/auto-lvm.sh	(copie de travail)
@@ -10,6 +10,55 @@
 	exit 1
 }
 
+# Creates, if needed, the envelope to hold LVM VGs.
+# 
+# We need to create the envelope only if one is not defined. This is the case 
+# when :
+# 	- the default device is not part of a PV declaration in the scheme (a PV
+#	  is declared when there's a method{ lvm } or method{ crypto } attribute) ;
+#	*AND*
+#	- the recipe contains a PV declaration *without* device.
+#
+# For this case the physical device used will be the default one.
+#
+# First arg : the scheme to add the envelope to
+# Second arg : the physical device (ie /dev/hda)
+# Third arg : the method to use (lvm or crypto)
+# Returns : the scheme with the envelope if needed
+#
+auto_lvm_create_envelope {
+	local scheme physdev method
+	scheme="$1"
+	physdev=$2
+	method=$3
+
+	if 	! echo "$scheme" | grep -E "method\{ (lvm|crypto) \}" | grep -q "device{ $physdev[[:digit:]]* }" && \
+		! echo "$scheme" | grep -E "method\{ (lvm|crypto) \}" | grep -q -v "device{"; then
+			scheme="$scheme${NL}100 1000 10 ext3 \$primary{ } method{ $method }"
+	fi
+
+	echo "$scheme"
+}
+
+# This function  depends on the existence of $scheme and $devfspv_devices in scope
+#
+# It will create the partitions needed by a recipe / scheme to hold all PVs
+#
+# First arg : the path to the partman directory for the device
+#
+auto_lvm_create_partitions() {
+	local dev free_size
+	dev=$1
+	
+	get_disk_infos $dev;
+	free_size=$(expr 000"$free_size" : '0*\(..*\)..$') # convert to megabytes
+
+	expand_scheme
+
+	create_primary_partitions
+	create_partitions
+}
+
 auto_lvm_prepare() {
 	local dev method size free_size normalscheme target
 	dev=$1
Index: partman-auto/display.d/initial_auto
===
--- partman-auto/display.d/initial_auto	(révision 54731)
+++ partman-auto/display.d/initial_auto	(copie de travail)
@@ -9,26 +9,6 @@
 
 . /lib/partman/lib/auto-shared.sh
 
-dev_to_partman () {
-	local dev_name="$1"
-
-	local mapped_dev_name="$(mapdevfs $dev_name)"
-	if [ -n "$mapped_dev_name" ]; then
-		dev_name="$mapped_dev_name"
-	fi
-
-	for dev in $DEVICES/*; do
-		[ -d "$dev" ] || continue
-
-		# mapdevfs both to allow for different ways to refer to the
-		# same device using devfs, and to allow user input in
-		# non-devfs form
-		if [ "$(mapdevfs $(cat $dev/device))" = "$dev_name" ]; then
-			echo $dev
-		fi
-	done
-}
-
 # Skip if no disks detected and don't run on S/390
 if [ -z "$(get_auto_disks)" ] || \
[ "$(udpkg --print-architecture)" = s390 ]; then
Index: partman-auto/lib/auto-shared.sh
===
--- partman-auto/lib/auto-shared.sh	(révision 54731)
+++ partman-auto/lib/auto-shared.sh	(copie de travail)
@@ -8,6 +8,13 @@
 	. /lib/partman/lib/disk-label.sh
 	create_new_label "$dev" no || return 1
 
+	get_disk_infos $dev
+}
+
+get_d

Bug#462396: Multiple disks support for partman-auto-lvm

2008-07-31 Thread Grégory Oestreicher
>>   We need to create the envelope only if one is not defined. This is the case
>> when :
>> - the default device is not part of a PV declaration in the scheme (a PV
>>   is declared when there's a method{ lvm } or method{ crypto } 
>> attribute).
>> - the recipe contains a PV declaration *without* device. For this case 
>> the
>>   physical device used will be the default one.
>>
>> If it's OK with you (and clear) I'll integrate this in the next patch.
>
> Yes, that's a lot better.  If you could add an example use case for each
> of these, it would be even better. ;)

This made me realize that I forgot the 'and' between the two points...

> Preseeded encrypted installations are fairly rare: you need to be
> sitting right behind the box to type a meaningful passphrase.  I think
> it does not make a lot of sense to make partman-auto-lvm more complex
> that it already is for such corner cases.

OK then.

>> > > +# The recipe contains all the necessary informations about 
>> > > eventuals
>> > > +# extra VGs to create
>> > > +# The VGs to create are :
>> > > +#   - the default one if some partitions don't have the invg{ } 
>> > > tag
>> > > +#   - the ones present in vgname{ } tags of PVs
>> > > +decode_recipe $recipe lvm
>> > > […]

> I might have overlooked that decode_recipe() is called twice, but I
> still think this part would be more at its place in auto_lvm_prepare() as
> its extracting informations for the recipe and not yet acting on them.

This is not possible either : $pv_devices is not defined in auto_lvm_prepare() 
and it
is needed some lines further. The only thing I see to prevent the double call to
decode_recipe() is to save $scheme in a temporary location and reload it here.

I'm fine with all your other points and will integrate them.

Cheers,
Grégory




--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]



Bug#462396: Multiple disks support for partman-auto-lvm

2008-07-29 Thread Jérémy Bobbio
On Tue, Jul 29, 2008 at 07:42:26PM +0200, Grégory Oestreicher wrote:
> Le mardi 29 juillet 2008 02:18, Jérémy Bobbio a écrit :
> > > +# This function was copied from another file
> > > (partman-auto/lib/initial_auto). +# Maybe it should be useful to put it
> > > in auto-shared.sh or in base.sh +dev_to_partman () {
> > > […]
> >
> > It is already in partman-auto-raid, so I think it should be factored in
> > auto-shared.sh.
> 
> OK, I'll move this into auto-shared.sh and submit an updated patch later this 
> week.

Please make this a different patch in your patch set.

> > The comment should probably be expanded a little bit to mention the use
> > cases.
> 
> Here it is :
>   We need to create the envelope only if one is not defined. This is the case 
> when :
>     - the default device is not part of a PV declaration in the scheme (a PV
>       is declared when there's a method{ lvm } or method{ crypto } attribute).
>     - the recipe contains a PV declaration *without* device. For this case the
>       physical device used will be the default one.
> 
> If it's OK with you (and clear) I'll integrate this in the next patch.

Yes, that's a lot better.  If you could add an example use case for each
of these, it would be even better. ;)

> > This makes me uneasy: optional partman components tries to stay pretty
> > independant from each others and the use of "_crypt" here breaks this
> > assumption.  I don't get exactly how is this useful, either.
> 
> I did this to allow to use the name of the encrypted device in the recipes. 
> Now that you point it I don't think it's really useful, it was more a 
> convenience. I'll remove it from future versions.
> 
> However the same kind of checks / removal is done further down in the file, 
> and these, despite being a convenience, are still useful : they allow people 
> to use "normal" name of the device (ie /dev/hda2) instead of the 
> partman-crypto name (/dev/mapper/hda2_crypt) in the recipes.

Preseeded encrypted installations are fairly rare: you need to be
sitting right behind the box to type a meaningful passphrase.  I think
it does not make a lot of sense to make partman-auto-lvm more complex
that it already is for such corner cases.

> > > + # The recipe contains all the necessary informations about eventuals
> > > + # extra VGs to create
> > > + # The VGs to create are :
> > > + #   - the default one if some partitions don't have the invg{ } tag
> > > + #   - the ones present in vgname{ } tags of PVs
> > > + decode_recipe $recipe lvm
> > > […]
> >
> > Is there anything preventing this step to happen in auto_lvm_prepare
> > where the recipe is already available?
> 
> decode_recipe() is called twice : once in auto_lvm_prepare, and once in 
> auto_lvm_perform. This is needed to have $scheme in scope for further checks. 
> Before the call to decode_recipe, $scheme is not visible in the current scope 
> (I don't think it still exists at all in fact).

I might have overlooked that decode_recipe() is called twice, but I
still think this part would be more at its place in auto_lvm_prepare() as
its extracting informations for the recipe and not yet acting on them.

> > > --- autopartition-crypto  (révision 54599)
> > > +++ autopartition-crypto  (copie de travail)
> > > @@ -3,10 +3,10 @@
> > >  . /lib/partman/lib/auto-lvm.sh
> > >  . /lib/partman/lib/crypto-base.sh
> > >
> > > -dev="$1"
> > > +argdev="$1"
> > >  method=crypto
> > >
> > > -auto_lvm_prepare $dev $method || exit 1
> > > +auto_lvm_prepare $argdev $method || exit 1
> >
> > Looks like a gratious change.  Any rationale?
> 
> Yes, $dev is overwritten by the next for loop and I need it untouched as 
> first 
> argument to auto_lvm_perform. I thought there were less risks of side effects 
> to change the name of this variable here rather than changing all $dev 
> occurences in the for loop.

You are right.  But $argdev does not sound too good.  What about
$targetdev or something similar?

> > > […]
> > > - found=yes
> > > - break
> > > +#found=yes
> > > +#break
> > >   done
> > > - [ $found = yes ] && break
> > > +#[ $found = yes ] && break
> >
> > Commented out code without any explaination is a major hassle.  If it
> > should not be there anymore, please delete it and explain why.
> 
> I hesitated before leaving it commented out in the public patch, but decided 
> to go anyway like this to have advices from people who know why these breaks 
> are here. It's true I should have added a comment asking. I couldn't see any 
> problem with commenting this out, as all devices must be encrypted when there 
> are more than one, but once again, it needs to be approved.

Well, the previous behaviour was encrypting only one device, so it
maked sense to quit the loop as soon as it was found.  If you change
this assumption, then code based on it should be removed.

> > On the overall, parts of this patch makes me uneasy.  Maybe because
> > refactoring is mixed up with f

Bug#462396: Multiple disks support for partman-auto-lvm

2008-07-29 Thread Grégory Oestreicher
Le mardi 29 juillet 2008 02:18, Jérémy Bobbio a écrit :
> > +# This function was copied from another file
> > (partman-auto/lib/initial_auto). +# Maybe it should be useful to put it
> > in auto-shared.sh or in base.sh +dev_to_partman () {
> > […]
>
> It is already in partman-auto-raid, so I think it should be factored in
> auto-shared.sh.

OK, I'll move this into auto-shared.sh and submit an updated patch later this 
week.

> > +   if [ $extra_devices ]; then
> > +   for extradev in $dev $extra_devices; do
> > +   extradev_str="$extradev_str $(cat $extradev/device)"
> > +   done
> > +   target="Multiple disks ($extradev_str)"
> >
> > +   else
> > +   target="$(humandev $(cat $dev/device)) - $(cat $dev/model)"
> > +   fi
>
> "Multiple disks" should be translatable.  Maybe factored out in a
> humandevs() function?

I don't think writing a function to perform a db_metaget and a printf will be 
necessary. The humandev() function also tries to get the exact type of disk 
which is not needed here, so the translation can be done inline.

> > @@ -101,27 +141,70 @@
> > # (still one atm)
> > devfspv_devices=''
> >
> > -   create_primary_partitions
> > +   # Creating envelope
> > +   # Only if one does not already exist (identified by 'method{ lvm }' and
> > by +# the current device in the scheme)
> > +   physdev=$(cat $dev/device)
> > +   if ! echo "$normalscheme" | grep -E "method\{ (lvm|crypto) \}" | grep
> > -q "device{ $physdev[[:digit:]]* }" && \ + ! echo "$normalscheme" |
> > grep -E "method\{ (lvm|crypto) \}" | grep -q -v "device{"; then
> > +   normalscheme="$normalscheme${NL}100 1000 10 ext3 
> > \$primary{ }
> > method{ $method }" +fi
>
> The comment should probably be expanded a little bit to mention the use
> cases.

Here it is :
  We need to create the envelope only if one is not defined. This is the case 
when :
    - the default device is not part of a PV declaration in the scheme (a PV
      is declared when there's a method{ lvm } or method{ crypto } attribute).
    - the recipe contains a PV declaration *without* device. For this case the
      physical device used will be the default one.

If it's OK with you (and clear) I'll integrate this in the next patch.

> Why the test is actually made twice?  Isn't "method{ lvm }" always
> defining a physical volume?  All your test recipes have a
> "device{ ...  }" defined, what happens when it is not specified?

I think you missed the '-v' in the last grep of the second test. When a device 
is not specified, as it is the case with the default recipes, the PV will be 
created on the default device. It will also be integrated in the default VG.

>
> > […]
> >  auto_lvm_perform() {
> > +   local IFS physdev defvgname schemeline schemevg schemedev targetdir
> > targetvg pvdevice + [ -n $1 ] || return 1
> > +
> > +   physdev=$(cat $1/device)
> > +   if echo $physdev | grep -q '/mapper/.*_crypt'; then
> > +   physdev=$(echo $physdev | sed -re
> > 's#mapper/([^[:digit:]]+)[[:digit:]]_crypt#\1#') +  fi
>
> This makes me uneasy: optional partman components tries to stay pretty
> independant from each others and the use of "_crypt" here breaks this
> assumption.  I don't get exactly how is this useful, either.

I did this to allow to use the name of the encrypted device in the recipes. 
Now that you point it I don't think it's really useful, it was more a 
convenience. I'll remove it from future versions.

However the same kind of checks / removal is done further down in the file, 
and these, despite being a convenience, are still useful : they allow people 
to use "normal" name of the device (ie /dev/hda2) instead of the 
partman-crypto name (/dev/mapper/hda2_crypt) in the recipes.

> > +   # The recipe contains all the necessary informations about eventuals
> > +   # extra VGs to create
> > +   # The VGs to create are :
> > +   #   - the default one if some partitions don't have the invg{ } tag
> > +   #   - the ones present in vgname{ } tags of PVs
> > +   decode_recipe $recipe lvm
> > […]
>
> Is there anything preventing this step to happen in auto_lvm_prepare
> where the recipe is already available?

decode_recipe() is called twice : once in auto_lvm_prepare, and once in 
auto_lvm_perform. This is needed to have $scheme in scope for further checks. 
Before the call to decode_recipe, $scheme is not visible in the current scope 
(I don't think it still exists at all in fact).

> > […]
> > --- perform_recipe_by_lvm   (révision 54599)
> > +++ perform_recipe_by_lvm   (copie de travail)
> > […]
> >  decode_recipe $recipe lvm
> > +tmpscheme=$(echo "$scheme" | grep lvmok)
> > +scheme=$(echo "$tmpscheme" | grep "invg{ *$VG_name *}")
> > +if [ $VG_name = $default_vgname ]; then
> > +   # Adding in the scheme the partitions that have no VG declared
> > +   extraschemelines=$(echo "$tmpscheme" | grep lvmok | grep -v 'invg{')
>^

Bug#462396: Multiple disks support for partman-auto-lvm

2008-07-28 Thread Jérémy Bobbio
On Mon, Jul 28, 2008 at 06:54:12PM +0200, Grégory Oestreicher wrote:
> Here is an updated version of the patch, dealing with LVM+crypto, as
> suggested by Jérémy Bobbio. This patch also incorporate his modifications
> to add the possibility to define a custom LV name (see
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=484272).

A few comments after a quick review:

> Index: debian/partman-auto-lvm.templates
> ===
> --- debian/partman-auto-lvm.templates (révision 54599)
> +++ debian/partman-auto-lvm.templates (copie de travail)
> @@ -42,3 +42,19 @@
>   the volume group.
>   .
>   Check /var/log/syslog or see virtual console 4 for the details.
> +
> +Template: partman-auto-lvm/no_such_pv
> +Type: error
> +_Description: Inexistant PV declared
> + A Volume Group definition contains a reference to a Physical Volume
> + that is not present. 
> + .
> + Check for you connectivity or the recipe.
> +
> +Template: partman-auto-lvm/no_pv_in_vg
> +Type: error
> +_Description: No PV defined for creating VG
> + The recipe contains the definition of a Volume Group without
> + any Physical Volume in it.
> + .
> + Review the recipe.

These surely will need improvements by native speakers.

> Index: lib/auto-lvm.sh
> ===
> --- lib/auto-lvm.sh   (révision 54599)
> +++ lib/auto-lvm.sh   (copie de travail)
> @@ -10,14 +10,48 @@
>   exit 1
>  }
>  
> +# This function was copied from another file (partman-auto/lib/initial_auto).
> +# Maybe it should be useful to put it in auto-shared.sh or in base.sh
> +dev_to_partman () {
> […]

It is already in partman-auto-raid, so I think it should be factored in
auto-shared.sh.

>  auto_lvm_prepare() {
> - local dev method size free_size normalscheme target
> + local dev method size free_size normalscheme target extra_devices 
> tmpdev tmpdevdir physdev
> […]
> - target="$(humandev $(cat $dev/device)) - $(cat $dev/model)"
> + if [ $extra_devices ]; then
> + for extradev in $dev $extra_devices; do
> + extradev_str="$extradev_str $(cat $extradev/device)"
> + done
> + target="Multiple disks ($extradev_str)"

> + else
> + target="$(humandev $(cat $dev/device)) - $(cat $dev/model)"
> + fi

"Multiple disks" should be translatable.  Maybe factored out in a
humandevs() function?

> @@ -101,27 +141,70 @@
>   # (still one atm)
>   devfspv_devices=''
>  
> - create_primary_partitions
> + # Creating envelope
> + # Only if one does not already exist (identified by 'method{ lvm }' and 
> by 
> + # the current device in the scheme)
> + physdev=$(cat $dev/device)
> + if ! echo "$normalscheme" | grep -E "method\{ (lvm|crypto) \}" | grep 
> -q "device{ $physdev[[:digit:]]* }" && \
> +! echo "$normalscheme" | grep -E "method\{ (lvm|crypto) \}" | grep 
> -q -v "device{"; then
> + normalscheme="$normalscheme${NL}100 1000 10 ext3 
> \$primary{ } method{ $method }"
> + fi

The comment should probably be expanded a little bit to mention the use
cases.  Something like:
  Creating envelope:
  (This is only done for recipes where the location of the Physical
   Volumes have not been already defined, as the default ones.)

Why the test is actually made twice?  Isn't "method{ lvm }" always
defining a physical volume?  All your test recipes have a
"device{ ...  }" defined, what happens when it is not specified?

> […]
>  auto_lvm_perform() {
> + local IFS physdev defvgname schemeline schemevg schemedev targetdir 
> targetvg pvdevice
> + [ -n $1 ] || return 1
> + 
> + physdev=$(cat $1/device)
> + if echo $physdev | grep -q '/mapper/.*_crypt'; then
> + physdev=$(echo $physdev | sed -re 
> 's#mapper/([^[:digit:]]+)[[:digit:]]_crypt#\1#')
> + fi

This makes me uneasy: optional partman components tries to stay pretty
independant from each others and the use of "_crypt" here breaks this
assumption.  I don't get exactly how is this useful, either.

> + # The recipe contains all the necessary informations about eventuals
> + # extra VGs to create
> + # The VGs to create are :
> + #   - the default one if some partitions don't have the invg{ } tag
> + #   - the ones present in vgname{ } tags of PVs
> + decode_recipe $recipe lvm
> […]

Is there anything preventing this step to happen in auto_lvm_prepare
where the recipe is already available?

> […]
> --- perform_recipe_by_lvm (révision 54599)
> +++ perform_recipe_by_lvm (copie de travail)
> […]
>  decode_recipe $recipe lvm
> +tmpscheme=$(echo "$scheme" | grep lvmok)
> +scheme=$(echo "$tmpscheme" | grep "invg{ *$VG_name *}")
> +if [ $VG_name = $default_vgname ]; then
> + # Adding in the scheme the partitions that have no VG declared
> + extraschemelines=$(echo "$tmpscheme" | grep lvmok | grep -v 'invg{')

Bug#462396: Multiple disks support for partman-auto-lvm

2008-07-28 Thread Grégory Oestreicher
Damn, once again there are some remains from my tests... Please disregard
the file "partman-auto-lvm-multiple-disks-20080728.diff" from my previous
message. Here is the clean one.

Cheers,
GrégoryIndex: debian/partman-auto-lvm.templates
===
--- debian/partman-auto-lvm.templates	(révision 54599)
+++ debian/partman-auto-lvm.templates	(copie de travail)
@@ -42,3 +42,19 @@
  the volume group.
  .
  Check /var/log/syslog or see virtual console 4 for the details.
+
+Template: partman-auto-lvm/no_such_pv
+Type: error
+_Description: Inexistant PV declared
+ A Volume Group definition contains a reference to a Physical Volume
+ that is not present. 
+ .
+ Check for you connectivity or the recipe.
+
+Template: partman-auto-lvm/no_pv_in_vg
+Type: error
+_Description: No PV defined for creating VG
+ The recipe contains the definition of a Volume Group without
+ any Physical Volume in it.
+ .
+ Review the recipe.
Index: lib/auto-lvm.sh
===
--- lib/auto-lvm.sh	(révision 54599)
+++ lib/auto-lvm.sh	(copie de travail)
@@ -10,14 +10,48 @@
 	exit 1
 }
 
+# This function was copied from another file (partman-auto/lib/initial_auto).
+# Maybe it should be useful to put it in auto-shared.sh or in base.sh
+dev_to_partman () {
+	local dev_name="$1"
+	local dev
+
+	local mapped_dev_name="$(mapdevfs $dev_name)"
+	if [ -n "$mapped_dev_name" ]; then
+		dev_name="$mapped_dev_name"
+	fi
+
+	for dev in $DEVICES/*; do
+		[ -d "$dev" ] || continue
+
+		# mapdevfs both to allow for different ways to refer to the
+		# same device using devfs, and to allow user input in
+		# non-devfs form
+		if [ "$(mapdevfs $(cat $dev/device))" = "$dev_name" ]; then
+			echo $dev
+		fi
+	done
+}
+
 auto_lvm_prepare() {
-	local dev method size free_size normalscheme target
+	local dev method size free_size normalscheme target extra_devices tmpdev tmpdevdir physdev
 	dev=$1
 	method=$2
 
 	[ -f $dev/size ] || return 1
 	size=$(cat $dev/size)
 
+	extra_devices=''
+	if db_get partman-auto-lvm/extra_devices && [ "$RET" ]; then
+		for tmpdev in "$RET"; do
+			tmpdevdir="$(dev_to_partman $tmpdev)"
+			if [ -d "$tmpdevdir" ]; then
+size=$(($size + $(cat $tmpdevdir/size)))
+extra_devices="$extra_devices $tmpdevdir"
+			fi
+		done
+	fi
+
 	# Be sure the modules are loaded
 	modprobe dm-mod >/dev/null 2>&1 || true
 	modprobe lvm-mod >/dev/null 2>&1 || true
@@ -26,21 +60,32 @@
 		log-output -t update-dev update-dev
 	fi
 
-	target="$(humandev $(cat $dev/device)) - $(cat $dev/model)"
+	if [ $extra_devices ]; then
+		for extradev in $dev $extra_devices; do
+			extradev_str="$extradev_str $(cat $extradev/device)"
+		done
+		target="Multiple disks ($extradev_str)"
+	else
+		target="$(humandev $(cat $dev/device)) - $(cat $dev/model)"
+	fi
 	target="$target: $(longint2human $size)"
 	free_size=$(expr 000"$size" : '0*\(..*\)..$') # convert to megabytes
 
 	choose_recipe lvm "$target" "$free_size" || return $?
 
-	auto_init_disk "$dev" || return $?
+	size=0
+	for tmpdev in $dev $extra_devices; do
+		auto_init_disk "$tmpdev" || return $?
+		size=$(($size + $free_size))
 
-	# Check if partition is usable; use existing partman-auto template as we depend on it
-	if [ "$free_type" = unusable ]; then
-		db_input critical partman-auto/unusable_space || true
-		db_go || true
-		return 1
-	fi
-	free_size=$(expr 000"$free_size" : '0*\(..*\)..$') # convert to megabytes
+		# Check if partition is usable; use existing partman-auto template as we depend on it
+		if [ "$free_type" = unusable ]; then
+			db_input critical partman-auto/unusable_space || true
+			db_go || true
+			return 1
+		fi
+	done
+	free_size=$(expr 000"$size" : '0*\(..*\)..$') # convert to megabytes
 
 	decode_recipe $recipe lvm
 
@@ -88,11 +133,6 @@
 		;;
 	esac
 
-	# Creating envelope
-	scheme="$normalscheme${NL}100 1000 10 ext3 \$primary{ } method{ $method }"
-
-	expand_scheme
-
 	clean_method
 
 	# This variable will be used to store the partitions that will be LVM
@@ -101,27 +141,70 @@
 	# (still one atm)
 	devfspv_devices=''
 
-	create_primary_partitions
+	# Creating envelope
+	# Only if one does not already exist (identified by 'method{ lvm }' and by 
+	# the current device in the scheme)
+	physdev=$(cat $dev/device)
+	if ! echo "$normalscheme" | grep -E "method\{ (lvm|crypto) \}" | grep -q "device{ $physdev[[:digit:]]* }" && \
+	   ! echo "$normalscheme" | grep -E "method\{ (lvm|crypto) \}" | grep -q -v "device{"; then
+		normalscheme="$normalscheme${NL}100 1000 10 ext3 \$primary{ } method{ $method }"
+	fi
 
-	create_partitions
+	# Creating the partitions that have no device declared on the default device
+	# and partitions declared on it
+	scheme=$(echo "$normalscheme" | grep -v 'device{')
+	scheme=${scheme}${NL}$(echo "$normalscheme" | grep "device{ $physdev[[:digit:]]* }" )
+	auto_lvm_create_partitions $dev
 
+	# Creating the partitions 

Bug#462396: Multiple disks support for partman-auto-lvm

2008-07-28 Thread Grégory Oestreicher
Hi,

Here is an updated version of the patch, dealing with LVM+crypto, as
suggested by Jérémy Bobbio. This patch also incorporate his modifications
to add the possibility to define a custom LV name (see
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=484272).

This patch have been tested in the following cases, and worked fine each
time. Please note however that Grub installation failed systematically,
and I had to install it by hand from the install CD. I presume it is not
related to my patch, but to my lack of d-i fu when remastering the CD.

Here are the tests :

- with the three base recipes and method lvm ;
- with the three base recipes and method crypto ;
- with method lvm and the four recipes already provided,
  slightly modified to add "lvname{ }" at random places ;
- with method crypto and the four recipes already provided,
  modified as per per the previous point, and replacing
  "method{ lvm }" by "method{ crypto }" in the PV
  declarations ;
- with the first provided recipe, but booting without
  a disk used in the recipe, to test the error
  partman_auto_lvm/no_such_pv; It indeed failed ;
- with a custom recipe containing a VG declaration
  without a PV associated to test the error
  partman_auto_lvm/no_pv_in_vg.

Attached are the patches against each modified package, and the recipes
modified with the "lvname{ }" attributes.

Cheers,
GrégoryIndex: autopartition-crypto
===
--- autopartition-crypto	(révision 54599)
+++ autopartition-crypto	(copie de travail)
@@ -3,10 +3,10 @@
 . /lib/partman/lib/auto-lvm.sh
 . /lib/partman/lib/crypto-base.sh
 
-dev="$1"
+argdev="$1"
 method=crypto
 
-auto_lvm_prepare $dev $method || exit 1
+auto_lvm_prepare $argdev $method || exit 1
 
 found=no
 for dev in $DEVICES/*; do
@@ -28,15 +28,16 @@
 
 		echo dm-crypt > $id/crypto_type
 		crypto_prepare_method "$dev/$id" dm-crypt || exit 1
-		found=yes
-		break
+#		found=yes
+#		break
 	done
-	[ $found = yes ] && break
+#	[ $found = yes ] && break
 done
 
 crypto_check_setup || exit 1
 crypto_setup no || exit 1
 
+pv_devices=''
 # This is a kludge to workaround parted's refusal to allow dm devices
 # to be used for LVM
 for dev in $DEVICES/*; do
@@ -45,7 +46,7 @@
 	[ -f "$dev/device" ] || continue
 
 	# Found it
-	pv_devices=$(cat $dev/device)
+	pv_devices="$pv_devices $(cat $dev/device)"
 
 	# We should have only one partition, but lets be thorough
 	cd $dev
@@ -65,7 +66,7 @@
 	done
 done
 
-auto_lvm_perform || exit 1
+auto_lvm_perform $argdev || exit 1
 
 # partman likes to believe that the virtual devices have a changed
 # partition tableIndex: debian/partman-auto-lvm.templates
===
--- debian/partman-auto-lvm.templates	(révision 54599)
+++ debian/partman-auto-lvm.templates	(copie de travail)
@@ -42,3 +42,19 @@
  the volume group.
  .
  Check /var/log/syslog or see virtual console 4 for the details.
+
+Template: partman-auto-lvm/no_such_pv
+Type: error
+_Description: Inexistant PV declared
+ A Volume Group definition contains a reference to a Physical Volume
+ that is not present. 
+ .
+ Check for you connectivity or the recipe.
+
+Template: partman-auto-lvm/no_pv_in_vg
+Type: error
+_Description: No PV defined for creating VG
+ The recipe contains the definition of a Volume Group without
+ any Physical Volume in it.
+ .
+ Review the recipe.
Index: lib/auto-lvm.sh
===
--- lib/auto-lvm.sh	(révision 54599)
+++ lib/auto-lvm.sh	(copie de travail)
@@ -10,14 +10,48 @@
 	exit 1
 }
 
+# This function was copied from another file (partman-auto/lib/initial_auto).
+# Maybe it should be useful to put it in auto-shared.sh or in base.sh
+dev_to_partman () {
+	local dev_name="$1"
+	local dev
+
+	local mapped_dev_name="$(mapdevfs $dev_name)"
+	if [ -n "$mapped_dev_name" ]; then
+		dev_name="$mapped_dev_name"
+	fi
+
+	for dev in $DEVICES/*; do
+		[ -d "$dev" ] || continue
+
+		# mapdevfs both to allow for different ways to refer to the
+		# same device using devfs, and to allow user input in
+		# non-devfs form
+		if [ "$(mapdevfs $(cat $dev/device))" = "$dev_name" ]; then
+			echo $dev
+		fi
+	done
+}
+
 auto_lvm_prepare() {
-	local dev method size free_size normalscheme target
+	local dev method size free_size normalscheme target extra_devices tmpdev tmpdevdir physdev
 	dev=$1
 	method=$2
 
 	[ -f $dev/size ] || return 1
 	size=$(cat $dev/size)
 
+	extra_devices=''
+	if db_get partman-auto-lvm/extra_devices && [ "$RET" ]; then
+		for tmpdev in "$RET"; do
+			tmpdevdir="$(dev_to_partman $tmpdev)"
+			if [ -d "$tmpdevdir" ]; then
+size=$(($size + $(cat $tmpdevdir/size)))
+extra_devices="$extra_devices $tmpdevdir"
+			fi
+		done
+	fi
+
 	# Be sure the modules are loaded
 	modprobe dm-mod >/dev/null 2>&1 || true
 	modprobe lvm-mod >/dev/null 2>&1 || true
@@ -26,21 +60,32 @@
 		log-output -t update-dev update-dev
 	fi
 
-	target="$(human

Bug#462396: Multiple disks support for partman-auto-lvm

2008-03-20 Thread Frans Pop
On Thursday 20 March 2008, Grégory Oestreicher wrote:
> My bad, it is indeed a remain from the debugging of this script. I will
> generate a new diff file tonight (UTC+1 here) and attach it to the bug.

Not needed. I'll just remove that line from the patch.

Thanks for the quick reply.

Cheers,
FJP




Bug#462396: Multiple disks support for partman-auto-lvm

2008-03-20 Thread Grégory Oestreicher
Hi,

>  +++ b/packages/partman/partman-auto-lvm/lib/auto-lvm.sh
> [...]
> -+  if [ -d $tmpdevdir ]; then
> ++  if [ -d "$tmpdevdir" ]; then
> ++  echo "$tmpdevdir" >> /tmp/lvtest
>
> What is the purpose of this last line? Or is that just something left over
> from your testing?

My bad, it is indeed a remain from the debugging of this script. I will
generate a new diff file tonight (UTC+1 here) and attach it to the bug.

Cheers,
Grégory





Bug#462396: Multiple disks support for partman-auto-lvm

2008-03-20 Thread Frans Pop
Hi,

On Tuesday 19 February 2008, Grégory Oestreicher wrote:
> Le jeudi 24 janvier 2008 16:49, Frans Pop a écrit :
> > Attaching patch and especially the example recipes to ensure they
> > remain available. Recipes will be useful as documentation.
>
> Here is an updated patch that correct a bug when
> partman-auto-lvm/extra_devices is preseeded but none of them exist. This
> patch is against the SVN repository as of today (2008/02/19) and replaces
> the previous one.

Now that Beta1 is out I plan to look at your patch in detail soon.
I noticed the following change in the updated patch:

 +++ b/packages/partman/partman-auto-lvm/lib/auto-lvm.sh
[...]
-+  if [ -d $tmpdevdir ]; then
++  if [ -d "$tmpdevdir" ]; then
++  echo "$tmpdevdir" >> /tmp/lvtest

What is the purpose of this last line? Or is that just something left over 
from your testing?

Cheers,
FJP




Bug#462396: Multiple disks support for partman-auto-lvm

2008-02-19 Thread Grégory Oestreicher
Le jeudi 24 janvier 2008 16:49, Frans Pop a écrit :
> Attaching patch and especially the example recipes to ensure they remain
> available. Recipes will be useful as documentation.

Here is an updated patch that correct a bug when 
partman-auto-lvm/extra_devices is preseeded but none of them exist. This 
patch is against the SVN repository as of today (2008/02/19) and replaces the 
previous one.
Index: partman-auto-lvm/debian/partman-auto-lvm.templates
===
--- partman-auto-lvm/debian/partman-auto-lvm.templates	(révision 51520)
+++ partman-auto-lvm/debian/partman-auto-lvm.templates	(copie de travail)
@@ -42,3 +42,19 @@
  the volume group.
  .
  Check /var/log/syslog or see virtual console 4 for the details.
+
+Template: partman-auto-lvm/no_such_pv
+Type: error
+_Description: Inexistant PV declared
+ A Volume Group definition contains a reference to a Physical Volume
+ that is not present. 
+ .
+ Check for you connectivity or the recipe.
+
+Template: partman-auto-lvm/no_pv_in_vg
+Type: error
+_Description: No PV defined for creating VG
+ The recipe contains the definition of a Volume Group without
+ any Physical Volume in it.
+ .
+ Review the recipe.
Index: partman-auto-lvm/lib/auto-lvm.sh
===
--- partman-auto-lvm/lib/auto-lvm.sh	(révision 51520)
+++ partman-auto-lvm/lib/auto-lvm.sh	(copie de travail)
@@ -10,14 +10,49 @@
 	exit 1
 }
 
+# This function was copied from another file (partman-auto/lib/initial_auto).
+# Maybe it should be useful to put it in auto-shared.sh or in base.sh
+dev_to_partman () {
+	local dev_name="$1"
+	local dev
+
+	local mapped_dev_name="$(mapdevfs $dev_name)"
+	if [ -n "$mapped_dev_name" ]; then
+		dev_name="$mapped_dev_name"
+	fi
+
+	for dev in $DEVICES/*; do
+		[ -d "$dev" ] || continue
+
+		# mapdevfs both to allow for different ways to refer to the
+		# same device using devfs, and to allow user input in
+		# non-devfs form
+		if [ "$(mapdevfs $(cat $dev/device))" = "$dev_name" ]; then
+			echo $dev
+		fi
+	done
+}
+
 auto_lvm_prepare() {
-	local dev method size free_size normalscheme target
+	local dev method size free_size normalscheme target extra_devices tmpdev tmpdevdir physdev
 	dev=$1
 	method=$2
 
 	[ -f $dev/size ] || return 1
 	size=$(cat $dev/size)
 
+	extra_devices=''
+	if db_get partman-auto-lvm/extra_devices && [ "$RET" ]; then
+		for tmpdev in "$RET"; do
+			tmpdevdir="$(dev_to_partman $tmpdev)"
+			if [ -d "$tmpdevdir" ]; then
+echo "$tmpdevdir" >> /tmp/lvtest
+size=$(($size + $(cat $tmpdevdir/size)))
+extra_devices="$extra_devices $tmpdevdir"
+			fi
+		done
+	fi
+
 	# Be sure the modules are loaded
 	modprobe dm-mod >/dev/null 2>&1 || true
 	modprobe lvm-mod >/dev/null 2>&1 || true
@@ -26,21 +61,32 @@
 		log-output -t update-dev update-dev
 	fi
 
-	target="$(humandev $(cat $dev/device)) - $(cat $dev/model)"
+	if [ $extra_devices ]; then
+		for extradev in $dev $extra_devices; do
+			extradev_str="$extradev_str $(cat $extradev/device)"
+		done
+		target="Multiple disks ($extradev_str)"
+	else
+		target="$(humandev $(cat $dev/device)) - $(cat $dev/model)"
+	fi
 	target="$target: $(longint2human $size)"
 	free_size=$(expr 000"$size" : '0*\(..*\)..$') # convert to megabytes
 
 	choose_recipe lvm "$target" "$free_size" || return $?
 
-	auto_init_disk "$dev" || return $?
+	size=0
+	for tmpdev in $dev $extra_devices; do
+		auto_init_disk "$tmpdev" || return $?
+		size=$(($size + $free_size))
 
-	# Check if partition is usable; use existing partman-auto template as we depend on it
-	if [ "$free_type" = unusable ]; then
-		db_input critical partman-auto/unusable_space || true
-		db_go || true
-		return 1
-	fi
-	free_size=$(expr 000"$free_size" : '0*\(..*\)..$') # convert to megabytes
+		# Check if partition is usable; use existing partman-auto template as we depend on it
+		if [ "$free_type" = unusable ]; then
+			db_input critical partman-auto/unusable_space || true
+			db_go || true
+			return 1
+		fi
+	done
+	free_size=$(expr 000"$size" : '0*\(..*\)..$') # convert to megabytes
 
 	decode_recipe $recipe lvm
 
@@ -88,11 +134,6 @@
 		;;
 	esac
 
-	# Creating envelope
-	scheme="$normalscheme${NL}100 1000 10 ext3 \$primary{ } method{ $method }"
-
-	expand_scheme
-
 	clean_method
 
 	# This variable will be used to store the partitions that will be LVM
@@ -101,27 +142,65 @@
 	# (still one atm)
 	devfspv_devices=''
 
-	create_primary_partitions
+	# Creating envelope
+	# Only if one does not already exist (identified by 'method{ lvm }' and by 
+	# the current device in the scheme)
+	physdev=$(cat $dev/device)
+	if ! echo "$normalscheme" | grep "method{ lvm }" | grep -q "device{ $physdev[[:digit:]]* }" && \
+	   ! echo "$normalscheme" | grep "method{ lvm }" | grep -q -v "device{"; then
+		normalscheme="$normalscheme${NL}100 1000 10 ext3 \$primary{ } method{ $method }"
+	fi
 
-	create_partitions
+	# C

Bug#462396: Multiple disks support for partman-auto-lvm

2008-01-24 Thread Frans Pop
On Thursday 24 January 2008, Grégory Oestreicher wrote:
> I finally managed to write a more general patch[1] to have multiple disks
> support in partman-auto-lvm.

Attaching patch and especially the example recipes to ensure they remain 
available. Recipes will be useful as documentation.

Index: partman-auto-lvm/debian/partman-auto-lvm.templates
===
--- partman-auto-lvm/debian/partman-auto-lvm.templates	(révision 50997)
+++ partman-auto-lvm/debian/partman-auto-lvm.templates	(copie de travail)
@@ -42,3 +42,19 @@
  the volume group.
  .
  Check /var/log/syslog or see virtual console 4 for the details.
+
+Template: partman-auto-lvm/no_such_pv
+Type: error
+_Description: Inexistant PV declared
+ A Volume Group definition contains a reference to a Physical Volume
+ that is not present. 
+ .
+ Check for you connectivity or the recipe.
+
+Template: partman-auto-lvm/no_pv_in_vg
+Type: error
+_Description: No PV defined for creating VG
+ The recipe contains the definition of a Volume Group without
+ any Physical Volume in it.
+ .
+ Review the recipe.
Index: partman-auto-lvm/lib/auto-lvm.sh
===
--- partman-auto-lvm/lib/auto-lvm.sh	(révision 50997)
+++ partman-auto-lvm/lib/auto-lvm.sh	(copie de travail)
@@ -10,14 +10,48 @@
 	exit 1
 }
 
+# This function was copied from another file (partman-auto/lib/initial_auto).
+# Maybe it should be useful to put it in auto-shared.sh or in base.sh
+dev_to_partman () {
+	local dev_name="$1"
+	local dev
+
+	local mapped_dev_name="$(mapdevfs $dev_name)"
+	if [ -n "$mapped_dev_name" ]; then
+		dev_name="$mapped_dev_name"
+	fi
+
+	for dev in $DEVICES/*; do
+		[ -d "$dev" ] || continue
+
+		# mapdevfs both to allow for different ways to refer to the
+		# same device using devfs, and to allow user input in
+		# non-devfs form
+		if [ "$(mapdevfs $(cat $dev/device))" = "$dev_name" ]; then
+			echo $dev
+		fi
+	done
+}
+
 auto_lvm_prepare() {
-	local dev method size free_size normalscheme target
+	local dev method size free_size normalscheme target extra_devices tmpdev tmpdevdir physdev
 	dev=$1
 	method=$2
 
 	[ -f $dev/size ] || return 1
 	size=$(cat $dev/size)
 
+	extra_devices=''
+	if db_get partman-auto-lvm/extra_devices && [ "$RET" ]; then
+		for tmpdev in "$RET"; do
+			tmpdevdir="$(dev_to_partman $tmpdev)"
+			if [ -d $tmpdevdir ]; then
+size=$(($size + $(cat $tmpdevdir/size)))
+extra_devices="$extra_devices $tmpdevdir"
+			fi
+		done
+	fi
+
 	# Be sure the modules are loaded
 	modprobe dm-mod >/dev/null 2>&1 || true
 	modprobe lvm-mod >/dev/null 2>&1 || true
@@ -26,21 +60,32 @@
 		log-output -t update-dev update-dev
 	fi
 
-	target="$(humandev $(cat $dev/device)) - $(cat $dev/model)"
+	if [ $extra_devices ]; then
+		for extradev in $dev $extra_devices; do
+			extradev_str="$extradev_str $(cat $extradev/device)"
+		done
+		target="Multiple disks ($extradev_str)"
+	else
+		target="$(humandev $(cat $dev/device)) - $(cat $dev/model)"
+	fi
 	target="$target: $(longint2human $size)"
 	free_size=$(expr 000"$size" : '0*\(..*\)..$') # convert to megabytes
 
 	choose_recipe lvm "$target" "$free_size" || return $?
 
-	auto_init_disk "$dev" || return $?
+	size=0
+	for tmpdev in $dev $extra_devices; do
+		auto_init_disk "$tmpdev" || return $?
+		size=$(($size + $free_size))
 
-	# Check if partition is usable; use existing partman-auto template as we depend on it
-	if [ "$free_type" = unusable ]; then
-		db_input critical partman-auto/unusable_space || true
-		db_go || true
-		return 1
-	fi
-	free_size=$(expr 000"$free_size" : '0*\(..*\)..$') # convert to megabytes
+		# Check if partition is usable; use existing partman-auto template as we depend on it
+		if [ "$free_type" = unusable ]; then
+			db_input critical partman-auto/unusable_space || true
+			db_go || true
+			return 1
+		fi
+	done
+	free_size=$(expr 000"$size" : '0*\(..*\)..$') # convert to megabytes
 
 	decode_recipe $recipe lvm
 
@@ -88,11 +133,6 @@
 		;;
 	esac
 
-	# Creating envelope
-	scheme="$normalscheme${NL}100 1000 10 ext3 \$primary{ } method{ $method }"
-
-	expand_scheme
-
 	clean_method
 
 	# This variable will be used to store the partitions that will be LVM
@@ -101,27 +141,65 @@
 	# (still one atm)
 	devfspv_devices=''
 
-	create_primary_partitions
+	# Creating envelope
+	# Only if one does not already exist (identified by 'method{ lvm }' and by 
+	# the current device in the scheme)
+	physdev=$(cat $dev/device)
+	if ! echo "$normalscheme" | grep "method{ lvm }" | grep -q "device{ $physdev[[:digit:]]* }" && \
+	   ! echo "$normalscheme" | grep "method{ lvm }" | grep -q -v "device{"; then
+		normalscheme="$normalscheme${NL}100 1000 10 ext3 \$primary{ } method{ $method }"
+	fi
 
-	create_partitions
+	# Creating the partitions that have no device declared on the default device
+	# and partitions declared on it
+	scheme=$(echo "$normalscheme" | grep

Bug#462396: Multiple disks support for partman-auto-lvm

2008-01-24 Thread Grégory Oestreicher
Package: partman-auto-lvm
Version: 26
Severity: wishlist

Hi,

I finally managed to write a more general patch[1] to have multiple disks 
support in partman-auto-lvm. It has been successfully tested on x86 with
the  three base recipes of the installer (atomic, home and multi) to
prevent  regressions, and with four homebrew recipes that show what you
can do with  it.

This patch is based on the current (as of 2008/01/24) SVN version of 
partman-auto and partman-auto-lvm (the former needing only a minor 
modification).

A little note about how to use this patch : it is currently available only
 using preseeding. To do this, you must preseed three things :
- partman-auto/expert_recipe_file : nothing new here ;
- partman-auto/disk : the first disk, known from now as the default one.
This  is not mandatory, but may prevent bad surprises ;
- partman-auto-lvm/extra_devices : a new entry in which you declare all
extra  devices that will be available to partman-auto-lvm. You must at
least add  here all devices used in the recipe. Devices not used in the
recipe and  declared here will be silently ignored.

Now here is a description of the four recipes I wrote, and that must cover
all  use cases supported by this patch, with informations about new
attributes  available. All of these recipes use two disks (hda and hdb),
but it should  work with more (a bug in QEmu prevented me from using hdd,
but I hope I'll  can test it on VMWare on another machine).

The first recipe[2] creates two volume groups, vg00 and vg01, respectively
on  hda and hdb, plus a /boot partition. vg00 is used for all LV except
/home,  created on vg01.

In it you can see all the new attributes and their use :
- device{ } is used when declaring a new PV on a specific device. In this 
example, device is /dev/hda, but it can also be a partition as you'll see
in  the third recipe. If device is empty, the default device is used ; -
vgname{ } is used to declare of which VG the PV will be a member of. If 
vgname is empty, the default one will be used ;
- invg{ } is used to create a LV on a specific VG. If invg is empty, it is
 created on the default one.

The second recipe[3] is roughly the same as the first, with a minor change
:  no VG is declared for the LVs except /home. In this case, a default VG
(whose  name is the hostname of the machine) is created and all LVs that
don't have a  invg{ } declaration will be created on this one. Of course,
this VG must have  a free device available to be created on. But more on
the errors after the  recipes.

The third recipe[4] creates a single VG, with a given name, out of two 
partitions on the devices. The rest is simple to understand if you read
the  first two recipes : all LVs are created on a single VG, vg00,
declared on the  PVs hda2 and hdb1. I put a big, red, flashing warning
here : it's not because  you said "use hda2 and hdb1 to create this VG"
that these partitions will be  created. No, you must guess what the
partitions numbers will be based on the  behavior of partman-auto-lvm.
With the recent patch of Frans Pop, default PV  will always be created on
a primary partition. With your recipe it's on your  own to guess.

The fourth recipe[5] is the same as the third, except the PVs will be
added to  the default VG (whose name is the hostname of the machine) as
there's no  vgname and invg attributes.

OK, now about the errors that you can have. I added two debconf templates
in  partman-auto-lvm :
- partman-auto-lvm/no_such_pv : this means that the recipe contains a 
reference to a PV that doesn't exist. This is the case if it's not
declared  in partman-auto-lvm/extra_devices, or if it hasn't been
detected, or if you  didn't guessed correctly the partitioning ;
- partman-auto-lvm/no_pv_in_vg : happens if a VG was declared but that no
PV  could be associated with. For example it could happen if you forgot a
invg{ }  declaration on a LV (which then triggers the need for the default
VG), and if  there's no device without a vgname{ } attribute. But it's not
limited to this  case, and this error is a Bad News.

OK, I think it's all for now. If you have any question or if you found a
bug  I'll happily try to help. I hope you'll find this patch useful.

Cheers,
Grégory

[1] : http://ogre.nerim.net/d-i/partman-auto-lvm_20080124.diff
[2] : http://ogre.nerim.net/d-i/testrecipe-01
[3] : http://ogre.nerim.net/d-i/testrecipe-02
[4] : http://ogre.nerim.net/d-i/testrecipe-03
[5] : http://ogre.nerim.net/d-i/testrecipe-04