Bug#375491: yet another lvm_removal_on_demand patch suggestion

[stappers@stappers.nl (Geert Stappers)]

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 ...