[Cluster-devel] [PATCH] mkfs: Handle gfs2 creation on regular files

[Steven Whitehouse <swhiteho@redhat.com>]

Hi,

On Mon, 2011-07-11 at 15:23 +0100, Andrew Price wrote:
> mkfs.gfs2 previously exited if the target volume was a regular file.
> This patch allows creation of gfs2 file systems on regular files by
> removing an exiting check, guarding against calling device_topology()
> and falling back to the default block size if the target is a regular
> file. It also moves a stat() call out of check_mount() and into
> main_mkfs() so that the result can be queried multiple times.
> 
> rhbz#719943
> 
> Signed-off-by: Andrew Price <anprice@redhat.com>
> ---
>  gfs2/mkfs/main_mkfs.c |   16 ++++++++--------
>  1 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
> index b0bb6e3..e24b284 100644
> --- a/gfs2/mkfs/main_mkfs.c
> +++ b/gfs2/mkfs/main_mkfs.c
> @@ -483,14 +483,8 @@ static void are_you_sure(struct gfs2_sbd *sdp)
>  
>  static void check_mount(char *device)
>  {
> -	struct stat st_buf;
>  	int fd;
>  
> -	if (stat(device, &st_buf) < 0)
> -		die( _("could not stat device %s\n"), device);
> -	if (!S_ISBLK(st_buf.st_mode))
> -		die( _("%s is not a block device\n"), device);
> -
>  	fd = open(device, O_RDONLY | O_NONBLOCK | O_EXCL | O_CLOEXEC);
>  
>  	if (fd < 0) {

This whole check_mount() function looks suspicious to me... we seem to
call it to open the fd and then we close the fd and back in the caller
function immediately open it again with different args...

Perhaps we can just get rid of it completely since it doesn't add
anything useful. 

> @@ -561,6 +555,7 @@ void main_mkfs(int argc, char *argv[])
>  	int rgsize_specified = 0;
>  	uint64_t real_device_size;
>  	unsigned char uuid[16];
> +	struct stat st_buf;
>  
>  	memset(sdp, 0, sizeof(struct gfs2_sbd));
>  	sdp->bsize = -1;
> @@ -579,6 +574,9 @@ void main_mkfs(int argc, char *argv[])
>  
>  	verify_arguments(sdp);
>  
> +	if (stat(sdp->device_name, &st_buf) < 0)
> +		die( _("could not stat device %s\n"), sdp->device_name);
> +
>  	check_mount(sdp->device_name);
>  
>  	sdp->device_fd = open(sdp->device_name, O_RDWR | O_CLOEXEC);

It would be better to open the device unconditionally and then use fstat
in order to avoid any possible races.

Steve.

> @@ -589,15 +587,17 @@ void main_mkfs(int argc, char *argv[])
>  	if (!sdp->override)
>  		are_you_sure(sdp);
>  
> -	if (device_topology(sdp)) {
> +	if (!S_ISREG(st_buf.st_mode) && device_topology(sdp)) {
>  		fprintf(stderr, _("Device topology error\n"));
>  		exit(-1);
>  	}
>  
>  	if (sdp->bsize == -1) {
> +		if (S_ISREG(st_buf.st_mode))
> +			sdp->bsize = GFS2_DEFAULT_BSIZE;
>  		/* See if optimal_io_size (the biggest I/O we can submit
>  		   without incurring a penalty) is a suitable block size. */
> -		if (sdp->optimal_io_size <= getpagesize() &&
> +		else if (sdp->optimal_io_size <= getpagesize() &&
>  		    sdp->optimal_io_size >= sdp->minimum_io_size)
>  			sdp->bsize = sdp->optimal_io_size;
>  		/* See if physical_block_size (the smallest unit we can write