Bug#375491: yet another lvm_removal_on_demand patch suggestion
David Härdeman wrote:
I've attached a patch which details what this could look like...
And here's an updated patch. This one is tested and works with one
exception: if you do an automated partman-auto-lvm install two times in
a row, the parted server seems to become confused and the manual
partitioning screen is messed up thereafter.
This still needs fixing but I'm unsure of what goes wrong and why (note
that the idea of a disappearing disk, which is what a VG/LV removal
looks like to parted, has not been tried yet in partman-* as far as I
know).
I've CC:ed Colin Watson who perhaps knows a bit more than me about
parted_server?
Latest patch attached...(the section under the Make sure that parted
has no stale LVM info is what needs to be checked)...
Regards,
David
Index: partman-auto/debian/partman-auto.templates
===
--- partman-auto/debian/partman-auto.templates (revision 40312)
+++ partman-auto/debian/partman-auto.templates (working copy)
@@ -27,6 +27,21 @@
use the guided partitioning tool, you will still have a chance later to
review and customise the results.
+Template: partman-auto/purge_lvm_from_device
+Type: boolean
+Default: false
+_Description: Remove all Logical Volume Configuration?
+ Before creating any partitions, we need to remove all existing
+ Logical Volumes and Volume Groups from the disk.
+ .
+ WARNING This will erase all data on LVM Partitions
+
+Template: partman-auto/pv_on_device
+Type: error
+_Description: Existing physical volume on the selected device
+ The device you selected already contains one or more physical volumes. It
+ is not possible to automatically partition this device using LVM.
+
Template: partman-auto/disk
Type: string
# Only used for preseeding.
Index: partman-auto/auto-shared.sh
===
--- partman-auto/auto-shared.sh (revision 40234)
+++ partman-auto/auto-shared.sh (working copy)
@@ -1,9 +1,92 @@
## this file contains a bunch of shared code between partman-auto
## and partman-auto-lvm.
+# Wipes any traces of LVM from a disk
+# Normally you wouldn't want to use this function,
+# but wipe_disk() which will also call this function.
+lvm_wipe_disk() {
+ local dev realdev vg pvs pv lv tmpdev
+ dev=$1
+
+ if [ ! -e /lib/partman/lvm_tools.sh ]; then
+ return 0
+ fi
+
+ . /lib/partman/lvm_tools.sh
+
+ # Check if the device already contains any physical volumes
+ realdev=$(mapdevfs $(cat $dev/device))
+ if ! pv_on_device $realdev; then
+ return 0
+ fi
+
+ # Ask for permission to erase LVM volumes
+ db_input critical partman-auto/purge_lvm_from_device
+ db_go || return 1
+ db_get partman-auto/purge_lvm_from_device
+ if [ $RET != true ]; then
+ db_input critical partman-auto/pv_on_device || true
+ db_go || true
+ return 1
+ fi
+
+ # Check all VG's
+ for vg in $(vg_list); do
+ pvs=$(vg_list_pvs $vg)
+
+ # Only deal with VG's on the selected disk
+ if ! $(echo $pvs | grep -q $realdev); then
+ continue
+ fi
+
+ # Make sure the VG don't span any other disks
+ if $(echo -n $pvs | grep -q -v $realdev); then
+ log-output -t partman-auto-lvs vgs
+ db_input critical partman-auto/pv_on_device || true
+ db_go || true
+ return 1
+ fi
+
+ # Remove LV's from the VG
+ for lv in $(vg_list_lvs $vg); do
+ lv_delete $vg $lv
+ done
+
+ # Remove the VG and its PV's
+ vg_delete $vg
+ for pv in $pvs; do
+ pv_delete $pv
+ done
+ done
+
+ # Make sure that parted has no stale LVM info
+ for tmpdev in $DEVICES/*; do
+ realdev=$(cat $tmpdev/device)
+
+ if ! $(echo $realdev | grep -q /dev/mapper/); then
+ continue
+ fi
+
+ if [ -b $realdev ]; then
+ continue
+ fi
+
+ cd $tmpdev
+ open_dialog CLOSE $realdev
+ read_line response
+ close_dialog
+
+ rm -rf $tmpdev
+ done
+
+ return 0
+}
+
wipe_disk() {
cd $dev
+ lvm_wipe_disk $dev || return 1
+
open_dialog LABEL_TYPES
types=$(read_list)
close_dialog
Index: partman-auto-lvm/debian/partman-auto-lvm.templates
===
--- partman-auto-lvm/debian/partman-auto-lvm.templates (revision 40234)
+++ partman-auto-lvm/debian/partman-auto-lvm.templates (working copy)
@@
Bug#375491: yet another lvm_removal_on_demand patch suggestion
On Wed, Aug 30, 2006 at 08:39:58AM +0200, David Härdeman wrote: And here's an updated patch. This one is tested and works with one exception: if you do an automated partman-auto-lvm install two times in a row, the parted server seems to become confused and the manual partitioning screen is messed up thereafter. This still needs fixing but I'm unsure of what goes wrong and why (note that the idea of a disappearing disk, which is what a VG/LV removal looks like to parted, has not been tried yet in partman-* as far as I know). I've CC:ed Colin Watson who perhaps knows a bit more than me about parted_server? Latest patch attached...(the section under the Make sure that parted has no stale LVM info is what needs to be checked)... Nevermind, I found a fix for this problem. I've committed the fixed patch to SVN today. Regards, David
Bug#375491: yet another lvm_removal_on_demand patch suggestion
David Härdeman wrote:
On Thu, Aug 24, 2006 at 09:27:49PM +0200, Ronny Aasen wrote:
Index: debian/partman-auto-lvm.templates
===
--- debian/partman-auto-lvm.templates(revision 40192)
+++ debian/partman-auto-lvm.templates(working copy)
@@ -37,6 +37,15 @@
You can choose to ignore this warning, but that may result in a
failure to
reboot the system after the installation is completed.
+Template: partman-auto-lvm/purge_lvm_from_device
+Type: boolean
+Default: false
+_Description: Remove all Logical Volume Configuration?
+ Before creating any Logical Volume Groups, we need to remove all
exsisting
^
existing
+ Logical Volumes and Volume Groups from the disk we are partitioning.
In general, I think phrases such as we should be avoided, but it
would be better if Christian/Kamion/joeyh could review the template.
+ .
+ WARNING This will erase all data on LVM Partitions
This will erase all data on the LVM volumes and on the disk?
+
Template: partman-auto-lvm/pv_on_device
Type: error
_Description: Existing physical volume on the selected device
Index: autopartition-lvm
===
--- autopartition-lvm(revision 40192)
+++ autopartition-lvm(working copy)
@@ -26,10 +26,52 @@
log-output -t update-dev update-dev
fi
+
# Check if the device already contains any physical volumes
realdev=$(mapdevfs $(cat $dev/device))
if pv_on_device $realdev; then
-bail_out pv_on_device
+# Ask for mermission to erase LVM volumes +db_set
partman-auto-lvm/purge_lvm_from_device false
I think db_set will override preseedings (as Geert said)...
+db_input critical partman-auto-lvm/purge_lvm_from_device
+db_go
should be db_go || exit 1?
if you say so :)
+db_get partman-auto-lvm/purge_lvm_from_device
+if [ $RET = true ] ;then +
+targetvg=
+#what volume groups is on any of the the disk partitions.
+all_volume_groups=$(vg_list)
+#we only care about vg's on the selected disk
+for vg in $all_volume_groups
+do
+if [ $(vg_list_pvs $vg | grep -c $realdev) != 0
] ; then
this looks fishy...will it actually return zero? Shouldn't this be:
if $(vg_list_pvs $vg | grep -q $realdev); then
grep -c returns a number of lines matching. 0 if none do. but -q will
work as well and probably be a cleaner way to do it
+targetvg=${targetvg} $vg
weird quotation marks.targetvg=$targetvg $vg would be expected here
+fi
+done
+for vg in $targetvg
+do
+#make sure the volume groups on the target disk
don't span any other disks.
very good idea
+if [ $(lvm_get_info vgs pv_count $vg) != 1 ] ; then
+log-output -t partman-auto-lvs vgs
+bail_out pv_on_device
+fi
+done
+
+#it should now be safe to remove the vg's on the target disks
+
+#remove lv's from the target vg's.
+for vg in $targetvg
+do
+for lv in $(vg_list_lvs $vg)
+do
+#remove the logical volumes on the volume group
+ lv_delete $vg $lv
+done
+#remove the volume group
+vg_delete $vg
+done
+else
+bail_out pv_on_device
+fi
in addition to lv_delete and vg_delete, a pv_delete should also be
done. It needs to be added to lvm-tools.sh, and should call pvremove,
I've just added it myself :)
fi
choose_recipe $free_size lvm || exit $?
After having looked at this, I'm wondering whether this shouldn't be
added to wipe_disk() in partman-auto/auto-shared.sh instead. If it
was, that would mean that all methods would be able to warn that they
are about to remove LVM VG/LV/PV's before actually doing so.
Of course, it would need to include a check whether the lvm-tools.sh
script is present before doing so, but I think it would be a good idea
in general.
I've attached a patch which details what this could look like...
Thanks for your hard work on this and sorry that I've jumped in with
my comments so late :)
Regards,
David
As long as it happens, it's good :)
thanks for reviewing and fixing, hope removal of lvm volume will be
included in next partman :)
Regards
Ronny
--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#375491: yet another lvm_removal_on_demand patch suggestion
On Thu, Aug 24, 2006 at 09:27:49PM +0200, Ronny Aasen wrote:
Index: debian/partman-auto-lvm.templates
===
--- debian/partman-auto-lvm.templates (revision 40192)
+++ debian/partman-auto-lvm.templates (working copy)
@@ -37,6 +37,15 @@
You can choose to ignore this warning, but that may result in a failure to
reboot the system after the installation is completed.
+Template: partman-auto-lvm/purge_lvm_from_device
+Type: boolean
+Default: false
+_Description: Remove all Logical Volume Configuration?
+ Before creating any Logical Volume Groups, we need to remove all exsisting
^
existing
+ Logical Volumes and Volume Groups from the disk we are partitioning.
In general, I think phrases such as we should be avoided, but it would
be better if Christian/Kamion/joeyh could review the template.
+ .
+ WARNING This will erase all data on LVM Partitions
This will erase all data on the LVM volumes and on the disk?
+
Template: partman-auto-lvm/pv_on_device
Type: error
_Description: Existing physical volume on the selected device
Index: autopartition-lvm
===
--- autopartition-lvm (revision 40192)
+++ autopartition-lvm (working copy)
@@ -26,10 +26,52 @@
log-output -t update-dev update-dev
fi
+
# Check if the device already contains any physical volumes
realdev=$(mapdevfs $(cat $dev/device))
if pv_on_device $realdev; then
- bail_out pv_on_device
+ # Ask for mermission to erase LVM volumes
+ db_set partman-auto-lvm/purge_lvm_from_device false
I think db_set will override preseedings (as Geert said)...
+db_input critical partman-auto-lvm/purge_lvm_from_device
+db_go
should be db_go || exit 1?
+db_get partman-auto-lvm/purge_lvm_from_device
+ if [ $RET = true ] ;then
+
+ targetvg=
+ #what volume groups is on any of the the disk partitions.
+ all_volume_groups=$(vg_list)
+ #we only care about vg's on the selected disk
+ for vg in $all_volume_groups
+ do
+ if [ $(vg_list_pvs $vg | grep -c $realdev) != 0
] ; then
this looks fishy...will it actually return zero? Shouldn't this be:
if $(vg_list_pvs $vg | grep -q $realdev); then
+ targetvg=${targetvg} $vg
weird quotation marks.targetvg=$targetvg $vg would be expected
here
+ fi
+ done
+ for vg in $targetvg
+ do
+ #make sure the volume groups on the target disk don't
span any other disks.
very good idea
+ if [ $(lvm_get_info vgs pv_count $vg) != 1 ] ;
then
+ log-output -t partman-auto-lvs vgs
+ bail_out pv_on_device
+ fi
+ done
+
+ #it should now be safe to remove the vg's on the target disks
+
+ #remove lv's from the target vg's.
+ for vg in $targetvg
+ do
+ for lv in $(vg_list_lvs $vg)
+ do
+ #remove the logical volumes on the volume group
+ lv_delete $vg $lv
+ done
+ #remove the volume group
+ vg_delete $vg
+ done
+ else
+ bail_out pv_on_device
+ fi
in addition to lv_delete and vg_delete, a pv_delete should also be done.
It needs to be added to lvm-tools.sh, and should call pvremove, I've
just added it myself :)
fi
choose_recipe $free_size lvm || exit $?
After having looked at this, I'm wondering whether this shouldn't be
added to wipe_disk() in partman-auto/auto-shared.sh instead. If it was,
that would mean that all methods would be able to warn that they are
about to remove LVM VG/LV/PV's before actually doing so.
Of course, it would need to include a check whether the lvm-tools.sh
script is present before doing so, but I think it would be a good idea
in general.
I've attached a patch which details what this could look like...
Thanks for your hard work on this and sorry that I've jumped in with my
comments so late :)
Regards,
David
Index: auto-shared.sh
===
--- auto-shared.sh (revision 40234)
+++ auto-shared.sh (working copy)
@@ -1,9 +1,73 @@
## this file contains a bunch of shared code between partman-auto
## and partman-auto-lvm.
+lvm_wipe_disk() {
+ local realdev targetvgs all_vgs vg lv tmpdev
+
+ if [ ! -e /lib/partman/lvm_tools.sh ]; then
+ return 1
+ fi
+
+ .
Bug#375491: yet another lvm_removal_on_demand patch suggestion
On Thu, Aug 24, 2006 at 09:27:49PM +0200, Ronny Aasen wrote: snip/ this works interactivly, and can preseed it as normal i expect. ^^^ Is it 'can preseed' or 'can be preseeded' ? hope someone can review and judge this Ronny --- autopartition-lvm (revision 40192) +++ autopartition-lvm (working copy) @@ -26,10 +26,52 @@ log-output -t update-dev update-dev fi + Why two blank lines? ( Please avoid inserting blank lines ) # Check if the device already contains any physical volumes realdev=$(mapdevfs $(cat $dev/device)) if pv_on_device $realdev; then - bail_out pv_on_device + # Ask for mermission to erase LVM volumes + db_set partman-auto-lvm/purge_lvm_from_device false I think that overwrites preseeding. +db_input critical partman-auto-lvm/purge_lvm_from_device +db_go +db_get partman-auto-lvm/purge_lvm_from_device + if [ $RET = true ] ;then + + targetvg= I do prefer an implicied to indicate an empty string. + #what volume groups is on any of the the disk partitions. [ more added lines ] But no comment Geert Stappers Trying to say: Your patch has been seen ... -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#375491: yet another lvm_removal_on_demand patch suggestion
here is attached my 3rd attempth for lvm removal on installations.
It's basicaly Frans Pops suggestions. with a new template
if lvm is detected on the disk,
ask user if it's ok to delete them.
if yes ; delete
if no ; same error as before.
If this is acceptable ill see about adding something to the error
description.
this works interactivly, and can preseed it as normal i expect.
It still errors out if the VG's on the selected disk span multiple disks.
I also tried to reuse some of the variables an not create a dozen when
it was not needed.
hope someone can review and judge this
Ronny
Index: debian/partman-auto-lvm.templates
===
--- debian/partman-auto-lvm.templates (revision 40192)
+++ debian/partman-auto-lvm.templates (working copy)
@@ -37,6 +37,15 @@
You can choose to ignore this warning, but that may result in a failure to
reboot the system after the installation is completed.
+Template: partman-auto-lvm/purge_lvm_from_device
+Type: boolean
+Default: false
+_Description: Remove all Logical Volume Configuration?
+ Before creating any Logical Volume Groups, we need to remove all exsisting
+ Logical Volumes and Volume Groups from the disk we are partitioning.
+ .
+ WARNING This will erase all data on LVM Partitions
+
Template: partman-auto-lvm/pv_on_device
Type: error
_Description: Existing physical volume on the selected device
Index: autopartition-lvm
===
--- autopartition-lvm (revision 40192)
+++ autopartition-lvm (working copy)
@@ -26,10 +26,52 @@
log-output -t update-dev update-dev
fi
+
# Check if the device already contains any physical volumes
realdev=$(mapdevfs $(cat $dev/device))
if pv_on_device $realdev; then
- bail_out pv_on_device
+ # Ask for mermission to erase LVM volumes
+ db_set partman-auto-lvm/purge_lvm_from_device false
+db_input critical partman-auto-lvm/purge_lvm_from_device
+db_go
+db_get partman-auto-lvm/purge_lvm_from_device
+ if [ $RET = true ] ;then
+
+ targetvg=
+ #what volume groups is on any of the the disk partitions.
+ all_volume_groups=$(vg_list)
+ #we only care about vg's on the selected disk
+ for vg in $all_volume_groups
+ do
+ if [ $(vg_list_pvs $vg | grep -c $realdev) != 0
] ; then
+ targetvg=${targetvg} $vg
+ fi
+ done
+ for vg in $targetvg
+ do
+ #make sure the volume groups on the target disk don't
span any other disks.
+ if [ $(lvm_get_info vgs pv_count $vg) != 1 ] ;
then
+ log-output -t partman-auto-lvs vgs
+ bail_out pv_on_device
+ fi
+ done
+
+ #it should now be safe to remove the vg's on the target disks
+
+ #remove lv's from the target vg's.
+ for vg in $targetvg
+ do
+ for lv in $(vg_list_lvs $vg)
+ do
+ #remove the logical volumes on the volume group
+ lv_delete $vg $lv
+ done
+ #remove the volume group
+ vg_delete $vg
+ done
+ else
+ bail_out pv_on_device
+ fi
fi
choose_recipe $free_size lvm || exit $?
