Bug#462396: Multiple disks support for partman-auto-lvm
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
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
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
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
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
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
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
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
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
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
>> 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
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
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
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
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
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
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
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
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
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
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
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