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

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

[Andrew Price]

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) {
@@ -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);
@@ -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
-- 
1.7.6

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

[Steven Whitehouse]

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

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

[Andrew Price]

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 removes the check_mount() function and replaces it with an
fstat() call after opening the file to avoid races.

rhbz#719943

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/mkfs/main_mkfs.c |   45 +++++++++++----------------------------------
 1 files changed, 11 insertions(+), 34 deletions(-)

diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
index b0bb6e3..eaf2aa2 100644
--- a/gfs2/mkfs/main_mkfs.c
+++ b/gfs2/mkfs/main_mkfs.c
@@ -476,36 +476,6 @@ static void are_you_sure(struct gfs2_sbd *sdp)
 }
 
 /**
- * check_mount - check to see if device is mounted/busy
- * @device: the device to create the filesystem on
- *
- */
-
-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) {
-		if (errno == EBUSY) {
-			die( _("device %s is busy\n"), device);
-		}
-	}
-	else {
-		close(fd);
-	}
-
-	return;
-}
-
-/**
  * print_results - print out summary information
  * @sdp: the command line
  *
@@ -561,6 +531,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,25 +550,31 @@ void main_mkfs(int argc, char *argv[])
 
 	verify_arguments(sdp);
 
-	check_mount(sdp->device_name);
-
 	sdp->device_fd = open(sdp->device_name, O_RDWR | O_CLOEXEC);
 	if (sdp->device_fd < 0)
 		die( _("can't open device %s: %s\n"),
 		    sdp->device_name, strerror(errno));
 
+	if (fstat(sdp->device_fd, &st_buf) < 0) {
+		fprintf(stderr, _("could not fstat fd %d: %s\n"),
+			sdp->device_fd, strerror(errno));
+		exit(-1);
+	}
+
 	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
-- 
1.7.6

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

[Steven Whitehouse]

Hi,

On Mon, 2011-07-11 at 17:09 +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 removes the check_mount() function and replaces it with an
> fstat() call after opening the file to avoid races.
> 
> rhbz#719943
> 
That looks good. Thanks for fixing that up,

Steve.

> Signed-off-by: Andrew Price <anprice@redhat.com>
> ---
>  gfs2/mkfs/main_mkfs.c |   45 +++++++++++----------------------------------
>  1 files changed, 11 insertions(+), 34 deletions(-)
> 
> diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
> index b0bb6e3..eaf2aa2 100644
> --- a/gfs2/mkfs/main_mkfs.c
> +++ b/gfs2/mkfs/main_mkfs.c
> @@ -476,36 +476,6 @@ static void are_you_sure(struct gfs2_sbd *sdp)
>  }
>  
>  /**
> - * check_mount - check to see if device is mounted/busy
> - * @device: the device to create the filesystem on
> - *
> - */
> -
> -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) {
> -		if (errno == EBUSY) {
> -			die( _("device %s is busy\n"), device);
> -		}
> -	}
> -	else {
> -		close(fd);
> -	}
> -
> -	return;
> -}
> -
> -/**
>   * print_results - print out summary information
>   * @sdp: the command line
>   *
> @@ -561,6 +531,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,25 +550,31 @@ void main_mkfs(int argc, char *argv[])
>  
>  	verify_arguments(sdp);
>  
> -	check_mount(sdp->device_name);
> -
>  	sdp->device_fd = open(sdp->device_name, O_RDWR | O_CLOEXEC);
>  	if (sdp->device_fd < 0)
>  		die( _("can't open device %s: %s\n"),
>  		    sdp->device_name, strerror(errno));
>  
> +	if (fstat(sdp->device_fd, &st_buf) < 0) {
> +		fprintf(stderr, _("could not fstat fd %d: %s\n"),
> +			sdp->device_fd, strerror(errno));
> +		exit(-1);
> +	}
> +
>  	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