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

[Steven Whitehouse]

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 at redhat.com>
> > AuthorDate:    Fri Oct 10 16:04:29 2008 +0100
> > Committer:     Steven Whitehouse <swhiteho at 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.

> > 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.
> 
> Thanks
> Fabio
> 
> --
> I'm going to make him an offer he can't refuse.
> 
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. Also libuuid seems somewhat over engineered.
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.

Steve.