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
+
+       . /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
+               # Ask for mermission 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
+
+               # Find VG's on the selected disk
+               all_vgs=$(vg_list)
+               targetvgs=""
+               for vg in $all_volume_groups; do        
+                       if $(vg_list_pvs "$vg" | grep -q "$realdev"); then
+                               targetvgs="$targetvgs $vg"
+                       fi
+               done
+
+               # Make sure the VG's on the target disk don't span any other 
disks.
+               for vg in $targetvgs; do
+                       vg_get_info $vg
+                       if [ "$PVS" -gt "1" ]; then
+                               log-output -t partman-auto-lvs vgs
+                               db_input critical partman-auto/pv_on_device || 
true
+                               db_go || true
+                               return 1
+                       fi
+               done
+
+               # Remove LV's from the target VG's first and the VG's next.
+               for vg in $targetvgs; do
+                       for lv in $(vg_list_lvs $vg); do
+                               lv_delete $vg $lv
+                       done
+                       vg_delete $vg
+
+                       # Make sure that parted has no stale VG info
+                       for tmpdev in $DEVICES/*; do
+                               if [ "${tmpdev#/dev/mapper/}" = "$tmpdev" ]; 
then
+                                       continue
+                               elif [ ! -b "$(cat $tmpdev/device)" ]; then
+                                       rm -rf $tmpdev
+                               fi
+                       done
+               done
+       fi
+       return 0
+}
+
 wipe_disk() {
        cd $dev
 
+       lvm_wipe_disk || true
+
        open_dialog LABEL_TYPES
        types=$(read_list)
        close_dialog
Index: debian/partman-auto.templates
===================================================================
--- debian/partman-auto.templates       (revision 40312)
+++ 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.

Reply via email to