[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [RFC] Some love for partman-md v3



On Wednesday 04 June 2008, Jérémy Bobbio wrote:
>    Two questions popped up in my mind after this change:
>      * Would it be desirable to activate Volume Groups by default, like
>        it would now be done for RAID devices?

Since it basically only requires loading dm-mod and running 'vgscan -a y' 
I don't see any real objection.

>      * How are we going to cope with really twisted setups if we ever
>        fix #451535?

Maybe just detect them and bail out. There's only so much you can support 
out of the box. Also, once you've detected for example an encrypted 
device, there's not that much use in checking what's inside it.

Currently, preexisting encrypted partitions are basically happily ignored 
by guided partitioning.

> Comments welcome, as usual. :)

> +++ b/packages/mdcfg/debian/changelog
> Remove the attempt to load the anciant module "md".

s/anciant/ancient/

> +++ b/packages/partman/partman-base/debian/changelog
> +  * Instead of skipping every MD devices during partman
> initialization, we +    now only skip the ones that are deactivated. 
> (Closes: #475479)

s/every/all/

> +++ b/packages/partman/partman-base/init.d/parted
> +is_unactive_md() {

s/unactive/inactive/ (also in call!)

> +++ b/packages/partman/partman-md/init.d/md-devices
> +# Supported schemes: RAID 0, RAID 1, RAID 5

This comment does not really make sense anymore. What's supported is 
determined by what modules are available and the menu we offer to create 
devices, not by module loading.

> +++ b/packages/partman/partman-md/debian/changelog
> +    (for example, by automatic partitioning).

s/by/during/

> +++ b/packages/partman/partman-md/lib/md-remove.sh

In general I wonder if this should not be delayed to post-Lenny.
It may be better for now to just warn that RAID devices have been
detected and that those should be removed manually before restarting 
guided partitioning.

One thing I don't like is that we don't first present an overview of _all_ 
affected RAID devices.
Also, the template you are reusing is less suitable as it is designed as a 
confirmation of a specifically selected action, not for something that 
could be the unintended consequense of guided partitioning.

> @@ -0,0 +1,60 @@
> +# Wipes any traces of an active MD on the given device
> +device_remove_md() {
> +	local dev md_dev md_devs used_parts type device_dir
> +	dev="$1"
> +	cd $dev
> +
> +	realdev=$(mapdevfs "$(cat $dev/device)")
> +	md_devs=$(sed -n -e \
> +		"s,^\(md[0-9]*\) : active raid[0-9]*
> .*${realdev#/dev/}[^[]*\[[0-9]\].*,/dev/\1,p" \
> +		/proc/mdstat) 

I'd think that _all_ RAID devices should be removed, including inactive 
ones. If only to clear the superblocks.

> +	# Make sure that parted has no stale info on the MD
> +	restart=""
> +	for md_dev in $md_devs; do
> +		device_dir="$DEVICES/$(echo $md_dev | sed -e 's,/,=,g')"
> +		if ! [ -d "$device_dir" ]; then
> +			continue
> +		fi
> +		rm -rf $device_dir
> +		restart=1
> +	done

Is this necessary? Isn't this done automatically during the restart?
If we rely on that, a comment that explains it is probably useful.

> +++ b/packages/partman/partman-partitioning/debian/changelog
> +  * In create_new_label(), we now remove MD devices like it was
> already done +    for LVM.  As partman-md now scans for existing MD
> devices during partman +    initialization, it is required in order to
> make automatic partitioning +    working on disks which was previously
> used for an MD device. +    Requires partman-md (>= 42).

s/like it was already done for LVM/similar to what's already done for LVM/
s/automatic/guided/
s/working on disks which was/work on a disk that was/

Attachment: signature.asc
Description: This is a digitally signed message part.


Reply to: