Re: [PATCH] add support for dmraid devices

[Pavel Roskin]

On Wed, 2009-06-17 at 16:08 +0200, Felix Zielcke wrote:
> add support for dmraid devices

That's good.  I have a system with a PDC RAID, and although I only have
one drive connected, GRUB2 won't install on it.  I'm looking forward to
testing your patch on that hardware.

Unfortunately, your patch doesn't compile as is.  That hanging "12" in
grub_util_is_dmraid() is not needed.  Also please avoid adding trailing
whitespace.  STGit warns about it.  "hostdisc" should be spelled
"hostdisk".

What is "cediideh"?  Actually, on the system I mentioned, root is
mounted on /dev/mapper/pdc_dieaihahp2.  If "cediideh" is supposed to
represent all weird names 8 characters long, let's use numbers
"12345678".

Even after fixing the compile error, I'm getting a warning:

util/hostdisk.c: In function 'grub_util_biosdisk_get_grub_dev':
util/hostdisk.c:916: warning: 'disk' may be used uninitialized in this
function
util/hostdisk.c:916: note: 'disk' was declared here

Your patch removed the code where 'disk' is initialized:
disk = grub_disk_open (name);

Yet grub_disk_close() is still there.  Removing it fixes the warning,
but I'd like you to recheck the patch.  Apparently the 'disk' variable
was used for different purposes throughout the function.  While at that,
it would be great to avoid variable shadowing too and keep variables in
the innermost possible scope.

Finally, the test results, apparently negative:

address@hidden grub2.git]# mount
/dev/mapper/pdc_dieaihahp2 on / type ext4 (rw)
/proc on /proc type proc (rw)
sysfs on /sys type sysfs (rw)
devpts on /dev/pts type devpts (rw,gid=5,mode=620)
/dev/mapper/pdc_dieaihahp1 on /boot type ext3 (rw)
tmpfs on /dev/shm type tmpfs (rw)
debugfs on /sys/kernel/debug type debugfs (rw)
none on /proc/sys/fs/binfmt_misc type binfmt_misc (rw)
address@hidden grub2.git]# grub-install /dev/mapper/pdc_dieaihah --recheck
grub-probe: error: Cannot find a GRUB drive
for /dev/mapper/pdc_dieaihahp1.  Check your device.map.

Auto-detection of a filesystem module failed.
Please specify the module with the option `--modules' explicitly.
address@hidden grub2.git]# cat /boot/grub/device.map 
(fd0)   /dev/fd0
(hd0)   /dev/sda
address@hidden grub2.git]#

I think we need a list of possible dmraid names.  There we could go
through the list in a loop.  That applies to grub_util_is_dmraid() and
other places in the code.

-- 
Regards,
Pavel Roskin