[Natty] Gparted duplicates dmraid partition devices

I think I found the cause. DMRaid.cc: DMRaid::create_dev_map_entries() runs dmraid -ay -P "", which is explicitly telling dmraid not to add the 'p' partition separator. Curtis, can you shed some light on why -P is being used here? I think we need to remove it.

This behaviour is as designed.

BACKGROUND INFORMATION
One challenge with dmraid is that it is inconsistent in naming partition devices. Earlier versions never append a 'p' followed by the partition number. Later versions by default always append a 'p' followed by the partition number for partition device paths.

Unfortunately neither of these behaviours match with the commonly accepted partition naming practice:

a) If the device name ends with a number, then append a 'p' followed by the partition number.
b) If the device name does not end with a number, then append the partition number only.

The kpartx command does follow this commonly accepted naming standard, as does parted and udev as far as I can tell.

Earlier version of GParted used to rely on kpartx to create the proper device path names for partitions.

REASON FOR CURRENT BEHAVIOUR
Due to a number of bug reports from users not able to use gparted on dmraid devices when kpartx was not installed, I changed GParted to only use kpartx if it is available. To maintain compatibility with old versions of dmraid, I instruct new dmraid to never use 'p' between the device name and partition number. This permits gparted to work reliably with all versions of dmraid. Hence maximum compatibility is the reason for how GParted currently handles dmraid devices.

When GParted is used to make changes to dmraid partition tables, GParted will ensure that all affected /dev/mapper partition entries are deleted. GParted then creates the new partition entries without a 'p' between the device name and partition number. If kpartx is available, GParted will call kpartx to create device names that follow the commonly accepted partition naming practice.

Hopefully that explanation helps with understanding this design decision.

So gparted constructs the name of the partition device that it expects dmraid to create, rather than parsing the output from running dmraid to see the names it chose? And it uses the rule to only add the 'p' if the base name already ends in a digit? It sounds like dmraid should be fixed to use the same rule and gparted should be fixed to not use the -P parameter, or gparted should parse the output of dmraid for the name it uses.

I have looked into it a bit more and it seems that (lib)parted always adds the 'p', just like dmraid. I have sent an email trying to get a discussion going to come to a consensus but it seems to me that the right thing to do is to fix kpartx and gparted to conform to the dmraid and parted behavior of always adding the 'p'. gparted seems more closely related to parted than to multipath-tools, so should prefer its method if all else is equal.

It might be because I'm getting a little drunk at this point, but I can't quite see where in the code the decision is made to add the 'p' or not. DMRaid::make_path_dmraid_compatible() looked about the spot, but I can't quite wrap my head around it.

There are many things to consider before trying to change the behaviour of gparted.

The current solution maintains compatibility with all versions of dmraid and parted version 1.7.1 and higher.

Only newer versions of (lib)parted appear to use the dmraid library to handle naming. Previous versions of (lib)parted applied the commonly accepted naming practice to dmraid devices.

There are other players in this arena too, such as udev. I have found that some GNU/Linux distributions will make symbolic links, such as /dev/dm-#, to the /dev/mapper entries. Others will make symbolic links in the /dev/mapper directory to devices in /dev/dm-#. There are many different situations that gparted handles.

To learn about how dmraid is handled by GParted a thorough review of the DMRaid.cc file is recommended.

The current solution has been developed over a few years with testing on tens of GNU/Linux distributions

In my opinion, since dmraid is the only tool I am aware of that does not follow the commonly accepted naming practices, I would suggest that dmraid is the place to focus efforts to maintain naming compatibility.

The current solution does not maintain compatibility with recent versions of dmraid though, because when the system starts up, the partitions have the 'p', and when you launch gparted, it creates duplicate devices without the 'p'. Having both devices means that you can mount both at the same time and trash your fs.

The 'p' being added only when the base name already ends in a digit seems fine. As you suggest, dmraid should be fixed to comply, but then gparted also needs fixed. It definitely needs to not pass the -P flag to dmraid. To maintain compatibility with whichever behavior dmraid uses, I think it should parse the output of dmraid for the name it chose.

Unfortunately since dmraid offers _at least_ two different ways of creating partition names, there is never a guarantee that the dmraid default will match the device names chosen for use.

For example:

A) Persons wanting the default behaviour might use:

   sudo dmraid -ay -v

   to create partition names such as /dev/mapper/isw_efjbbijhh_Vol0p1 (new default)
   or /dev/mapper/isw_efjbbijhh_Vol01 (old default)

B) Persons wanting to maintain backward compatibility might use:

   sudo dmraid -dmraid -ay -P "" -v

   to create partition names such as /dev/mapper/isw_efjbbijhh_Vol01

C) Persons wishing to be able to easily parse partition names might use a unique character, such as 'Z':

   sudo dmraid -dmraid -ay -P "Z" -v

   to create partition names such as /dev/mapper/isw_efjbbijhh_Vol0Z1

In my mind the whole problem of partition name duplicates arises because dmraid does not follow the normal convention for naming partition, and now offers more than one way to choose partition names.

Right, that's why gparted should not pass -P and change the default behavior. It should just let dmraid use the name it wants, and follow along. On Maverick and before, Ubuntu has patched dmraid to never add the 'p'. We dropped that patch in Natty, but now it sounds like either both dmraid and parted need patched to only add the 'p' when the last character was already a digit. Or patching kpartx/multipath-tools to always add the 'p' so that they are in agreement would work too.

Whether the 'p' is added always or only when the last character is a digit, either way gparted needs to not pass -P to dmraid and alter its normal behavior, or you end up with duplicate devices.

I am open to considering a patch that performs as you mentioned, and also maintains compatibility will all versions of dmraid, and also (lib)parted versions 1.7.1 and higher.

Why does it try to call kpartx when dmraid has already created the devices?

For that matter, why does it need to run dmraid at all? Libparted already takes care of creating the new devices for you.

> Why does it try to call kpartx when dmraid has already created
> the devices?

If I recall correctly, kpartx did not create the device entries if dmraid had not been used to activate the device.

> For that matter, why does it need to run dmraid at all?
> Libparted already takes care of creating the new
> devices for you.

Older versions of libparted, or versions of libparted configured with --disable-device-mapper do not create the /dev/mapper entries.

The GParted code was developed to work with dev-mapper dmraid devices before libparted was able to deal with these accordingly.

If dmraid does not create the base disk device, then yes, kpartx has nothing to do. The problem arises when dmraid has activated the array, and creates the partitions with one name scheme, and then kpartx is run and tries to use another scheme. If dmraid does not add the 'p' and kpartx does, then kpartx ends up creating duplicate devices for the partitions with the 'p', so you have one with, and one without.

Now that libparted has the functionality, shouldn't it be removed from gparted, rather than duplicating it? If it is going to duplicate it, then it should at least do it the same way. Long term it seems like the whole DMRaid.cc module could go away and go back to letting libparted handle things, since it does so correctly now. If gparted wants to be able to work with older versions of libparted, then perhaps it could be made a configuration option?

As for fixing this bug for Natty, it looks like the -P argument needs dropped, and gparted needs to add the 'p'. Parted and dmraid also could be patched to add the 'p' only if the previous character is a digit. Does that sound like a reasonable simple fix for now?

> If dmraid does not add the 'p' and kpartx does, then kpartx ends up
> creating duplicate devices for the partitions with the 'p', so you
> have one with, and one without.

In my testing I recall some GNU/Linux distributions as having both
/dev/mapper and /dev/dm-# entries for the same dmraid device (and not
symbolic links). Hence duplicate entries might already exist, even
without using GParted. 'Just thought you might want to know that this
situation can arise even without the use of GParted.

Many users run GParted from a Live CD or USB image so that no
partitions are mounted and the full capabilities of GParted are
available. In this situation, the device naming does not matter to
the GNU/Linux distributions on the hard disk device because the live
image partition names disappear on reboot.

To better understand your perspective, why are you concerned that
GParted creates devices that follow the old dmraid naming standard?

> Now that libparted has the functionality, shouldn't it be removed
> from gparted, rather than duplicating it? If it is going to duplicate
> it, then it should at least do it the same way.
> Long term it seems like the whole DMRaid.cc module could go away and
> go back to letting libparted handle things, since it does so
> correctly now.

The long term plan would be to remove DMRaid.cc. However to my
knowledge (lib)parted versions that support dmraid have not been out
for very long. Parted 2.3 was released on 28 May 2010, so it has not
even been out for a year.

Parted versions 1.9.0 up to and including 2.2 have been out for longer
but have experienced a critical problem to varying degrees. The
critical problem is a failure to inform the kernel of partition
changes. This causes grief when used with GParted.
See:
Problem Resizing File Systems with GParted
http://gparted-forum.surf4.info/viewtopic.php?id=13777
GParted contains a work-around for these earlier version,
but it does not work 100% of the time.

Hence if I am to change the minimum requirement of GParted to be
libparted 2.3, then more time is needed for major GNU/Linux
distributions to all be using at least this version or higher in the
their currently supported releases.

> If gparted wants to be able to work with older versions of libparted,
> then perhaps it could be made a configuration option?

Yes, compatibility with older versions of (lib)parted is desired for
the reasons described above. I would suggest that this configuration
option should automatically be set based on the (lib)parted
version.

> As for fixing this bug for Natty, it looks like the -P argument
> needs dropped, and gparted needs to add the 'p'. Parted and dmraid
> also could be patched to add the 'p' only if the previous character
> is a digit. Does that sound like a reasonable simple fix for now?

If the goal is to have better support for dmraid devices in Natty, I
would suggest that a more recent version of GParted be used.

A problem with the dmraid path name was fixed in GParted 0.7.1:
https://bugzilla.gnome.org/show_bug.cgi?id=634553

The -P argument could be dropped; however, I think your idea to scan
the output would then be required to determine what type of partition...

On 02/17/2011 02:57 PM, Curtis Gedak wrote:
> In my testing I recall some GNU/Linux distributions as having both
> /dev/mapper and /dev/dm-# entries for the same dmraid device (and not
> symbolic links). Hence duplicate entries might already exist, even
> without using GParted. 'Just thought you might want to know that this
> situation can arise even without the use of GParted.

Not quite. One or other of those is a symlink. There is only one
actual device, so things like is it mounted checks follow the symlinks
and all check against the same bdev.

> Many users run GParted from a Live CD or USB image so that no
> partitions are mounted and the full capabilities of GParted are
> available. In this situation, the device naming does not matter to
> the GNU/Linux distributions on the hard disk device because the live
> image partition names disappear on reboot.

It matters if the partition is mounted, and then gparted makes a
completely separate block device show up that you can delete, and create
a new partition using that same space, despite the fact that the
original partition using that space is still mounted.

> To better understand your perspective, why are you concerned that
> GParted creates devices that follow the old dmraid naming standard?

I don't care WHICH standard is followed so much as that SOME standard is
obeyed by all components running in the system, so that the duplicate
device problem can be avoided. I think you have sold me though on the
'p' only when needed method that kpartx uses.

> The long term plan would be to remove DMRaid.cc. However to my
> knowledge (lib)parted versions that support dmraid have not been out
> for very long. Parted 2.3 was released on 28 May 2010, so it has not
> even been out for a year.

So why can't gparted releases from this year depend on libparted
versions also released this year?

> Parted versions 1.9.0 up to and including 2.2 have been out for longer
> but have experienced a critical problem to varying degrees. The
> critical problem is a failure to inform the kernel of partition
> changes. This causes grief when used with GParted.
> See:
> Problem Resizing File Systems with GParted
> http://gparted-forum.surf4.info/viewtopic.php?id=13777
> GParted contains a work-around for these earlier version,
> but it does not work 100% of the time.

This is the regression from dropping the BLKPG ioctl support and going
back to only BLKRRPART, so re-reading failed if any partition on the
drive was in use? I fixed that what? A year ago now?

> Hence if I am to change the minimum requirement of GParted to be
> libparted 2.3, then more time is needed for major GNU/Linux
> distributions to all be using at least this version or higher in the
> their currently supported releases.

Why? Who says a distribution needs to be able to run the latest version
of gparted, but an outdated version of libparted? If the distribution
can update to the latest version of gparted, then they can update parted
as well just as easily.

> Yes, compatibility with older versions of (lib)parted is desired for
> the reasons described above. I would suggest that this configuration
> option should automa...

Since we have several topics of discussion, I will try to break these
into sections. This will hopefully make easier to see what we need to
do. :-)

    ------------------------------
1) THANK YOU FOR YOUR PATCHES AND INTEREST IN PARTED AND GPARTED

I wish to begin by thanking you for the patches you have supplied for
parted. Your patches along with another from Jim Meyering have been
critical in helping to lessen problems for users of Parted and
GParted arising from a "failure to inform kernel of partition
changes".

The Jim Meyering patch I speak of is this one:
http://git.debian.org/?p=parted/parted.git;a=commit;h=0f850220b3f26bb969a1a7ff78dc550691a89566

There may yet be some further room for improvement in this area
because I have at least one report of the problem continuing.
See:
http://gparted-forum.surf4.info/viewtopic.php?id=14357
Personally I have not been able to reproduce this problem.

    ------------------------------
2) WHAT TO DO FOR FOR THE UPCOMING UBUNTU NATTY RELEASE?

Details of the changes in each release of GParted can be found in the
NEWS file:
http://git.gnome.org/browse/gparted/tree/NEWS

If it is too late in Natty's release cycle to include a newer version
of GParted, then perhaps we should focus on NATTY+1. The possibility
of duplicate dmraid device partition names has been present since
DMRaid support was added in GParted 0.4.4.

If there is still time to consider patches in Natty, I suggest the
following patch to fix incorrect dmraid partition path names:
http://git.gnome.org/browse/gparted/commit/?id=3c35a7ff42927bfcd89465661766b661f4fbcb76
In my mind this problem has a higher possibility of impacting a user
than duplicate dmraid device names.

    ------------------------------
3) WHY MAINTAIN BACKWARD COMPATIBILITY WITH OLDER LIBPARTED VERSIONS?

You might call this a personal pet peeve, but I hate when I am forced to
upgrade my entire operating system just to run a newer version of an
application. I find this particularly vexing when the operating
system or GNU/Linux distribution is still within a supported
time-frame.

This is why I _try_ my best to ensure that GParted will compile and
run on the currently supported major GNU/Linux distributions. I
recognize that there are situations where it makes sense to break from
this goal.

Unfortunately the following statement is not always true:
"If the distribution can update to the latest version of gparted, then
they can update parted as well just as easily."

A case in point is with Ubuntu 8.04 Hardy Heron. This release is
currently supported on both the desktop and server. However,
libparted-2.3 does not work well with GParted on Ubuntu 8.04. The
problem encountered when performing operations in GParted with
libparted 2.3 is the unfortunately familiar "failure to inform kernel
of partition changes".

Older versions of libparted, such as 1.7.1 that is packaged for Ubuntu
8.04, do not exhibit this problem on Ubuntu 8.04.

I have tested this on a physical computer installation, and in a
virtual machine with the same results.

    ------------------------------
4) WHAT TO DO FOR THE UPCOMING UBUNTU NATTY+1 RELEASE?

I would like to work together with ...

I actually remember filing the bug several years ago about gparted not supporting dmraid and talking with you about how it could do that. I'm also reading a book on autotools at the moment. While I grok the theory of operation, I don't yet have the details down. It sounds like you are saying that you are in the same boat, and that you agree that if it detects at least version 2.3.0 of libparted, then phase out DMRaid.cc, but are asking if I can help with actually implementing the test for >= 2.3.0. Hopefully by the time I finish this book I will.

I can certainly get in a patch for natty, and that is what I want to do, but updating to a whole new upstream release is not allowed AFAIK. That is why I am trying to figure out the minimum patch required to fix the duplicate device issue. That seems to mean fixing DMRaid.cc to add the 'p'. It either needs to always add it like dmraid and libparted do, or dmraid and libparted need patched to conform with the kpartx behavior. I am leaning towards the latter.

I wonder though, if you are going to backport the latest gparted to 8.04, why wouldn't you also want to backport libparted along with it?

Also I do not quite understand this: http://git.gnome.org/browse/gparted/commit/?id=3c35a7ff42927bfcd89465661766b661f4fbcb76

It looks like another bug with the Natty version of gparted. To fix that, you should open another bug report about it. If it does happen in Natty, and that upstream patch fixes it, then it can easily be cherry picked.

Based on comment #17, I think we are on same page. :-)

Attached is an initial patch to use native libparted /dev/mapper
dmraid support. This patch was generated using:

   $ git diff > enable-libparted-dmraid.patch

During the development process I discovered that an automatic
configure option that checked only the libparted version would not
work. The reason for this is that the naming standard used by the
installed version of dmraid might not match with the naming standard
used by the installed version of libparted. Hence I chose to
implement a manual configuration flag: --enable-libparted-dmraid.

My preference is to test in a virtual machine, but I do not know how
to do this when testing dmraid devices.
Do you know how to set up dmraid devices in a virtual machine?

This patch requires much more testing.
Are you able to test this with Natty?

You will need to configure with the "--enable-libparted-dmraid" flag
to test with Natty.

If this patch works then you do not need to worry about the previous
patch and bug report that I mentioned.

Updated patch attached. Generated with the command:

   $ git diff > enable-libparted-dmraid-2.patch

The difference between this and the prior patch is that no changes should be needed to src/DMRaid.cc.

I had to fix up two hunks that failed to apply in GParted_Core::set_mountpoints. It looked like in your version you had the if() test for dmraid first, then the for loop inside, but in ours it was the other way around. After I finally figured out I had to run autoconf AND autoheader for the new configure switch to actually work, it crashes the kernel in Natty. Seems to be an unrelated kernel bug there. I have been using the natty build of dmraid and libparted on my Maverick install for some time, and so I ran the patched gparted there and it worked fine with the 'p' in the name and no duplicate devices.

I tried to reproduce the kernel bug and it didn't crash this time. Gparted worked fine under Natty, just as it did under Maverick. It used the 'p' in the name quite happily. Tomorrow I'll push a bzr branch and propose it be merged to fix this. I think I'll also try to get in patches to dmraid and parted to hide the 'p' when the previous char is not a digit.

As you discovered, the patch was not made against the Natty code, but against the head of the master branch in the upstream git repository. I apologize for this misunderstanding.

Attached is another patch that includes an update to the src/Dialog_Progress.cc file. This change includes the " --enable-libparted-dmraid" in the save_details.htm log file to help with diagnosing GParted bugs.

I would appreciate if you could add the update for src/Dialog_Progress.cc into the patch for Natty.

The specific diff for this one file is as follows:

$ git diff src/Dialog_Progress.cc
diff --git a/src/Dialog_Progress.cc b/src/Dialog_Progress.cc
index 1921d8a..03ea376 100644
--- a/src/Dialog_Progress.cc
+++ b/src/Dialog_Progress.cc
@@ -354,7 +354,11 @@ void Dialog_Progress::on_save()
                        << "<title>" << _("GParted Details") << "</title>" << std::endl
                        << "</head>" << std::endl
                        << "<body>" << std::endl
- << "<p>" << _("GParted") << " " << VERSION << "</p>" << std::endl
+ << "<p>" << _("GParted") << " " << VERSION
+#ifdef USE_LIBPARTED_DMRAID
+ << " --enable-libparted-dmraid"
+#endif
+ << "</p>" << std::endl
                        << "<p>" << _("Libparted") << " " << signal_get_libparted_version .emit() << "</p>" << std::endl ;

                        //Write out each operation
$

In the meantime I am downloading the daily build of natty-desktop-i386.iso today (Feb 22, 2011) in preparation for testing this patch with Natty.

My testing using GParted compiled from the master branch of the latest upstream git repository worked very well with the new --enable-libparted-dmraid configure option.

The relevant upstream commit can be viewed at the following link:
http://git.gnome.org/browse/gparted/commit/?id=2f99511228c01e35df545b2514cb7ba642ce242d

My testing was performed using the live image of natty-desktop-i386.iso from today (Feb 22, 2011).

Bumping importance to high since this can lead to data loss.

By testing, do you mean that you played with it a bit manually, or ran automated unit testing?

There is no automated unit testing for gparted (at least that I am aware of). My testing included creating, deleting, moving, and resizing of primary and logical partitions.

This bug was fixed in the package gparted - 0.7.0-1ubuntu1

---------------
gparted (0.7.0-1ubuntu1) natty; urgency=low

  [ Phillip Susi ]
  * Add 02_enable-libparted-dmraid-2.patch: cherry pick
    new configure parameter from upstream to disable
    gparted specific dmraid handling and leave it to
    libparted, which handles it correctly now. (LP: #719129)

  [ Michael Terry ]
  * debian/rules, debian/control:
    - Rather than modifying configure directly, call dh_autoreconf
  * debian/patches/02_enable-libparted-dmraid-2.patch:
    - Use DEP3 headers to explain origin of this patch
 -- Phillip Susi <email address hidden> Mon, 28 Feb 2011 14:53:10 -0500

GParted 0.8.1 has been released upstream. It includes a new configure option --enable-libparted-dmraid that permits this same functionality.

Thanks again go to Phillip Susi for help with this idea.