Bug 242701 - Add Filesystem UUID to GFS2 utils.
Summary: Add Filesystem UUID to GFS2 utils.
Alias: None
Product: Red Hat Enterprise Linux 5
Classification: Red Hat
Component: gfs2-utils   
(Show other bugs)
Version: 5.0
Hardware: All Linux
Target Milestone: ---
: ---
Assignee: Robert Peterson
QA Contact: Dean Jansa
Keywords: FutureFeature
Depends On: 242696
TreeView+ depends on / blocked
Reported: 2007-06-05 14:23 UTC by Scott Crenshaw
Modified: 2010-01-12 03:38 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Enhancement
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2009-09-02 11:02:31 UTC
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)
Proposed patch (6.42 KB, patch)
2009-01-22 21:56 UTC, Robert Peterson
no flags Details | Diff
Proposed patch revised (9.80 KB, patch)
2009-01-29 17:23 UTC, Robert Peterson
no flags Details | Diff
Proposed patch with Steve's suggestions implemented (9.43 KB, patch)
2009-01-29 18:17 UTC, Robert Peterson
no flags Details | Diff
Upstream version of the proposed patch (9.69 KB, patch)
2009-01-30 16:10 UTC, Robert Peterson
no flags Details | Diff

External Trackers
Tracker ID Priority Status Summary Last Updated
Red Hat Product Errata RHSA-2009:1337 normal SHIPPED_LIVE Low: gfs2-utils security and bug fix update 2009-09-01 10:41:56 UTC

Description Rob Kenna 2007-06-05 14:23:02 UTC
ext2/3 support file system labels and uuid's (different bz) which are useful for
specifying in fstab for mount.

This needs to be added to the gfs2 utils, including mkfs.gfs2 and any display tools.

bug 242696 is the kernel piece

Comment 1 Chris Feist 2007-06-05 20:09:47 UTC
Re-assigning to rpeterso.

Comment 2 Robert Peterson 2007-06-18 22:58:23 UTC
Adding Dave T. to the cc list to get his input, as discussed in last
week's cluster meeting.

Comment 3 Robert Peterson 2007-06-21 16:25:10 UTC
See Dave T's comments in bug #242690.  It's a Fedora bug record, but 
comments still apply.

Comment 7 Steve Whitehouse 2008-12-16 15:14:24 UTC
Already in upstream. Also need to update udev as well I think (maybe need to open a bug for that) and the patch for that is also upstream.

The kernel part of this for RHEL is now in POST.

Comment 8 Steve Whitehouse 2009-01-09 14:03:28 UTC
This should be trivial. Bob, can you finish this one off?

Comment 9 Robert Peterson 2009-01-22 21:56:54 UTC
Created attachment 329741 [details]
Proposed patch

Here is a patch that adds uuid to the user space tools for gfs2.
The changes are as follows:

1. gfs2_edit was modified to print the uuid value.
2. libgfs2 was modified to add some support functions.
3. The build_sb function was modified to accept a uuid parameter.
4. mkfs.gfs2 was modified to generate a random uuid using some code
   that was borrowed (and modified slightly) from libuuid in e2fsprogs 

Steve: Can you look it over and make sure it has everything you expect?

Comment 10 Steve Whitehouse 2009-01-23 08:37:06 UTC
From the patch:

+ * get_random_fd
+ * Borrowed from gen_uuid.c in e2fsprogs/lib
+ */
+static int get_random_fd(void)
+	struct timeval	tv;
+	static int	fd = -2;
+	int		i;
+	if (fd == -2) {
+		gettimeofday(&tv, 0);

Looks like fd will always be -2 so that if is not required.

+ * generate_uuid - generate a random uuid
+ */
+void generate_uuid(void)
+	get_random_bytes(uuid, sizeof(uuid));
Do we really need another name for the same function? Also please don't use a global variable to pass the uuid around as you chould be able to put it in the sb directly.

If you make changes here beyond the existing code in the master branch, don't forget to update that as well.

Comment 11 Robert Peterson 2009-01-27 19:53:43 UTC
Eric Sandeen recommended that we just add a dependency on
e2fsprogs-libs and make calls into libuuid to generate the UUIDs.
I didn't want to add the dependency, but apparently there is already
a precedent: The xfs user space tools apparently depend on e2fsprogs!
Apparently there are lots of things that have this dependency.
"<sandeen> try rpm -e e2fsprogs-libs and see what you get :)"

I'm adding Eric and Chris Feist to the list to get some opinions
flying.  Chris: Do you see any problems adding this dependency?
Or is the dependency already there for another reason?

I've also made some changes to the patch to allow mkfs.gfs2 to
specify a uuid, and am allowing gfs2_tool to modify it.  In the
(extremely) unlikely event of duplicate uuids for file systems,
we want to allow them to change it after the fact.  Some research
revealed that e2fsprogs has tools to change the uuid but strangely,
mkfs.ext3 doesn't seem to have a way to set it.  IIRC, the xfs tools
allow you to both set it with mkfs.xfs and change it after the fact.

Comment 12 Eric Sandeen 2009-01-27 21:29:06 UTC
re: xfs, xfs_admin -U <uuid> will set it on an existing filesystem.  There's no mkfs.xfs option to choose it at mkfs time, though.

Comment 13 Steve Whitehouse 2009-01-28 11:00:11 UTC
I'd rather not add a dependency just because we need some random bytes. It doesn't need anything complicated and reading /dev/urandom is enough. If that fails, then we can just leave the UUID blank. We have to deal with that case anyway for backwards compatibility, so its not really a big deal.

I agree that we should not allow people to choose the UUID at mkfs time. If they want to choose an identifier themselves, then it should be the label rather than the UUID.

Currently the label must be unique over all mounted filesystems as its used as the directory name under sysfs, and the mount fails if there is an existing name which is identical. This is perhaps a bug that we need to fix. One way might be to use the UUID in case its unique, but to make up an id, if its not unique. We could then have links to the fs to maintain the old "by label" method. But that is another bugzilla...

Comment 14 Robert Peterson 2009-01-29 17:23:32 UTC
Created attachment 330380 [details]
Proposed patch revised

This patch includes, in order of appearance:

1. The ability for gfs2_edit to print the uuid value with -p sb and
   also to show it in the "structures" display.
2. The ability for gfs2_tool to print the uuid value with "sb"..."all"
   and "sb"..."uuid" and the ability to change the uuid through the
   same mechanism.
3. Update to the gfs2_tool.8 man page to reflect that
4. This version of mkfs concocts the new uuid randomly, but does not
   allow the users to specify it (the same as mkfs.ext3 and mkfs.xfs)

Comment 15 Robert Peterson 2009-01-29 18:17:34 UTC
Created attachment 330388 [details]
Proposed patch with Steve's suggestions implemented

This version has a less hokey rework of function get_random_bytes.
The original from libuuid wasn't the best.

Comment 16 Robert Peterson 2009-01-30 16:10:13 UTC
Created attachment 330482 [details]
Upstream version of the proposed patch

Here's the upstream patch I plan to push.  The basic difference is
that I've added a few #ifdef GFS2_HAS_UUID statements to allow it to
compile on some older kernels.

Comment 17 Robert Peterson 2009-01-30 16:41:59 UTC
The upstream patch is now committed to the master branch of the
gfs2-utils git repository.  The RHEL5 patch is now committed to the
RHEL5 branch of the cluster.git repository for inclusion into 5.4.
It was tested on system roth-01.  Changing status to MODIFIED.

Comment 23 errata-xmlrpc 2009-09-02 11:02:31 UTC
An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.


Note You need to log in before you can comment on or make changes to this bug.