[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Cluster-devel] Re: master - libgfs2: Add support for UUID generation to gfs2_mkfs



On Mon, 13 Oct 2008, Steven Whitehouse wrote:

Hi,

On Fri, 2008-10-10 at 19:31 +0200, Fabio M. Di Nitto wrote:
On Fri, 10 Oct 2008, Steven Whitehouse wrote:

Gitweb:        http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=85049a0824daa9abaa38f5dca377767907b53b39
Commit:        85049a0824daa9abaa38f5dca377767907b53b39
Parent:        d763f902abf33655e635fc3b8b1919fd31d4f66c
Author:        Steven Whitehouse <swhiteho redhat com>
AuthorDate:    Fri Oct 10 16:04:29 2008 +0100
Committer:     Steven Whitehouse <swhiteho redhat com>
CommitterDate: Fri Oct 10 16:12:12 2008 +0100

libgfs2: Add support for UUID generation to gfs2_mkfs

nice...

Uses /dev/urandom to create 16 byte UUIDs for GFS2 filesystems
at mkfs time. Backwards and forwards compatible with all
GFS2 filesystems.

You'll need a set of kernel headers with
the new field defined in order for this feature to be
enabled. Bugzilla #242690

What kernel version is required to have UUID support? is it in .27 or will
be in .28?

It will be .28, but this is a build dep only. It doesn't make any
difference what kernel you run once mkfs has been compiled.

Absolutely fine. I need to make sure to test when we switch to .28 headers.


diff --git a/gfs2/libgfs2/structures.c b/gfs2/libgfs2/structures.c
index eb4c7bd..002edb6 100644
--- a/gfs2/libgfs2/structures.c
+++ b/gfs2/libgfs2/structures.c
@@ -58,7 +58,17 @@ build_sb(struct gfs2_sbd *sdp)
	sb.sb_root_dir = sdp->md.rooti->i_di.di_num;
	strcpy(sb.sb_lockproto, sdp->lockproto);
	strcpy(sb.sb_locktable, sdp->locktable);
-
+#ifdef GFS2_HAS_UUID
+	{
+		int fd = open("/dev/urandom", O_RDONLY);
+		int n;
+		if (fd >= 0)
+			n = read(fd, &sb.sb_uuid, 16);
+		if (fd < 0 || n != 16)
+			memset(&sb.sb_uuid, 0, 16);
+		close(fd);
+	}
+#endif

NACK.

Please use libuuid for this operation.


Hmm, well there are some problems with that... it will introduce another
build dep, and one which we can't #ifdef around since it will be in the
rpm rather than the source.

This is not an issue.

Also libuuid seems somewhat over engineered.

Right, it is, but there are situations where /dev/urandom might not have the entropy required to spit out one single bit. I have seen it happening a few times, specially on remote servers where there is no mouse or keyboard to generate randomness and took over a few hours to generate an ssh key.

I guess we should set the format correctly though. Something like:

sb.sb_uuid[7] &= ~0xf0; /* time_hi_and_version */
sb.sb_uuid[7] |= 0x40;
sb.sb_uuid[8] &= ~0xc0; /* clock_seq_hi_and_reserved */
sb.sb_uuid[8] |= 0x40;

after the memset would do the trick. There doesn't seem much point using
any of the other UUID formats since they seem to result less entropy
than the random method and we have a reasonable random number generator
available (which is not a PRNG despite the implication in the libuuid
source).

Even with those 6 bits made constant we still have 122 bits left so the
chances of clashing UUIDs is 1 in 2^122 assuming that we can rely
on /dev/urandom so I don't think we are likely to have any problems
here.

I would still prefer to avoid implementating our own version of a UUID generator and rely on what's already done.

Fabio

--
I'm going to make him an offer he can't refuse.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]