On Tue, Aug 24, 2010 at 07:30:01PM +0300, Modestas Vainius wrote: > current versions of grub-probe in squeeze and sid have problems with DMRAID > partitions. The problems are demonstrated below. As a result, grub > lenny->squeeze upgrade failed leaving my system without a bootloader. While > grub-common/lenny had no support for DMRAID at all, it would be a pity to > release current grub-common with half working DMRAID support. Therefore, it > would be great if the bug was fixed in squeeze. A proposed patch is attached > to this mail. Please forward the bug and/or the patch upstream.
Thanks for this. Since I'm an upstream developer ... :-) > From: Modestas Vainius <mo...@debian.org> > Subject: fix DMRAID support in grub-probe > Forwarded: no > Last-Update: 2010-08-24 > > /dev/mapper/* are symlinks to /dev/dm-* and > convert_system_partition_to_system_disk() checks against fully a canonicalized > real path. Therefore, it should look for dm-* when identifying DMRAID devices. > Likewise, return the same form (/dev/mapper/* or /dev/dm-$minor) which was > asked for in the input. This is needed since strcmp() is typically used by the > callers to check if returned device is the same as given. I disagree with this approach. In general, we should be treating /dev/mapper/* as the canonical name, despite the symlink layout. For one, those device names convey much more information than the terse /dev/dm-*; for another, it's consistent with other GNU packages, such as GNU Parted: commit c1eb485b9fd8919e18f192d678bc52b0488e6ee0 Author: Hans de Goede <hdego...@redhat.com> Date: Tue Apr 6 15:57:18 2010 +0200 libparted: don't canonicalize /dev/mapper paths Besides fixing the issue displayed by libparted/tests/symlink.c, this has the added advantage that the output of parted p on one of these devices now says: "Disk /dev/mapper/BigVol2-lv_iscsi_disk2: 34.4GB" Which is a lot more user friendly then the output before this patch: "Disk /dev/dm-6: 34.4GB" * libparted/device.c (ped_device_get): Don't canonicalize names that start with "/dev/mapper/". * NEWS (Bug fixes): Mention it. Thanks to Ales Kozumplik for the analysis. Details in <http://bugzilla.redhat.com/577824>. I've committed a patch upstream which implements this: 2010-09-17 Colin Watson <cjwat...@ubuntu.com> Fix DM-RAID probing with recent versions of device-mapper udev rules. * grub-core/kern/emu/hostdisk.c (read_device_map): Don't canonicalise device paths under /dev/mapper/. (convert_system_partition_to_system_disk): Compare the uncanonicalised path to /dev/mapper/ rather than the canonicalised path, since device nodes under /dev/mapper/ are often symlinks. > While at it, the patch also fixes a memory pool leak in dm_* related code. > devmapper library complains loudly about memory pool leaks. [...] > - static struct dm_tree *tree = NULL; > + struct dm_tree *tree = NULL; It's a shame to have to repeatedly reconstruct the dm_tree, but I suppose we have little choice if libdevmapper is going to complain loudly at us. I've committed a patch upstream, based on the general idea of your patch but I rearranged things so that all the freeing happens on a single exit path from the function: 2010-09-17 Colin Watson <cjwat...@ubuntu.com> * grub-core/kern/emu/hostdisk.c (convert_system_partition_to_system_disk): Fix devmapper memory pool leak. Reported and based on patch by: Modestas Vainius. I'll backport these two patches in my next upload. Thanks, -- Colin Watson [cjwat...@ubuntu.com] -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org