[Cluster-devel] gfs2-utils: master - mkfs.gfs2: Move the new rgrp creation code into libgfs2

[Bob Peterson <rpeterso@redhat.com>]

Hi,

Comments embedded.

----- Original Message -----
| mkfs.gfs2: Move the new rgrp creation code into libgfs2
(snip)
| +// Temporary function to aid in API migration

This ^ is a C++ comment, and should be: /* ... */

| + * rglen: The required length of the resource group. If its is 0 the default

"If its is 0" should be "If it is 0" (Sorry: Grammar Nazi at work here. :) )

| +	rg->ri.ri_length = rgblocks2bitblocks(rgs->bsize, rglen, &rg->ri.ri_data);
| +	rg->ri.ri_data0 = rg->ri.ri_addr + rg->ri.ri_length;
| +	rg->ri.ri_bitbytes = rg->ri.ri_data / GFS2_NBBY;
| +	rg->rg.rg_header.mh_magic = GFS2_MAGIC;
| +	rg->rg.rg_header.mh_type = GFS2_METATYPE_RG;
| +	rg->rg.rg_header.mh_format = GFS2_FORMAT_RG;

I don't know if this memory gets zeroed out, but perhaps we should explicitly
zero out rg->rg.rg_header.__pad0 as well. There have been times when we reused
unused fields like this for things like generation numbers (mainly for
debugging purposes) so I'd hate to have this value uninitialized. If the struct
is zeroed out, don't worry about it.

| +/**
| + * Write a resource group to a file descriptor.
| + * Returns 0 on success or non-zero on failure with errno set
| + */
| +int lgfs2_rgrp_write(int fd, lgfs2_rgrp_t rg, unsigned bsize)
| +{
| +	ssize_t ret = 0;
| +	size_t len = rg->ri.ri_length * bsize;
| +	unsigned int i;
| +	const struct gfs2_meta_header bmh = {
| +		.mh_magic = GFS2_MAGIC,
| +		.mh_type = GFS2_METATYPE_RB,
| +		.mh_format = GFS2_FORMAT_RB,

Here again, maybe we should set __pad0 explicitly rather than leaving the
value uninitialized.

The rest looks good to me.

Regards,

Bob Peterson
Red Hat File Systems