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

Bug#468832: multipath support

On Saturday 01 March 2008, Guido Günther wrote:
> attached is a first version of multipath support for grub-install to get
> some comments. It basically uses the sataraid code.

In general I'm not sure that this approach is correct. In the case of SATA
RAID, the BIOS actually knows what the RAID devices are and the RAID device
_is_ what you select in the BIOS as the "disk" to be booted from.

I somehow doubt that the same is the case for multipath. If it is not, the
dialogs (and maybe even the way grub gets installed) should probably be
fundamentally different?

Also, I suspect that multipath is not i386/amd64-specific, and is probably
quite interesting for other (mini system) arches. It would be good to contact
porters about supporting multipath setups in their boot loaders

> The current code has:
> - if ... [ $frgrubroot -gt 0 ] && [ -e $ROOT$frdev$frbootpart ]; then
> which I changed to
> + if ... [ $frgrubroot -ge 0 ] && [ -e $ROOT$frdev$frbootpart ]; then
> since grub starts counting from zero. I wonder why sataraid can only
> have partition numbers > 0?

I think you are correct. They are basically just sanity checks.
I have committed this fix to SVN. As it is a bugfix, it should not be mixed
in with your other changes.

Some comments on the patch.

+ GRUB is always installed to the master boot record (MBR) of the multipath
+ device. It is also assumed that that this device is listed as the first hard
+ device in the boot order defined in the system's BIOS setup.

"first hard _device_" is a meaningless term IMO.

+ An error occurred while setting up GRUB for the multipathed device.

I have seen you use "multipathed" before and every time my reaction was
"ugh, ugly". Why not just use "multipath" instead? IMO the meaning remains

+if [ "$frtype" = multipath ]; then
+    $chroot $ROOT umount /proc

Should use tab for indentation.

+		if echo "$disc_offered" | 	   \

Redundant tab before line continuation.

+        	apt-install dmsetup
+        	$chroot $ROOT mount /proc
+        	$chroot $ROOT dmsetup mknodes
+	   	db_subst grub-installer/$frtype GRUBROOT $ROOT$frdev$frbootpart
-	    	error "Unsupported architecture for SATA RAID installation"
+	    	error "Unsupported architecture for SATA RAID/multipath installation"

Spaces before tabs in indentation.
(There were quite a few of those already in SVN, which I have cleaned up;
you may have to update your patch for that.)

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

Reply to: