[parted-devel] "linux: use devicemapper task name instead of device node name" causes dmraid breakage

[Colin Watson]

On Mon, May 17, 2010 at 10:30:32AM +0200, Hans de Goede wrote:
> On 05/17/2010 12:35 AM, Phillip Susi wrote:
>> I have traced a severe regression in Ubuntu 10.04 for the commit with
>> the title in the subject.  The finger print is 162ac17e.  This change
>> caused a 'p' to be inserted into the dev node for partitions on dmraid
>> devices, but dmraid does not use the p, and partman does not expect
>> there to be a p, so it could not find the device after creating it.
>>
>> For now I have reverted this patch in Ubuntu to resolve the issue, but
>> I'm not sure that is the best solution given that I do not fully
>> understand the purpose of this change.  Perhaps Hans could explain it a
>> bit further and reason out the correct way to fix it.
>
> With newer lvm2 versions the files under /dev/mapper are no longer created
> by the lvm tools, but by udev (as they should be). As a consequence they
> are no longer device nodes but symlinks.
>
> IOW /dev/mapper/foo no longer is a device node, but a symlink to ie
> /dev/dm-0

IIRC the member of udev upstream who works for us disagrees with this
naming scheme; Scott, could you comment?

> As parted resolves symlinks before operating on them this means that
> new partitions on /dev/mapper/foo would be named /dev/mapper/dm-0p#
>
> So the patch you blame for the regression you are seeing changes the
> code to determine the name for the partition mapping to use the
> devicemapper name (which is the foo part of /dev/mapper/foo)
> of the devicemapper node operated on rather then the devicenode name
> (/dev/dm-0).
>
> Note that before this change the partition name would be
> /dev/mapper/dm-0p#
>
> IOW the p is still there, the code before this patch called
> _device_get_part_path to determine the partition name, and that has
> the following snippet in it:
>
>         } else if (dev->type == PED_DEVICE_DAC960
>                         || dev->type == PED_DEVICE_CPQARRAY
>                         || dev->type == PED_DEVICE_ATARAID
>                         || dev->type == PED_DEVICE_DM
>                         || isdigit (dev->path[path_len - 1]))
>                 snprintf (result, result_len, "%sp%d", dev->path, num);
>
> Notice the p in the format string. The new code does not change
> this behavior, as the code in question only operates on devices
> with a type of PED_DEVICE_DM, and thus the new code always
> inserts the p.
>
> So if reverting this patch results in parted creating partitions
> without an extra p in the name, then that means that Ubuntu's parted
> is carying a patch changing the behavior of _device_get_part_path()

Yes, this appears to be the case.

> Note that upstream dmraid (the cmdline tool), will by default also
> create devicemapper mappings for partitions when activating a
> RAID set, and these mappings also have a p in the name.
>
> Likewise kpartx (which one should use rather then dmraid's build
> in partition code as kpartx can handle gpt labels too and dmraid
> cannot), also creates partition mappings with a p in the name
> by default.
>
> Thus if partman expects there to be no p in the name, and other tools
> in Ubunutu don't add the p when activating partitions during boot, then
> that means that Ubuntu is deviating from various upstreams here (3 to
> be precise) all 3 of which are kept in careful sync wrt their behavior
> when it comes to partition names. And the fix here is not to revert
> this patch, but to drop the patches from the Ubuntu packages in
> question so that they are not unnecessarily deviating from upstream,
> and to fix partman to expect the p in the partition device node name.

I don't mind resyncing for our development release provided that
everyone actually agrees.  For a stable update to 10.04, we're probably
better off making things work with the naming scheme in use elsewhere in
that release; I don't want to have to go around patching GRUB and the
like as well.

-- 
Colin Watson                                       [cjwatson at ubuntu.com]