[Cluster-devel] [PATCH 0/6] More mkfs.gfs2 reworking

cluster-devel.redhat.com archive mirror
 help / color / mirror / Atom feed

* [Cluster-devel] [PATCH 0/6] More mkfs.gfs2 reworking
@ 2013-05-14 10:45 Andrew Price
  2013-05-14 10:45 ` [Cluster-devel] [PATCH 1/6] mkfs.gfs2: Add options for stripe size and width Andrew Price
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: Andrew Price @ 2013-05-14 10:45 UTC (permalink / raw)
  To: cluster-devel.redhat.com

The stripe alignment work is ongoing but these patches are ready to go so I thought I'd get them posted.

Andrew Price (6):
  mkfs.gfs2: Add options for stripe size and width
  libgfs2: Remove 'writes' field from gfs2_sbd
  mkfs.gfs2: Link to libblkid
  mkfs.gfs2: Use libblkid for checking contents
  mkfs.gfs2: Add a struct to store device info
  libgfs2: Clarify gfs2_compute_bitstructs's parameters

 configure.ac                |   1 +
 gfs2/convert/gfs2_convert.c |   2 +-
 gfs2/fsck/rgrepair.c        |   6 +-
 gfs2/libgfs2/buf.c          |   1 -
 gfs2/libgfs2/fs_geometry.c  |   2 +-
 gfs2/libgfs2/libgfs2.h      |   3 +-
 gfs2/libgfs2/rgrp.c         |  10 +-
 gfs2/libgfs2/super.c        |   2 +-
 gfs2/mkfs/Makefile.am       |   3 +
 gfs2/mkfs/main_mkfs.c       | 446 ++++++++++++++++++++++++++------------------
 10 files changed, 280 insertions(+), 196 deletions(-)

-- 
1.8.1.4



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Cluster-devel] [PATCH 1/6] mkfs.gfs2: Add options for stripe size and width
  2013-05-14 10:45 [Cluster-devel] [PATCH 0/6] More mkfs.gfs2 reworking Andrew Price
@ 2013-05-14 10:45 ` Andrew Price
  2013-05-14 10:49   ` Steven Whitehouse
  2013-05-14 10:45 ` [Cluster-devel] [PATCH 2/6] libgfs2: Remove 'writes' field from gfs2_sbd Andrew Price
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Andrew Price @ 2013-05-14 10:45 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Add generic parsing of options passed in with -o and use it to accept
sunit and swidth options (names chosen to be the same as mkfs.xfs). We
don't do anything with these options yet.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/mkfs/main_mkfs.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 105 insertions(+), 2 deletions(-)

diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
index 610a641..8d5549d 100644
--- a/gfs2/mkfs/main_mkfs.c
+++ b/gfs2/mkfs/main_mkfs.c
@@ -19,6 +19,7 @@
 #include <sys/time.h>
 #include <libintl.h>
 #include <sys/ioctl.h>
+#include <limits.h>
 
 #define _(String) gettext(String)
 
@@ -55,6 +56,7 @@ static void print_usage(const char *prog_name)
 	    "-j", _("<number>"), _("Number of journals"),
 	    "-K", NULL,          _("Don't try to discard unused blocks"),
 	    "-O", NULL,          _("Don't ask for confirmation"),
+	    "-o", _("<key>[=<value>][,...]"), _("Specify extended options"),
 	    "-p", _("<name>"),   _("Name of the locking protocol"),
 	    "-q", NULL,          _("Don't print anything"),
 	    "-r", _("<size>"),   _("Size of resource groups, in megabytes"),
@@ -73,7 +75,7 @@ static void print_usage(const char *prog_name)
 		option = options[i];
 		param = options[i+1];
 		desc = options[i+2];
-		printf("%3s %-15s %s\n", option, param ? param : "", desc);
+		printf("%3s %-22s %s\n", option, param ? param : "", desc);
 	}
 }
 
@@ -82,6 +84,8 @@ struct mkfs_opts {
 	unsigned qcsize;
 	unsigned jsize;
 	unsigned rgsize;
+	unsigned sunit;
+	unsigned swidth;
 	uint64_t fssize;
 	uint32_t journals;
 	const char *lockproto;
@@ -93,6 +97,8 @@ struct mkfs_opts {
 	unsigned got_qcsize:1;
 	unsigned got_jsize:1;
 	unsigned got_rgsize:1;
+	unsigned got_sunit:1;
+	unsigned got_swidth:1;
 	unsigned got_fssize:1;
 	unsigned got_journals:1;
 	unsigned got_lockproto:1;
@@ -145,11 +151,95 @@ static int discard_blocks(int fd, uint64_t len, int debug)
         return 0;
 }
 
+/**
+ * Convert a human-readable size string to a long long.
+ * Copied and adapted from xfs_mkfs.c.
+ */
+static long long cvtnum(unsigned int blocksize, unsigned int sectorsize, char *s)
+{
+        long long i;
+        char *sp;
+
+        i = strtoll(s, &sp, 0);
+        if (i == 0 && sp == s)
+                return -1LL;
+        if (*sp == '\0')
+                return i;
+
+	*sp = tolower(*sp);
+        if (*sp == 'b' && sp[1] == '\0') {
+                if (blocksize)
+                        return i * blocksize;
+                fprintf(stderr, _("Block size not available yet.\n"));
+		exit(1);
+        }
+        if (*sp == 's' && sp[1] == '\0') {
+                if (sectorsize)
+                        return i * sectorsize;
+                return i * GFS2_BASIC_BLOCK;
+        }
+        if (*sp == 'k' && sp[1] == '\0')
+                return 1024LL * i;
+        if (*sp == 'm' && sp[1] == '\0')
+                return 1024LL * 1024LL * i;
+        if (*sp == 'g' && sp[1] == '\0')
+                return 1024LL * 1024LL * 1024LL * i;
+        if (*sp == 't' && sp[1] == '\0')
+                return 1024LL * 1024LL * 1024LL * 1024LL * i;
+        if (*sp == 'p' && sp[1] == '\0')
+                return 1024LL * 1024LL * 1024LL * 1024LL * 1024LL * i;
+        if (*sp == 'e' && sp[1] == '\0')
+                return 1024LL * 1024LL * 1024LL * 1024LL * 1024LL * 1024LL * i;
+        return -1LL;
+}
+
+static void opt_tok_unsigned(struct mkfs_opts *opts, char *s, unsigned *n, const char *key)
+{
+	long long l;
+	char *val = strtok_r(NULL, "=", &s);
+	if (val == NULL || *val == '\0') {
+		fprintf(stderr, _("Missing argument to '%s'\n"), key);
+		exit(-1);
+	}
+	l = cvtnum(opts->bsize, 0, val);
+	if (l > UINT_MAX || l < 0) {
+		fprintf(stderr, _("Value of '%s' is invalid\n"), key);
+		exit(-1);
+	}
+	*n = (unsigned)l;
+}
+
+static void opt_parse_options(char *str, struct mkfs_opts *opts)
+{
+	char *tok;
+	char *key;
+	char *o, *a;
+
+	for (tok = strtok_r(str, ",", &o); tok != NULL; tok = strtok_r(NULL, ",", &o)) {
+		key = strtok_r(tok, "=", &a);
+		if (key == NULL || *key == '\0') {
+			fprintf(stderr, _("Missing argument to '-o' option\n"));
+			exit(-1);
+		}
+		if (strcmp("sunit", key) == 0) {
+			opt_tok_unsigned(opts, a, &opts->sunit, "sunit");
+			opts->got_sunit = 1;
+		} else if (strcmp("swidth", key) == 0) {
+			opt_tok_unsigned(opts, a, &opts->swidth, "swidth");
+			opts->got_swidth = 1;
+		} else {
+			fprintf(stderr, _("Invalid option '%s'\n"), key);
+			exit(-1);
+		}
+	}
+}
+
 static void opts_get(int argc, char *argv[], struct mkfs_opts *opts)
 {
 	int c;
+	char *o = NULL;
 	while (1) {
-		c = getopt(argc, argv, "-b:c:DhJ:j:KOp:qr:t:VX");
+		c = getopt(argc, argv, "-b:c:DhJ:j:KOo:p:qr:t:VX");
 		if (c == -1)
 			break;
 
@@ -197,6 +287,10 @@ static void opts_get(int argc, char *argv[], struct mkfs_opts *opts)
 			opts->rgsize = atoi(optarg);
 			opts->got_rgsize = 1;
 			break;
+		case 'o':
+			/* Defer until the other options are gathered */
+			o = optarg;
+			break;
 		case 'V':
 			printf("mkfs.gfs2 %s (built %s %s)\n", VERSION,
 			       __DATE__, __TIME__);
@@ -228,6 +322,8 @@ static void opts_get(int argc, char *argv[], struct mkfs_opts *opts)
 			break;
 		};
 	}
+	if (o)
+		opt_parse_options(o, opts);
 }
 
 /**
@@ -655,6 +751,8 @@ static void sbd_init(struct gfs2_sbd *sdp, struct mkfs_opts *opts, struct lgfs2_
 		printf("  rgsize = %u\n", sdp->rgsize);
 		printf("  table = %s\n", sdp->locktable);
 		printf("  fssize = %"PRIu64"\n", opts->fssize);
+		printf("  sunit = %u\n", opts->sunit);
+		printf("  swidth = %u\n", opts->swidth);
 	}
 }
 
@@ -673,6 +771,11 @@ void main_mkfs(int argc, char *argv[])
 	opts_init(&opts);
 	opts_get(argc, argv, &opts);
 
+	if (!opts.got_device) {
+		fprintf(stderr, _("No device specified. Use -h for help\n"));
+		exit(1);
+	}
+
 	fd = open(opts.device, O_RDWR | O_CLOEXEC);
 	if (fd < 0){
 		perror(opts.device);
-- 
1.8.1.4



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Cluster-devel] [PATCH 1/6] mkfs.gfs2: Add options for stripe size and width
  2013-05-14 10:45 ` [Cluster-devel] [PATCH 1/6] mkfs.gfs2: Add options for stripe size and width Andrew Price
@ 2013-05-14 10:49   ` Steven Whitehouse
  2013-05-14 11:21     ` Andrew Price
  2013-05-14 12:53     ` [Cluster-devel] [PATCH 1/6 v2] " Andrew Price
  0 siblings, 2 replies; 15+ messages in thread
From: Steven Whitehouse @ 2013-05-14 10:49 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Hi,

On Tue, 2013-05-14 at 11:45 +0100, Andrew Price wrote:
> Add generic parsing of options passed in with -o and use it to accept
> sunit and swidth options (names chosen to be the same as mkfs.xfs). We
> don't do anything with these options yet.
> 
> Signed-off-by: Andrew Price <anprice@redhat.com>
> ---
>  gfs2/mkfs/main_mkfs.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 105 insertions(+), 2 deletions(-)
> 
> diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
> index 610a641..8d5549d 100644
> --- a/gfs2/mkfs/main_mkfs.c
> +++ b/gfs2/mkfs/main_mkfs.c
> @@ -19,6 +19,7 @@
>  #include <sys/time.h>
>  #include <libintl.h>
>  #include <sys/ioctl.h>
> +#include <limits.h>
>  
>  #define _(String) gettext(String)
>  
> @@ -55,6 +56,7 @@ static void print_usage(const char *prog_name)
>  	    "-j", _("<number>"), _("Number of journals"),
>  	    "-K", NULL,          _("Don't try to discard unused blocks"),
>  	    "-O", NULL,          _("Don't ask for confirmation"),
> +	    "-o", _("<key>[=<value>][,...]"), _("Specify extended options"),
>  	    "-p", _("<name>"),   _("Name of the locking protocol"),
>  	    "-q", NULL,          _("Don't print anything"),
>  	    "-r", _("<size>"),   _("Size of resource groups, in megabytes"),
> @@ -73,7 +75,7 @@ static void print_usage(const char *prog_name)
>  		option = options[i];
>  		param = options[i+1];
>  		desc = options[i+2];
> -		printf("%3s %-15s %s\n", option, param ? param : "", desc);
> +		printf("%3s %-22s %s\n", option, param ? param : "", desc);
>  	}
>  }
>  
> @@ -82,6 +84,8 @@ struct mkfs_opts {
>  	unsigned qcsize;
>  	unsigned jsize;
>  	unsigned rgsize;
> +	unsigned sunit;
> +	unsigned swidth;
>  	uint64_t fssize;
>  	uint32_t journals;
>  	const char *lockproto;
> @@ -93,6 +97,8 @@ struct mkfs_opts {
>  	unsigned got_qcsize:1;
>  	unsigned got_jsize:1;
>  	unsigned got_rgsize:1;
> +	unsigned got_sunit:1;
> +	unsigned got_swidth:1;
>  	unsigned got_fssize:1;
>  	unsigned got_journals:1;
>  	unsigned got_lockproto:1;
> @@ -145,11 +151,95 @@ static int discard_blocks(int fd, uint64_t len, int debug)
>          return 0;
>  }
>  
> +/**
> + * Convert a human-readable size string to a long long.
> + * Copied and adapted from xfs_mkfs.c.
> + */
> +static long long cvtnum(unsigned int blocksize, unsigned int sectorsize, char *s)
> +{
> +        long long i;
> +        char *sp;
> +
> +        i = strtoll(s, &sp, 0);
> +        if (i == 0 && sp == s)
> +                return -1LL;
> +        if (*sp == '\0')
> +                return i;
> +
> +	*sp = tolower(*sp);
> +        if (*sp == 'b' && sp[1] == '\0') {
> +                if (blocksize)
> +                        return i * blocksize;
> +                fprintf(stderr, _("Block size not available yet.\n"));
> +		exit(1);
> +        }
> +        if (*sp == 's' && sp[1] == '\0') {
> +                if (sectorsize)
> +                        return i * sectorsize;
> +                return i * GFS2_BASIC_BLOCK;
> +        }
> +        if (*sp == 'k' && sp[1] == '\0')
> +                return 1024LL * i;
> +        if (*sp == 'm' && sp[1] == '\0')
> +                return 1024LL * 1024LL * i;
> +        if (*sp == 'g' && sp[1] == '\0')
> +                return 1024LL * 1024LL * 1024LL * i;
> +        if (*sp == 't' && sp[1] == '\0')
> +                return 1024LL * 1024LL * 1024LL * 1024LL * i;
> +        if (*sp == 'p' && sp[1] == '\0')
> +                return 1024LL * 1024LL * 1024LL * 1024LL * 1024LL * i;
> +        if (*sp == 'e' && sp[1] == '\0')
> +                return 1024LL * 1024LL * 1024LL * 1024LL * 1024LL * 1024LL * i;
> +        return -1LL;
> +}
> +
> +static void opt_tok_unsigned(struct mkfs_opts *opts, char *s, unsigned *n, const char *key)
> +{
> +	long long l;
> +	char *val = strtok_r(NULL, "=", &s);
> +	if (val == NULL || *val == '\0') {
> +		fprintf(stderr, _("Missing argument to '%s'\n"), key);
> +		exit(-1);
> +	}
> +	l = cvtnum(opts->bsize, 0, val);
> +	if (l > UINT_MAX || l < 0) {
> +		fprintf(stderr, _("Value of '%s' is invalid\n"), key);
> +		exit(-1);
> +	}
> +	*n = (unsigned)l;
> +}
> +
> +static void opt_parse_options(char *str, struct mkfs_opts *opts)
> +{
> +	char *tok;
> +	char *key;
> +	char *o, *a;
> +
> +	for (tok = strtok_r(str, ",", &o); tok != NULL; tok = strtok_r(NULL, ",", &o)) {
> +		key = strtok_r(tok, "=", &a);
> +		if (key == NULL || *key == '\0') {
> +			fprintf(stderr, _("Missing argument to '-o' option\n"));
> +			exit(-1);
> +		}
> +		if (strcmp("sunit", key) == 0) {
> +			opt_tok_unsigned(opts, a, &opts->sunit, "sunit");
> +			opts->got_sunit = 1;
> +		} else if (strcmp("swidth", key) == 0) {
> +			opt_tok_unsigned(opts, a, &opts->swidth, "swidth");
> +			opts->got_swidth = 1;
> +		} else {
> +			fprintf(stderr, _("Invalid option '%s'\n"), key);
> +			exit(-1);
> +		}
> +	}
> +}
> +
Hmm, strsep maybe better than strtok_r perhaps?

>  static void opts_get(int argc, char *argv[], struct mkfs_opts *opts)
>  {
>  	int c;
> +	char *o = NULL;
>  	while (1) {
> -		c = getopt(argc, argv, "-b:c:DhJ:j:KOp:qr:t:VX");
> +		c = getopt(argc, argv, "-b:c:DhJ:j:KOo:p:qr:t:VX");
>  		if (c == -1)
>  			break;
>  
> @@ -197,6 +287,10 @@ static void opts_get(int argc, char *argv[], struct mkfs_opts *opts)
>  			opts->rgsize = atoi(optarg);
>  			opts->got_rgsize = 1;
>  			break;
> +		case 'o':
> +			/* Defer until the other options are gathered */
> +			o = optarg;
> +			break;
>  		case 'V':
>  			printf("mkfs.gfs2 %s (built %s %s)\n", VERSION,
>  			       __DATE__, __TIME__);
> @@ -228,6 +322,8 @@ static void opts_get(int argc, char *argv[], struct mkfs_opts *opts)
>  			break;
>  		};
>  	}
> +	if (o)
> +		opt_parse_options(o, opts);

Looks like this might silently ignore multiple -o options, only parsing
the last one?

>  }
>  
>  /**
> @@ -655,6 +751,8 @@ static void sbd_init(struct gfs2_sbd *sdp, struct mkfs_opts *opts, struct lgfs2_
>  		printf("  rgsize = %u\n", sdp->rgsize);
>  		printf("  table = %s\n", sdp->locktable);
>  		printf("  fssize = %"PRIu64"\n", opts->fssize);
> +		printf("  sunit = %u\n", opts->sunit);
> +		printf("  swidth = %u\n", opts->swidth);
>  	}
>  }
>  
> @@ -673,6 +771,11 @@ void main_mkfs(int argc, char *argv[])
>  	opts_init(&opts);
>  	opts_get(argc, argv, &opts);
>  
> +	if (!opts.got_device) {
> +		fprintf(stderr, _("No device specified. Use -h for help\n"));
> +		exit(1);
> +	}
> +
>  	fd = open(opts.device, O_RDWR | O_CLOEXEC);
>  	if (fd < 0){
>  		perror(opts.device);

Otherwise looks good,

Steve.




^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Cluster-devel] [PATCH 1/6] mkfs.gfs2: Add options for stripe size and width
  2013-05-14 10:49   ` Steven Whitehouse
@ 2013-05-14 11:21     ` Andrew Price
  2013-05-14 12:53     ` [Cluster-devel] [PATCH 1/6 v2] " Andrew Price
  1 sibling, 0 replies; 15+ messages in thread
From: Andrew Price @ 2013-05-14 11:21 UTC (permalink / raw)
  To: cluster-devel.redhat.com

On 14/05/13 11:49, Steven Whitehouse wrote:
> Hi,
>
> On Tue, 2013-05-14 at 11:45 +0100, Andrew Price wrote:
>> Add generic parsing of options passed in with -o and use it to accept
>> sunit and swidth options (names chosen to be the same as mkfs.xfs). We
>> don't do anything with these options yet.
>>
>> Signed-off-by: Andrew Price <anprice@redhat.com>
>> ---
>>   gfs2/mkfs/main_mkfs.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 105 insertions(+), 2 deletions(-)
>>
>> diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
>> index 610a641..8d5549d 100644
>> --- a/gfs2/mkfs/main_mkfs.c
>> +++ b/gfs2/mkfs/main_mkfs.c
>> @@ -19,6 +19,7 @@
>>   #include <sys/time.h>
>>   #include <libintl.h>
>>   #include <sys/ioctl.h>
>> +#include <limits.h>
>>
>>   #define _(String) gettext(String)
>>
>> @@ -55,6 +56,7 @@ static void print_usage(const char *prog_name)
>>   	    "-j", _("<number>"), _("Number of journals"),
>>   	    "-K", NULL,          _("Don't try to discard unused blocks"),
>>   	    "-O", NULL,          _("Don't ask for confirmation"),
>> +	    "-o", _("<key>[=<value>][,...]"), _("Specify extended options"),
>>   	    "-p", _("<name>"),   _("Name of the locking protocol"),
>>   	    "-q", NULL,          _("Don't print anything"),
>>   	    "-r", _("<size>"),   _("Size of resource groups, in megabytes"),
>> @@ -73,7 +75,7 @@ static void print_usage(const char *prog_name)
>>   		option = options[i];
>>   		param = options[i+1];
>>   		desc = options[i+2];
>> -		printf("%3s %-15s %s\n", option, param ? param : "", desc);
>> +		printf("%3s %-22s %s\n", option, param ? param : "", desc);
>>   	}
>>   }
>>
>> @@ -82,6 +84,8 @@ struct mkfs_opts {
>>   	unsigned qcsize;
>>   	unsigned jsize;
>>   	unsigned rgsize;
>> +	unsigned sunit;
>> +	unsigned swidth;
>>   	uint64_t fssize;
>>   	uint32_t journals;
>>   	const char *lockproto;
>> @@ -93,6 +97,8 @@ struct mkfs_opts {
>>   	unsigned got_qcsize:1;
>>   	unsigned got_jsize:1;
>>   	unsigned got_rgsize:1;
>> +	unsigned got_sunit:1;
>> +	unsigned got_swidth:1;
>>   	unsigned got_fssize:1;
>>   	unsigned got_journals:1;
>>   	unsigned got_lockproto:1;
>> @@ -145,11 +151,95 @@ static int discard_blocks(int fd, uint64_t len, int debug)
>>           return 0;
>>   }
>>
>> +/**
>> + * Convert a human-readable size string to a long long.
>> + * Copied and adapted from xfs_mkfs.c.
>> + */
>> +static long long cvtnum(unsigned int blocksize, unsigned int sectorsize, char *s)
>> +{
>> +        long long i;
>> +        char *sp;
>> +
>> +        i = strtoll(s, &sp, 0);
>> +        if (i == 0 && sp == s)
>> +                return -1LL;
>> +        if (*sp == '\0')
>> +                return i;
>> +
>> +	*sp = tolower(*sp);
>> +        if (*sp == 'b' && sp[1] == '\0') {
>> +                if (blocksize)
>> +                        return i * blocksize;
>> +                fprintf(stderr, _("Block size not available yet.\n"));
>> +		exit(1);
>> +        }
>> +        if (*sp == 's' && sp[1] == '\0') {
>> +                if (sectorsize)
>> +                        return i * sectorsize;
>> +                return i * GFS2_BASIC_BLOCK;
>> +        }
>> +        if (*sp == 'k' && sp[1] == '\0')
>> +                return 1024LL * i;
>> +        if (*sp == 'm' && sp[1] == '\0')
>> +                return 1024LL * 1024LL * i;
>> +        if (*sp == 'g' && sp[1] == '\0')
>> +                return 1024LL * 1024LL * 1024LL * i;
>> +        if (*sp == 't' && sp[1] == '\0')
>> +                return 1024LL * 1024LL * 1024LL * 1024LL * i;
>> +        if (*sp == 'p' && sp[1] == '\0')
>> +                return 1024LL * 1024LL * 1024LL * 1024LL * 1024LL * i;
>> +        if (*sp == 'e' && sp[1] == '\0')
>> +                return 1024LL * 1024LL * 1024LL * 1024LL * 1024LL * 1024LL * i;
>> +        return -1LL;
>> +}
>> +
>> +static void opt_tok_unsigned(struct mkfs_opts *opts, char *s, unsigned *n, const char *key)
>> +{
>> +	long long l;
>> +	char *val = strtok_r(NULL, "=", &s);
>> +	if (val == NULL || *val == '\0') {
>> +		fprintf(stderr, _("Missing argument to '%s'\n"), key);
>> +		exit(-1);
>> +	}
>> +	l = cvtnum(opts->bsize, 0, val);
>> +	if (l > UINT_MAX || l < 0) {
>> +		fprintf(stderr, _("Value of '%s' is invalid\n"), key);
>> +		exit(-1);
>> +	}
>> +	*n = (unsigned)l;
>> +}
>> +
>> +static void opt_parse_options(char *str, struct mkfs_opts *opts)
>> +{
>> +	char *tok;
>> +	char *key;
>> +	char *o, *a;
>> +
>> +	for (tok = strtok_r(str, ",", &o); tok != NULL; tok = strtok_r(NULL, ",", &o)) {
>> +		key = strtok_r(tok, "=", &a);
>> +		if (key == NULL || *key == '\0') {
>> +			fprintf(stderr, _("Missing argument to '-o' option\n"));
>> +			exit(-1);
>> +		}
>> +		if (strcmp("sunit", key) == 0) {
>> +			opt_tok_unsigned(opts, a, &opts->sunit, "sunit");
>> +			opts->got_sunit = 1;
>> +		} else if (strcmp("swidth", key) == 0) {
>> +			opt_tok_unsigned(opts, a, &opts->swidth, "swidth");
>> +			opts->got_swidth = 1;
>> +		} else {
>> +			fprintf(stderr, _("Invalid option '%s'\n"), key);
>> +			exit(-1);
>> +		}
>> +	}
>> +}
>> +
> Hmm, strsep maybe better than strtok_r perhaps?

It could be cleaner I guess. I'll give it a try.

>>   static void opts_get(int argc, char *argv[], struct mkfs_opts *opts)
>>   {
>>   	int c;
>> +	char *o = NULL;
>>   	while (1) {
>> -		c = getopt(argc, argv, "-b:c:DhJ:j:KOp:qr:t:VX");
>> +		c = getopt(argc, argv, "-b:c:DhJ:j:KOo:p:qr:t:VX");
>>   		if (c == -1)
>>   			break;
>>
>> @@ -197,6 +287,10 @@ static void opts_get(int argc, char *argv[], struct mkfs_opts *opts)
>>   			opts->rgsize = atoi(optarg);
>>   			opts->got_rgsize = 1;
>>   			break;
>> +		case 'o':
>> +			/* Defer until the other options are gathered */
>> +			o = optarg;
>> +			break;
>>   		case 'V':
>>   			printf("mkfs.gfs2 %s (built %s %s)\n", VERSION,
>>   			       __DATE__, __TIME__);
>> @@ -228,6 +322,8 @@ static void opts_get(int argc, char *argv[], struct mkfs_opts *opts)
>>   			break;
>>   		};
>>   	}
>> +	if (o)
>> +		opt_parse_options(o, opts);
>
> Looks like this might silently ignore multiple -o options, only parsing
> the last one?

Hm good point, I hadn't thought of multiple -o's. Will redo this one.

Thanks,
Andy

>
>>   }
>>
>>   /**
>> @@ -655,6 +751,8 @@ static void sbd_init(struct gfs2_sbd *sdp, struct mkfs_opts *opts, struct lgfs2_
>>   		printf("  rgsize = %u\n", sdp->rgsize);
>>   		printf("  table = %s\n", sdp->locktable);
>>   		printf("  fssize = %"PRIu64"\n", opts->fssize);
>> +		printf("  sunit = %u\n", opts->sunit);
>> +		printf("  swidth = %u\n", opts->swidth);
>>   	}
>>   }
>>
>> @@ -673,6 +771,11 @@ void main_mkfs(int argc, char *argv[])
>>   	opts_init(&opts);
>>   	opts_get(argc, argv, &opts);
>>
>> +	if (!opts.got_device) {
>> +		fprintf(stderr, _("No device specified. Use -h for help\n"));
>> +		exit(1);
>> +	}
>> +
>>   	fd = open(opts.device, O_RDWR | O_CLOEXEC);
>>   	if (fd < 0){
>>   		perror(opts.device);
>
> Otherwise looks good,
>
> Steve.
>
>



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Cluster-devel] [PATCH 1/6 v2] mkfs.gfs2: Add options for stripe size and width
  2013-05-14 10:49   ` Steven Whitehouse
  2013-05-14 11:21     ` Andrew Price
@ 2013-05-14 12:53     ` Andrew Price
  2013-05-14 12:52       ` Steven Whitehouse
  1 sibling, 1 reply; 15+ messages in thread
From: Andrew Price @ 2013-05-14 12:53 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Add generic parsing of options passed in with -o and use it to accept
sunit and swidth options (names chosen to be the same as mkfs.xfs). We
don't do anything with these options yet.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/mkfs/main_mkfs.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 98 insertions(+), 2 deletions(-)

diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
index 610a641..6471600 100644
--- a/gfs2/mkfs/main_mkfs.c
+++ b/gfs2/mkfs/main_mkfs.c
@@ -19,6 +19,7 @@
 #include <sys/time.h>
 #include <libintl.h>
 #include <sys/ioctl.h>
+#include <limits.h>
 
 #define _(String) gettext(String)
 
@@ -55,6 +56,7 @@ static void print_usage(const char *prog_name)
 	    "-j", _("<number>"), _("Number of journals"),
 	    "-K", NULL,          _("Don't try to discard unused blocks"),
 	    "-O", NULL,          _("Don't ask for confirmation"),
+	    "-o", _("<key>[=<value>][,...]"), _("Specify extended options"),
 	    "-p", _("<name>"),   _("Name of the locking protocol"),
 	    "-q", NULL,          _("Don't print anything"),
 	    "-r", _("<size>"),   _("Size of resource groups, in megabytes"),
@@ -73,7 +75,7 @@ static void print_usage(const char *prog_name)
 		option = options[i];
 		param = options[i+1];
 		desc = options[i+2];
-		printf("%3s %-15s %s\n", option, param ? param : "", desc);
+		printf("%3s %-22s %s\n", option, param ? param : "", desc);
 	}
 }
 
@@ -82,6 +84,8 @@ struct mkfs_opts {
 	unsigned qcsize;
 	unsigned jsize;
 	unsigned rgsize;
+	unsigned sunit;
+	unsigned swidth;
 	uint64_t fssize;
 	uint32_t journals;
 	const char *lockproto;
@@ -93,6 +97,8 @@ struct mkfs_opts {
 	unsigned got_qcsize:1;
 	unsigned got_jsize:1;
 	unsigned got_rgsize:1;
+	unsigned got_sunit:1;
+	unsigned got_swidth:1;
 	unsigned got_fssize:1;
 	unsigned got_journals:1;
 	unsigned got_lockproto:1;
@@ -145,11 +151,91 @@ static int discard_blocks(int fd, uint64_t len, int debug)
         return 0;
 }
 
+/**
+ * Convert a human-readable size string to a long long.
+ * Copied and adapted from xfs_mkfs.c.
+ */
+static long long cvtnum(unsigned int blocksize, unsigned int sectorsize, const char *s)
+{
+        long long i;
+        char *sp;
+
+        i = strtoll(s, &sp, 0);
+        if (i == 0 && sp == s)
+                return -1LL;
+        if (*sp == '\0')
+                return i;
+
+	*sp = tolower(*sp);
+        if (*sp == 'b' && sp[1] == '\0') {
+                if (blocksize)
+                        return i * blocksize;
+                fprintf(stderr, _("Block size not available yet.\n"));
+		exit(1);
+        }
+        if (*sp == 's' && sp[1] == '\0') {
+                if (sectorsize)
+                        return i * sectorsize;
+                return i * GFS2_BASIC_BLOCK;
+        }
+        if (*sp == 'k' && sp[1] == '\0')
+                return 1024LL * i;
+        if (*sp == 'm' && sp[1] == '\0')
+                return 1024LL * 1024LL * i;
+        if (*sp == 'g' && sp[1] == '\0')
+                return 1024LL * 1024LL * 1024LL * i;
+        if (*sp == 't' && sp[1] == '\0')
+                return 1024LL * 1024LL * 1024LL * 1024LL * i;
+        if (*sp == 'p' && sp[1] == '\0')
+                return 1024LL * 1024LL * 1024LL * 1024LL * 1024LL * i;
+        if (*sp == 'e' && sp[1] == '\0')
+                return 1024LL * 1024LL * 1024LL * 1024LL * 1024LL * 1024LL * i;
+        return -1LL;
+}
+
+static void parse_unsigned(struct mkfs_opts *opts, const char *key, const char *val, unsigned *n)
+{
+	long long l;
+	if (val == NULL || *val == '\0') {
+		fprintf(stderr, _("Missing argument to '%s'\n"), key);
+		exit(-1);
+	}
+	l = cvtnum(opts->bsize, 0, val);
+	if (l > UINT_MAX || l < 0) {
+		fprintf(stderr, _("Value of '%s' is invalid\n"), key);
+		exit(-1);
+	}
+	*n = (unsigned)l;
+}
+
+static void opt_parse_extended(char *str, struct mkfs_opts *opts)
+{
+	char *opt;
+	while ((opt = strsep(&str, ",")) != NULL) {
+		char *key = strsep(&opt, "=");
+		char *val = strsep(&opt, "=");
+		if (key == NULL || *key == '\0') {
+			fprintf(stderr, _("Missing argument to '-o' option\n"));
+			exit(-1);
+		}
+		if (strcmp("sunit", key) == 0) {
+			parse_unsigned(opts, "sunit", val, &opts->sunit);
+			opts->got_sunit = 1;
+		} else if (strcmp("swidth", key) == 0) {
+			parse_unsigned(opts, "swidth", val, &opts->swidth);
+			opts->got_swidth = 1;
+		} else {
+			fprintf(stderr, _("Invalid option '%s'\n"), key);
+			exit(-1);
+		}
+	}
+}
+
 static void opts_get(int argc, char *argv[], struct mkfs_opts *opts)
 {
 	int c;
 	while (1) {
-		c = getopt(argc, argv, "-b:c:DhJ:j:KOp:qr:t:VX");
+		c = getopt(argc, argv, "-b:c:DhJ:j:KOo:p:qr:t:VX");
 		if (c == -1)
 			break;
 
@@ -197,6 +283,9 @@ static void opts_get(int argc, char *argv[], struct mkfs_opts *opts)
 			opts->rgsize = atoi(optarg);
 			opts->got_rgsize = 1;
 			break;
+		case 'o':
+			opt_parse_extended(optarg, opts);
+			break;
 		case 'V':
 			printf("mkfs.gfs2 %s (built %s %s)\n", VERSION,
 			       __DATE__, __TIME__);
@@ -655,6 +744,8 @@ static void sbd_init(struct gfs2_sbd *sdp, struct mkfs_opts *opts, struct lgfs2_
 		printf("  rgsize = %u\n", sdp->rgsize);
 		printf("  table = %s\n", sdp->locktable);
 		printf("  fssize = %"PRIu64"\n", opts->fssize);
+		printf("  sunit = %u\n", opts->sunit);
+		printf("  swidth = %u\n", opts->swidth);
 	}
 }
 
@@ -673,6 +764,11 @@ void main_mkfs(int argc, char *argv[])
 	opts_init(&opts);
 	opts_get(argc, argv, &opts);
 
+	if (!opts.got_device) {
+		fprintf(stderr, _("No device specified. Use -h for help\n"));
+		exit(1);
+	}
+
 	fd = open(opts.device, O_RDWR | O_CLOEXEC);
 	if (fd < 0){
 		perror(opts.device);
-- 
1.8.1.4



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Cluster-devel] [PATCH 1/6 v2] mkfs.gfs2: Add options for stripe size and width
  2013-05-14 12:53     ` [Cluster-devel] [PATCH 1/6 v2] " Andrew Price
@ 2013-05-14 12:52       ` Steven Whitehouse
  0 siblings, 0 replies; 15+ messages in thread
From: Steven Whitehouse @ 2013-05-14 12:52 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Hi,

That looks good. Thanks,

Steve.

On Tue, 2013-05-14 at 13:53 +0100, Andrew Price wrote:
> Add generic parsing of options passed in with -o and use it to accept
> sunit and swidth options (names chosen to be the same as mkfs.xfs). We
> don't do anything with these options yet.
> 
> Signed-off-by: Andrew Price <anprice@redhat.com>
> ---
>  gfs2/mkfs/main_mkfs.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 98 insertions(+), 2 deletions(-)
> 
> diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
> index 610a641..6471600 100644
> --- a/gfs2/mkfs/main_mkfs.c
> +++ b/gfs2/mkfs/main_mkfs.c
> @@ -19,6 +19,7 @@
>  #include <sys/time.h>
>  #include <libintl.h>
>  #include <sys/ioctl.h>
> +#include <limits.h>
>  
>  #define _(String) gettext(String)
>  
> @@ -55,6 +56,7 @@ static void print_usage(const char *prog_name)
>  	    "-j", _("<number>"), _("Number of journals"),
>  	    "-K", NULL,          _("Don't try to discard unused blocks"),
>  	    "-O", NULL,          _("Don't ask for confirmation"),
> +	    "-o", _("<key>[=<value>][,...]"), _("Specify extended options"),
>  	    "-p", _("<name>"),   _("Name of the locking protocol"),
>  	    "-q", NULL,          _("Don't print anything"),
>  	    "-r", _("<size>"),   _("Size of resource groups, in megabytes"),
> @@ -73,7 +75,7 @@ static void print_usage(const char *prog_name)
>  		option = options[i];
>  		param = options[i+1];
>  		desc = options[i+2];
> -		printf("%3s %-15s %s\n", option, param ? param : "", desc);
> +		printf("%3s %-22s %s\n", option, param ? param : "", desc);
>  	}
>  }
>  
> @@ -82,6 +84,8 @@ struct mkfs_opts {
>  	unsigned qcsize;
>  	unsigned jsize;
>  	unsigned rgsize;
> +	unsigned sunit;
> +	unsigned swidth;
>  	uint64_t fssize;
>  	uint32_t journals;
>  	const char *lockproto;
> @@ -93,6 +97,8 @@ struct mkfs_opts {
>  	unsigned got_qcsize:1;
>  	unsigned got_jsize:1;
>  	unsigned got_rgsize:1;
> +	unsigned got_sunit:1;
> +	unsigned got_swidth:1;
>  	unsigned got_fssize:1;
>  	unsigned got_journals:1;
>  	unsigned got_lockproto:1;
> @@ -145,11 +151,91 @@ static int discard_blocks(int fd, uint64_t len, int debug)
>          return 0;
>  }
>  
> +/**
> + * Convert a human-readable size string to a long long.
> + * Copied and adapted from xfs_mkfs.c.
> + */
> +static long long cvtnum(unsigned int blocksize, unsigned int sectorsize, const char *s)
> +{
> +        long long i;
> +        char *sp;
> +
> +        i = strtoll(s, &sp, 0);
> +        if (i == 0 && sp == s)
> +                return -1LL;
> +        if (*sp == '\0')
> +                return i;
> +
> +	*sp = tolower(*sp);
> +        if (*sp == 'b' && sp[1] == '\0') {
> +                if (blocksize)
> +                        return i * blocksize;
> +                fprintf(stderr, _("Block size not available yet.\n"));
> +		exit(1);
> +        }
> +        if (*sp == 's' && sp[1] == '\0') {
> +                if (sectorsize)
> +                        return i * sectorsize;
> +                return i * GFS2_BASIC_BLOCK;
> +        }
> +        if (*sp == 'k' && sp[1] == '\0')
> +                return 1024LL * i;
> +        if (*sp == 'm' && sp[1] == '\0')
> +                return 1024LL * 1024LL * i;
> +        if (*sp == 'g' && sp[1] == '\0')
> +                return 1024LL * 1024LL * 1024LL * i;
> +        if (*sp == 't' && sp[1] == '\0')
> +                return 1024LL * 1024LL * 1024LL * 1024LL * i;
> +        if (*sp == 'p' && sp[1] == '\0')
> +                return 1024LL * 1024LL * 1024LL * 1024LL * 1024LL * i;
> +        if (*sp == 'e' && sp[1] == '\0')
> +                return 1024LL * 1024LL * 1024LL * 1024LL * 1024LL * 1024LL * i;
> +        return -1LL;
> +}
> +
> +static void parse_unsigned(struct mkfs_opts *opts, const char *key, const char *val, unsigned *n)
> +{
> +	long long l;
> +	if (val == NULL || *val == '\0') {
> +		fprintf(stderr, _("Missing argument to '%s'\n"), key);
> +		exit(-1);
> +	}
> +	l = cvtnum(opts->bsize, 0, val);
> +	if (l > UINT_MAX || l < 0) {
> +		fprintf(stderr, _("Value of '%s' is invalid\n"), key);
> +		exit(-1);
> +	}
> +	*n = (unsigned)l;
> +}
> +
> +static void opt_parse_extended(char *str, struct mkfs_opts *opts)
> +{
> +	char *opt;
> +	while ((opt = strsep(&str, ",")) != NULL) {
> +		char *key = strsep(&opt, "=");
> +		char *val = strsep(&opt, "=");
> +		if (key == NULL || *key == '\0') {
> +			fprintf(stderr, _("Missing argument to '-o' option\n"));
> +			exit(-1);
> +		}
> +		if (strcmp("sunit", key) == 0) {
> +			parse_unsigned(opts, "sunit", val, &opts->sunit);
> +			opts->got_sunit = 1;
> +		} else if (strcmp("swidth", key) == 0) {
> +			parse_unsigned(opts, "swidth", val, &opts->swidth);
> +			opts->got_swidth = 1;
> +		} else {
> +			fprintf(stderr, _("Invalid option '%s'\n"), key);
> +			exit(-1);
> +		}
> +	}
> +}
> +
>  static void opts_get(int argc, char *argv[], struct mkfs_opts *opts)
>  {
>  	int c;
>  	while (1) {
> -		c = getopt(argc, argv, "-b:c:DhJ:j:KOp:qr:t:VX");
> +		c = getopt(argc, argv, "-b:c:DhJ:j:KOo:p:qr:t:VX");
>  		if (c == -1)
>  			break;
>  
> @@ -197,6 +283,9 @@ static void opts_get(int argc, char *argv[], struct mkfs_opts *opts)
>  			opts->rgsize = atoi(optarg);
>  			opts->got_rgsize = 1;
>  			break;
> +		case 'o':
> +			opt_parse_extended(optarg, opts);
> +			break;
>  		case 'V':
>  			printf("mkfs.gfs2 %s (built %s %s)\n", VERSION,
>  			       __DATE__, __TIME__);
> @@ -655,6 +744,8 @@ static void sbd_init(struct gfs2_sbd *sdp, struct mkfs_opts *opts, struct lgfs2_
>  		printf("  rgsize = %u\n", sdp->rgsize);
>  		printf("  table = %s\n", sdp->locktable);
>  		printf("  fssize = %"PRIu64"\n", opts->fssize);
> +		printf("  sunit = %u\n", opts->sunit);
> +		printf("  swidth = %u\n", opts->swidth);
>  	}
>  }
>  
> @@ -673,6 +764,11 @@ void main_mkfs(int argc, char *argv[])
>  	opts_init(&opts);
>  	opts_get(argc, argv, &opts);
>  
> +	if (!opts.got_device) {
> +		fprintf(stderr, _("No device specified. Use -h for help\n"));
> +		exit(1);
> +	}
> +
>  	fd = open(opts.device, O_RDWR | O_CLOEXEC);
>  	if (fd < 0){
>  		perror(opts.device);




^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Cluster-devel] [PATCH 2/6] libgfs2: Remove 'writes' field from gfs2_sbd
  2013-05-14 10:45 [Cluster-devel] [PATCH 0/6] More mkfs.gfs2 reworking Andrew Price
  2013-05-14 10:45 ` [Cluster-devel] [PATCH 1/6] mkfs.gfs2: Add options for stripe size and width Andrew Price
@ 2013-05-14 10:45 ` Andrew Price
  2013-05-14 10:45 ` [Cluster-devel] [PATCH 3/6] mkfs.gfs2: Link to libblkid Andrew Price
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Andrew Price @ 2013-05-14 10:45 UTC (permalink / raw)
  To: cluster-devel.redhat.com

This was only used to print out how many writes mkfs.gfs2 had made
during its run in debug mode, which doesn't really make sense any more
as we don't use bwrite for much of the mkfs process now.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/libgfs2/buf.c     | 1 -
 gfs2/libgfs2/libgfs2.h | 1 -
 gfs2/mkfs/main_mkfs.c  | 5 -----
 3 files changed, 7 deletions(-)

diff --git a/gfs2/libgfs2/buf.c b/gfs2/libgfs2/buf.c
index 68f0731..6fcdd17 100644
--- a/gfs2/libgfs2/buf.c
+++ b/gfs2/libgfs2/buf.c
@@ -80,7 +80,6 @@ int bwrite(struct gfs2_buffer_head *bh)
 
 	if (pwritev(sdp->device_fd, &bh->iov, 1, bh->b_blocknr * sdp->bsize) != bh->iov.iov_len)
 		return -1;
-	sdp->writes++;
 	bh->b_modified = 0;
 	return 0;
 }
diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index 7d2d9ff..e069e9d 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -311,7 +311,6 @@ struct gfs2_sbd {
 	struct gfs2_inode *master_dir;
 	struct master_dir md;
 
-	unsigned int writes;
 	int metafs_fd;
 	char metafs_path[PATH_MAX]; /* where metafs is mounted */
 	struct special_blocks eattr_blocks;
diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
index 8d5549d..9949a46 100644
--- a/gfs2/mkfs/main_mkfs.c
+++ b/gfs2/mkfs/main_mkfs.c
@@ -590,15 +590,10 @@ static void print_results(struct gfs2_sbd *sdp, uint64_t real_device_size,
 	printf("%-27s%llu\n", _("Resource groups:"), (unsigned long long)sdp->rgrps);
 	printf("%-27s\"%s\"\n", _("Locking protocol:"), sdp->lockproto);
 	printf("%-27s\"%s\"\n", _("Lock table:"), sdp->locktable);
-
-	if (opts->debug) {
-		printf("%-27s%u\n", _("Writes:"), sdp->writes);
-	}
 	/* Translators: "UUID" = universally unique identifier. */
 	printf("%-27s%s\n", _("UUID:"), str_uuid(uuid));
 }
 
-
 /**
  * If path is a symlink, return 1 with *abspath pointing to the absolute path,
  * otherwise return 0. Exit on errors. The caller must free the memory pointed
-- 
1.8.1.4



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Cluster-devel] [PATCH 3/6] mkfs.gfs2: Link to libblkid
  2013-05-14 10:45 [Cluster-devel] [PATCH 0/6] More mkfs.gfs2 reworking Andrew Price
  2013-05-14 10:45 ` [Cluster-devel] [PATCH 1/6] mkfs.gfs2: Add options for stripe size and width Andrew Price
  2013-05-14 10:45 ` [Cluster-devel] [PATCH 2/6] libgfs2: Remove 'writes' field from gfs2_sbd Andrew Price
@ 2013-05-14 10:45 ` Andrew Price
  2013-05-14 10:45 ` [Cluster-devel] [PATCH 4/6] mkfs.gfs2: Use libblkid for checking contents Andrew Price
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Andrew Price @ 2013-05-14 10:45 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Add the build system bits to check for, and link mkfs.gfs2 to libblkid.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 configure.ac          | 1 +
 gfs2/mkfs/Makefile.am | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/configure.ac b/configure.ac
index 45b38eb..6940d79 100644
--- a/configure.ac
+++ b/configure.ac
@@ -126,6 +126,7 @@ PKG_CHECK_MODULES([check], [check >= 0.9.8],
 AM_CONDITIONAL([BUILD_TESTS], [test "x$have_check" = "xyes"])
 
 PKG_CHECK_MODULES([zlib],[zlib])
+PKG_CHECK_MODULES([blkid],[blkid])
 
 # old versions of ncurses don't ship pkg-config files
 PKG_CHECK_MODULES([ncurses],[ncurses],,
diff --git a/gfs2/mkfs/Makefile.am b/gfs2/mkfs/Makefile.am
index 3e72026..6ed89fa 100644
--- a/gfs2/mkfs/Makefile.am
+++ b/gfs2/mkfs/Makefile.am
@@ -20,6 +20,9 @@ mkfs_gfs2_CPPFLAGS	= -D_FILE_OFFSET_BITS=64 -D_LARGEFILE64_SOURCE \
 			  -I$(top_srcdir)/gfs2/include \
 			  -I$(top_srcdir)/gfs2/libgfs2
 
+mkfs_gfs2_CFLAGS	= $(blkid_CFLAGS)
+mkfs_gfs2_LDFLAGS	= $(blkid_LIBS)
+
 mkfs_gfs2_LDADD		= $(top_builddir)/gfs2/libgfs2/libgfs2.la
 
 relative_sbin		= $(shell perl -MFile::Spec -le 'print File::Spec->abs2rel("/sbin", "$(sbindir)")')
-- 
1.8.1.4



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Cluster-devel] [PATCH 4/6] mkfs.gfs2: Use libblkid for checking contents
  2013-05-14 10:45 [Cluster-devel] [PATCH 0/6] More mkfs.gfs2 reworking Andrew Price
                   ` (2 preceding siblings ...)
  2013-05-14 10:45 ` [Cluster-devel] [PATCH 3/6] mkfs.gfs2: Link to libblkid Andrew Price
@ 2013-05-14 10:45 ` Andrew Price
  2013-05-14 10:52   ` Steven Whitehouse
  2013-05-14 10:45 ` [Cluster-devel] [PATCH 5/6] mkfs.gfs2: Add a struct to store device info Andrew Price
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Andrew Price @ 2013-05-14 10:45 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Use libblkid to probe the contents of the target device instead of
execing 'file'.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/mkfs/main_mkfs.c | 181 ++++++++++++++------------------------------------
 1 file changed, 51 insertions(+), 130 deletions(-)

diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
index 9949a46..91c89d3 100644
--- a/gfs2/mkfs/main_mkfs.c
+++ b/gfs2/mkfs/main_mkfs.c
@@ -20,6 +20,7 @@
 #include <libintl.h>
 #include <sys/ioctl.h>
 #include <limits.h>
+#include <blkid.h>
 
 #define _(String) gettext(String)
 
@@ -123,6 +124,7 @@ static void opts_init(struct mkfs_opts *opts)
 	opts->rgsize = GFS2_DEFAULT_RGSIZE;
 	opts->lockproto = "lock_dlm";
 	opts->locktable = "";
+	opts->confirm = 1;
 }
 
 #ifndef BLKDISCARD
@@ -475,105 +477,6 @@ static void opts_check(struct mkfs_opts *opts)
 
 }
 
-static int get_file_output(int fd, char *buffer, size_t buflen)
-{
-	struct pollfd pf = { .fd = fd, .events = POLLIN|POLLRDHUP };
-	int flags;
-	int pos = 0;
-	int rv;
-
-	flags = fcntl(fd, F_GETFL, 0);
-	if (flags < 0)
-		return flags;
-
-	flags |= O_NONBLOCK;
-	rv = fcntl(fd, F_SETFL, flags);
-	if (rv < 0)
-		return rv;
-
-	while (1) {
-		rv = poll(&pf, 1, 10 * 1000);
-		if (rv == 0)
-			break;
-		if (rv < 0)
-			return rv;
-		if (pf.revents & POLLIN) {
-			rv = read(fd, buffer + pos,
-				   buflen - pos);
-			if (rv < 0) {
-				if (errno == EAGAIN)
-					continue;
-				return rv;
-			}
-			if (rv == 0)
-				break;
-			pos += rv;
-			if (pos >= buflen)
-				return -1;
-			buffer[pos] = 0;
-			continue;
-		}
-		if (pf.revents & (POLLRDHUP | POLLHUP | POLLERR))
-			break;
-	}
-	return 0;
-}
-
-static void check_dev_content(const char *devname)
-{
-	struct sigaction sa;
-	char content[1024] = { 0, };
-	char * args[] = {
-		(char *)"/usr/bin/file",
-		(char *)"-bsL",
-		(char *)devname,
-		NULL };
-	int p[2] = {-1, -1};
-	int ret;
-	int pid;
-
-	ret = sigaction(SIGCHLD, NULL, &sa);
-	if  (ret)
-		return;
-	sa.sa_handler = SIG_IGN;
-	sa.sa_flags |= (SA_NOCLDSTOP | SA_NOCLDWAIT);
-	ret = sigaction(SIGCHLD, &sa, NULL);
-	if (ret)
-		goto fail;
-
-	ret = pipe(p);
-	if (ret)
-		goto fail;
-
-	pid = fork();
-
-	if (pid < 0) {
-		close(p[1]);
-		goto fail;
-	}
-
-	if (pid) {
-		close(p[1]);
-		ret = get_file_output(p[0], content, sizeof(content));
-		if (ret) {
-fail:
-			printf( _("Content of file or device unknown (do you have GNU fileutils installed?)\n"));
-		} else {
-			if (*content == 0)
-				goto fail;
-			printf( _("It appears to contain: %s"), content);
-		}
-		if (p[0] >= 0)
-			close(p[0]);
-		return;
-	}
-
-	close(p[0]);
-	dup2(p[1], STDOUT_FILENO);
-	close(STDIN_FILENO);
-	exit(execv(args[0], args));
-}
-
 static void print_results(struct gfs2_sbd *sdp, uint64_t real_device_size,
                           struct mkfs_opts *opts, unsigned char uuid[16])
 {
@@ -594,30 +497,27 @@ static void print_results(struct gfs2_sbd *sdp, uint64_t real_device_size,
 	printf("%-27s%s\n", _("UUID:"), str_uuid(uuid));
 }
 
-/**
- * If path is a symlink, return 1 with *abspath pointing to the absolute path,
- * otherwise return 0. Exit on errors. The caller must free the memory pointed
- * to by *abspath.
- */
-static int is_symlink(const char *path, char **abspath)
+static void warn_of_destruction(const char *path)
 {
 	struct stat lnkstat;
+	char *abspath = NULL;
 
 	if (lstat(path, &lnkstat) == -1) {
 		perror(_("Failed to lstat the device"));
 		exit(EXIT_FAILURE);
 	}
-	if (!S_ISLNK(lnkstat.st_mode)) {
-		return 0;
-	}
-	*abspath = canonicalize_file_name(path);
-	if (*abspath == NULL) {
-		perror(_("Could not find the absolute path of the device"));
-		exit(EXIT_FAILURE);
+	if (S_ISLNK(lnkstat.st_mode)) {
+		abspath = canonicalize_file_name(path);
+		if (abspath == NULL) {
+			perror(_("Could not find the absolute path of the device"));
+			exit(EXIT_FAILURE);
+		}
+		/* Translators: Example: "/dev/vg/lv is a symbolic link to /dev/dm-2" */
+		printf( _("%s is a symbolic link to %s\n"), path, abspath);
+		path = abspath;
 	}
-	/* Translators: Example: "/dev/vg/lv is a symbolic link to /dev/dm-2" */
-	printf( _("%s is a symbolic link to %s\n"), path, *abspath);
-	return 1;
+	printf(_("This will destroy any data on %s\n"), path);
+	free(abspath);
 }
 
 static int writerg(int fd, const struct rgrp_tree *rgt, const unsigned bsize)
@@ -751,6 +651,38 @@ static void sbd_init(struct gfs2_sbd *sdp, struct mkfs_opts *opts, struct lgfs2_
 	}
 }
 
+static int probe_contents(int fd)
+{
+	int ret;
+	const char *contents;
+	blkid_probe pr = blkid_new_probe();
+	if (pr == NULL || blkid_probe_set_device(pr, fd, 0, 0) != 0
+	               || blkid_probe_enable_superblocks(pr, TRUE) != 0
+	               || blkid_probe_enable_partitions(pr, TRUE) != 0) {
+		fprintf(stderr, _("Failed to create probe\n"));
+		return -1;
+	}
+
+	ret = blkid_do_fullprobe(pr);
+	if (ret == -1) {
+		fprintf(stderr, _("Failed to probe device\n"));
+		return -1;
+	}
+
+	if (ret == 1)
+		return 0;
+
+	if (!blkid_probe_lookup_value(pr, "TYPE", &contents, NULL)) {
+		printf(_("It appears to contain an existing filesystem (%s)\n"), contents);
+	} else if (!blkid_probe_lookup_value(pr, "PTTYPE", &contents, NULL)) {
+		printf(_("It appears to contain a partition table (%s).\n"), contents);
+	}
+
+	blkid_free_probe(pr);
+
+	return 0;
+}
+
 void main_mkfs(int argc, char *argv[])
 {
 	struct gfs2_sbd sbd;
@@ -758,9 +690,6 @@ void main_mkfs(int argc, char *argv[])
 	struct lgfs2_dev_info dinfo;
 	int error;
 	unsigned char uuid[16];
-	char *absname = NULL;
-	char *fdpath = NULL;
-	int islnk = 0;
 	int fd;
 
 	opts_init(&opts);
@@ -783,21 +712,13 @@ void main_mkfs(int argc, char *argv[])
 	}
 
 	opts_check(&opts);
-	sbd_init(&sbd, &opts, &dinfo, fd);
+	warn_of_destruction(opts.device);
 
-	if (asprintf(&fdpath, "/proc/%d/fd/%d", getpid(), fd) < 0) {
-		perror(_("Failed to build string"));
+	error = probe_contents(fd);
+	if (error)
 		exit(EXIT_FAILURE);
-	}
 
-	if (!opts.override) {
-		islnk = is_symlink(opts.device, &absname);
-		printf(_("This will destroy any data on %s.\n"), islnk ? absname : opts.device);
-		free(absname);
-		check_dev_content(fdpath);
-		opts.confirm = 1;
-	}
-	free(fdpath);
+	sbd_init(&sbd, &opts, &dinfo, fd);
 
 	if (opts.confirm && !opts.override)
 		are_you_sure();
-- 
1.8.1.4



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Cluster-devel] [PATCH 4/6] mkfs.gfs2: Use libblkid for checking contents
  2013-05-14 10:45 ` [Cluster-devel] [PATCH 4/6] mkfs.gfs2: Use libblkid for checking contents Andrew Price
@ 2013-05-14 10:52   ` Steven Whitehouse
  2013-05-14 11:00     ` Andrew Price
  0 siblings, 1 reply; 15+ messages in thread
From: Steven Whitehouse @ 2013-05-14 10:52 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Hi,

Looks good. Do we need to change the package spec file now we no longer
need file?

Steve.

On Tue, 2013-05-14 at 11:45 +0100, Andrew Price wrote:
> Use libblkid to probe the contents of the target device instead of
> execing 'file'.
> 
> Signed-off-by: Andrew Price <anprice@redhat.com>
> ---
>  gfs2/mkfs/main_mkfs.c | 181 ++++++++++++++------------------------------------
>  1 file changed, 51 insertions(+), 130 deletions(-)
> 
> diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
> index 9949a46..91c89d3 100644
> --- a/gfs2/mkfs/main_mkfs.c
> +++ b/gfs2/mkfs/main_mkfs.c
> @@ -20,6 +20,7 @@
>  #include <libintl.h>
>  #include <sys/ioctl.h>
>  #include <limits.h>
> +#include <blkid.h>
>  
>  #define _(String) gettext(String)
>  
> @@ -123,6 +124,7 @@ static void opts_init(struct mkfs_opts *opts)
>  	opts->rgsize = GFS2_DEFAULT_RGSIZE;
>  	opts->lockproto = "lock_dlm";
>  	opts->locktable = "";
> +	opts->confirm = 1;
>  }
>  
>  #ifndef BLKDISCARD
> @@ -475,105 +477,6 @@ static void opts_check(struct mkfs_opts *opts)
>  
>  }
>  
> -static int get_file_output(int fd, char *buffer, size_t buflen)
> -{
> -	struct pollfd pf = { .fd = fd, .events = POLLIN|POLLRDHUP };
> -	int flags;
> -	int pos = 0;
> -	int rv;
> -
> -	flags = fcntl(fd, F_GETFL, 0);
> -	if (flags < 0)
> -		return flags;
> -
> -	flags |= O_NONBLOCK;
> -	rv = fcntl(fd, F_SETFL, flags);
> -	if (rv < 0)
> -		return rv;
> -
> -	while (1) {
> -		rv = poll(&pf, 1, 10 * 1000);
> -		if (rv == 0)
> -			break;
> -		if (rv < 0)
> -			return rv;
> -		if (pf.revents & POLLIN) {
> -			rv = read(fd, buffer + pos,
> -				   buflen - pos);
> -			if (rv < 0) {
> -				if (errno == EAGAIN)
> -					continue;
> -				return rv;
> -			}
> -			if (rv == 0)
> -				break;
> -			pos += rv;
> -			if (pos >= buflen)
> -				return -1;
> -			buffer[pos] = 0;
> -			continue;
> -		}
> -		if (pf.revents & (POLLRDHUP | POLLHUP | POLLERR))
> -			break;
> -	}
> -	return 0;
> -}
> -
> -static void check_dev_content(const char *devname)
> -{
> -	struct sigaction sa;
> -	char content[1024] = { 0, };
> -	char * args[] = {
> -		(char *)"/usr/bin/file",
> -		(char *)"-bsL",
> -		(char *)devname,
> -		NULL };
> -	int p[2] = {-1, -1};
> -	int ret;
> -	int pid;
> -
> -	ret = sigaction(SIGCHLD, NULL, &sa);
> -	if  (ret)
> -		return;
> -	sa.sa_handler = SIG_IGN;
> -	sa.sa_flags |= (SA_NOCLDSTOP | SA_NOCLDWAIT);
> -	ret = sigaction(SIGCHLD, &sa, NULL);
> -	if (ret)
> -		goto fail;
> -
> -	ret = pipe(p);
> -	if (ret)
> -		goto fail;
> -
> -	pid = fork();
> -
> -	if (pid < 0) {
> -		close(p[1]);
> -		goto fail;
> -	}
> -
> -	if (pid) {
> -		close(p[1]);
> -		ret = get_file_output(p[0], content, sizeof(content));
> -		if (ret) {
> -fail:
> -			printf( _("Content of file or device unknown (do you have GNU fileutils installed?)\n"));
> -		} else {
> -			if (*content == 0)
> -				goto fail;
> -			printf( _("It appears to contain: %s"), content);
> -		}
> -		if (p[0] >= 0)
> -			close(p[0]);
> -		return;
> -	}
> -
> -	close(p[0]);
> -	dup2(p[1], STDOUT_FILENO);
> -	close(STDIN_FILENO);
> -	exit(execv(args[0], args));
> -}
> -
>  static void print_results(struct gfs2_sbd *sdp, uint64_t real_device_size,
>                            struct mkfs_opts *opts, unsigned char uuid[16])
>  {
> @@ -594,30 +497,27 @@ static void print_results(struct gfs2_sbd *sdp, uint64_t real_device_size,
>  	printf("%-27s%s\n", _("UUID:"), str_uuid(uuid));
>  }
>  
> -/**
> - * If path is a symlink, return 1 with *abspath pointing to the absolute path,
> - * otherwise return 0. Exit on errors. The caller must free the memory pointed
> - * to by *abspath.
> - */
> -static int is_symlink(const char *path, char **abspath)
> +static void warn_of_destruction(const char *path)
>  {
>  	struct stat lnkstat;
> +	char *abspath = NULL;
>  
>  	if (lstat(path, &lnkstat) == -1) {
>  		perror(_("Failed to lstat the device"));
>  		exit(EXIT_FAILURE);
>  	}
> -	if (!S_ISLNK(lnkstat.st_mode)) {
> -		return 0;
> -	}
> -	*abspath = canonicalize_file_name(path);
> -	if (*abspath == NULL) {
> -		perror(_("Could not find the absolute path of the device"));
> -		exit(EXIT_FAILURE);
> +	if (S_ISLNK(lnkstat.st_mode)) {
> +		abspath = canonicalize_file_name(path);
> +		if (abspath == NULL) {
> +			perror(_("Could not find the absolute path of the device"));
> +			exit(EXIT_FAILURE);
> +		}
> +		/* Translators: Example: "/dev/vg/lv is a symbolic link to /dev/dm-2" */
> +		printf( _("%s is a symbolic link to %s\n"), path, abspath);
> +		path = abspath;
>  	}
> -	/* Translators: Example: "/dev/vg/lv is a symbolic link to /dev/dm-2" */
> -	printf( _("%s is a symbolic link to %s\n"), path, *abspath);
> -	return 1;
> +	printf(_("This will destroy any data on %s\n"), path);
> +	free(abspath);
>  }
>  
>  static int writerg(int fd, const struct rgrp_tree *rgt, const unsigned bsize)
> @@ -751,6 +651,38 @@ static void sbd_init(struct gfs2_sbd *sdp, struct mkfs_opts *opts, struct lgfs2_
>  	}
>  }
>  
> +static int probe_contents(int fd)
> +{
> +	int ret;
> +	const char *contents;
> +	blkid_probe pr = blkid_new_probe();
> +	if (pr == NULL || blkid_probe_set_device(pr, fd, 0, 0) != 0
> +	               || blkid_probe_enable_superblocks(pr, TRUE) != 0
> +	               || blkid_probe_enable_partitions(pr, TRUE) != 0) {
> +		fprintf(stderr, _("Failed to create probe\n"));
> +		return -1;
> +	}
> +
> +	ret = blkid_do_fullprobe(pr);
> +	if (ret == -1) {
> +		fprintf(stderr, _("Failed to probe device\n"));
> +		return -1;
> +	}
> +
> +	if (ret == 1)
> +		return 0;
> +
> +	if (!blkid_probe_lookup_value(pr, "TYPE", &contents, NULL)) {
> +		printf(_("It appears to contain an existing filesystem (%s)\n"), contents);
> +	} else if (!blkid_probe_lookup_value(pr, "PTTYPE", &contents, NULL)) {
> +		printf(_("It appears to contain a partition table (%s).\n"), contents);
> +	}
> +
> +	blkid_free_probe(pr);
> +
> +	return 0;
> +}
> +
>  void main_mkfs(int argc, char *argv[])
>  {
>  	struct gfs2_sbd sbd;
> @@ -758,9 +690,6 @@ void main_mkfs(int argc, char *argv[])
>  	struct lgfs2_dev_info dinfo;
>  	int error;
>  	unsigned char uuid[16];
> -	char *absname = NULL;
> -	char *fdpath = NULL;
> -	int islnk = 0;
>  	int fd;
>  
>  	opts_init(&opts);
> @@ -783,21 +712,13 @@ void main_mkfs(int argc, char *argv[])
>  	}
>  
>  	opts_check(&opts);
> -	sbd_init(&sbd, &opts, &dinfo, fd);
> +	warn_of_destruction(opts.device);
>  
> -	if (asprintf(&fdpath, "/proc/%d/fd/%d", getpid(), fd) < 0) {
> -		perror(_("Failed to build string"));
> +	error = probe_contents(fd);
> +	if (error)
>  		exit(EXIT_FAILURE);
> -	}
>  
> -	if (!opts.override) {
> -		islnk = is_symlink(opts.device, &absname);
> -		printf(_("This will destroy any data on %s.\n"), islnk ? absname : opts.device);
> -		free(absname);
> -		check_dev_content(fdpath);
> -		opts.confirm = 1;
> -	}
> -	free(fdpath);
> +	sbd_init(&sbd, &opts, &dinfo, fd);
>  
>  	if (opts.confirm && !opts.override)
>  		are_you_sure();




^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Cluster-devel] [PATCH 4/6] mkfs.gfs2: Use libblkid for checking contents
  2013-05-14 10:52   ` Steven Whitehouse
@ 2013-05-14 11:00     ` Andrew Price
  0 siblings, 0 replies; 15+ messages in thread
From: Andrew Price @ 2013-05-14 11:00 UTC (permalink / raw)
  To: cluster-devel.redhat.com

On 14/05/13 11:52, Steven Whitehouse wrote:
> Hi,
>
> Looks good. Do we need to change the package spec file now we no longer
> need file?

Yes, I've made a note to do that when we do the next release along with 
updating the build docs.

Andy

>
> Steve.
>
> On Tue, 2013-05-14 at 11:45 +0100, Andrew Price wrote:
>> Use libblkid to probe the contents of the target device instead of
>> execing 'file'.
>>
>> Signed-off-by: Andrew Price <anprice@redhat.com>
>> ---
>>   gfs2/mkfs/main_mkfs.c | 181 ++++++++++++++------------------------------------
>>   1 file changed, 51 insertions(+), 130 deletions(-)
>>
>> diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
>> index 9949a46..91c89d3 100644
>> --- a/gfs2/mkfs/main_mkfs.c
>> +++ b/gfs2/mkfs/main_mkfs.c
>> @@ -20,6 +20,7 @@
>>   #include <libintl.h>
>>   #include <sys/ioctl.h>
>>   #include <limits.h>
>> +#include <blkid.h>
>>
>>   #define _(String) gettext(String)
>>
>> @@ -123,6 +124,7 @@ static void opts_init(struct mkfs_opts *opts)
>>   	opts->rgsize = GFS2_DEFAULT_RGSIZE;
>>   	opts->lockproto = "lock_dlm";
>>   	opts->locktable = "";
>> +	opts->confirm = 1;
>>   }
>>
>>   #ifndef BLKDISCARD
>> @@ -475,105 +477,6 @@ static void opts_check(struct mkfs_opts *opts)
>>
>>   }
>>
>> -static int get_file_output(int fd, char *buffer, size_t buflen)
>> -{
>> -	struct pollfd pf = { .fd = fd, .events = POLLIN|POLLRDHUP };
>> -	int flags;
>> -	int pos = 0;
>> -	int rv;
>> -
>> -	flags = fcntl(fd, F_GETFL, 0);
>> -	if (flags < 0)
>> -		return flags;
>> -
>> -	flags |= O_NONBLOCK;
>> -	rv = fcntl(fd, F_SETFL, flags);
>> -	if (rv < 0)
>> -		return rv;
>> -
>> -	while (1) {
>> -		rv = poll(&pf, 1, 10 * 1000);
>> -		if (rv == 0)
>> -			break;
>> -		if (rv < 0)
>> -			return rv;
>> -		if (pf.revents & POLLIN) {
>> -			rv = read(fd, buffer + pos,
>> -				   buflen - pos);
>> -			if (rv < 0) {
>> -				if (errno == EAGAIN)
>> -					continue;
>> -				return rv;
>> -			}
>> -			if (rv == 0)
>> -				break;
>> -			pos += rv;
>> -			if (pos >= buflen)
>> -				return -1;
>> -			buffer[pos] = 0;
>> -			continue;
>> -		}
>> -		if (pf.revents & (POLLRDHUP | POLLHUP | POLLERR))
>> -			break;
>> -	}
>> -	return 0;
>> -}
>> -
>> -static void check_dev_content(const char *devname)
>> -{
>> -	struct sigaction sa;
>> -	char content[1024] = { 0, };
>> -	char * args[] = {
>> -		(char *)"/usr/bin/file",
>> -		(char *)"-bsL",
>> -		(char *)devname,
>> -		NULL };
>> -	int p[2] = {-1, -1};
>> -	int ret;
>> -	int pid;
>> -
>> -	ret = sigaction(SIGCHLD, NULL, &sa);
>> -	if  (ret)
>> -		return;
>> -	sa.sa_handler = SIG_IGN;
>> -	sa.sa_flags |= (SA_NOCLDSTOP | SA_NOCLDWAIT);
>> -	ret = sigaction(SIGCHLD, &sa, NULL);
>> -	if (ret)
>> -		goto fail;
>> -
>> -	ret = pipe(p);
>> -	if (ret)
>> -		goto fail;
>> -
>> -	pid = fork();
>> -
>> -	if (pid < 0) {
>> -		close(p[1]);
>> -		goto fail;
>> -	}
>> -
>> -	if (pid) {
>> -		close(p[1]);
>> -		ret = get_file_output(p[0], content, sizeof(content));
>> -		if (ret) {
>> -fail:
>> -			printf( _("Content of file or device unknown (do you have GNU fileutils installed?)\n"));
>> -		} else {
>> -			if (*content == 0)
>> -				goto fail;
>> -			printf( _("It appears to contain: %s"), content);
>> -		}
>> -		if (p[0] >= 0)
>> -			close(p[0]);
>> -		return;
>> -	}
>> -
>> -	close(p[0]);
>> -	dup2(p[1], STDOUT_FILENO);
>> -	close(STDIN_FILENO);
>> -	exit(execv(args[0], args));
>> -}
>> -
>>   static void print_results(struct gfs2_sbd *sdp, uint64_t real_device_size,
>>                             struct mkfs_opts *opts, unsigned char uuid[16])
>>   {
>> @@ -594,30 +497,27 @@ static void print_results(struct gfs2_sbd *sdp, uint64_t real_device_size,
>>   	printf("%-27s%s\n", _("UUID:"), str_uuid(uuid));
>>   }
>>
>> -/**
>> - * If path is a symlink, return 1 with *abspath pointing to the absolute path,
>> - * otherwise return 0. Exit on errors. The caller must free the memory pointed
>> - * to by *abspath.
>> - */
>> -static int is_symlink(const char *path, char **abspath)
>> +static void warn_of_destruction(const char *path)
>>   {
>>   	struct stat lnkstat;
>> +	char *abspath = NULL;
>>
>>   	if (lstat(path, &lnkstat) == -1) {
>>   		perror(_("Failed to lstat the device"));
>>   		exit(EXIT_FAILURE);
>>   	}
>> -	if (!S_ISLNK(lnkstat.st_mode)) {
>> -		return 0;
>> -	}
>> -	*abspath = canonicalize_file_name(path);
>> -	if (*abspath == NULL) {
>> -		perror(_("Could not find the absolute path of the device"));
>> -		exit(EXIT_FAILURE);
>> +	if (S_ISLNK(lnkstat.st_mode)) {
>> +		abspath = canonicalize_file_name(path);
>> +		if (abspath == NULL) {
>> +			perror(_("Could not find the absolute path of the device"));
>> +			exit(EXIT_FAILURE);
>> +		}
>> +		/* Translators: Example: "/dev/vg/lv is a symbolic link to /dev/dm-2" */
>> +		printf( _("%s is a symbolic link to %s\n"), path, abspath);
>> +		path = abspath;
>>   	}
>> -	/* Translators: Example: "/dev/vg/lv is a symbolic link to /dev/dm-2" */
>> -	printf( _("%s is a symbolic link to %s\n"), path, *abspath);
>> -	return 1;
>> +	printf(_("This will destroy any data on %s\n"), path);
>> +	free(abspath);
>>   }
>>
>>   static int writerg(int fd, const struct rgrp_tree *rgt, const unsigned bsize)
>> @@ -751,6 +651,38 @@ static void sbd_init(struct gfs2_sbd *sdp, struct mkfs_opts *opts, struct lgfs2_
>>   	}
>>   }
>>
>> +static int probe_contents(int fd)
>> +{
>> +	int ret;
>> +	const char *contents;
>> +	blkid_probe pr = blkid_new_probe();
>> +	if (pr == NULL || blkid_probe_set_device(pr, fd, 0, 0) != 0
>> +	               || blkid_probe_enable_superblocks(pr, TRUE) != 0
>> +	               || blkid_probe_enable_partitions(pr, TRUE) != 0) {
>> +		fprintf(stderr, _("Failed to create probe\n"));
>> +		return -1;
>> +	}
>> +
>> +	ret = blkid_do_fullprobe(pr);
>> +	if (ret == -1) {
>> +		fprintf(stderr, _("Failed to probe device\n"));
>> +		return -1;
>> +	}
>> +
>> +	if (ret == 1)
>> +		return 0;
>> +
>> +	if (!blkid_probe_lookup_value(pr, "TYPE", &contents, NULL)) {
>> +		printf(_("It appears to contain an existing filesystem (%s)\n"), contents);
>> +	} else if (!blkid_probe_lookup_value(pr, "PTTYPE", &contents, NULL)) {
>> +		printf(_("It appears to contain a partition table (%s).\n"), contents);
>> +	}
>> +
>> +	blkid_free_probe(pr);
>> +
>> +	return 0;
>> +}
>> +
>>   void main_mkfs(int argc, char *argv[])
>>   {
>>   	struct gfs2_sbd sbd;
>> @@ -758,9 +690,6 @@ void main_mkfs(int argc, char *argv[])
>>   	struct lgfs2_dev_info dinfo;
>>   	int error;
>>   	unsigned char uuid[16];
>> -	char *absname = NULL;
>> -	char *fdpath = NULL;
>> -	int islnk = 0;
>>   	int fd;
>>
>>   	opts_init(&opts);
>> @@ -783,21 +712,13 @@ void main_mkfs(int argc, char *argv[])
>>   	}
>>
>>   	opts_check(&opts);
>> -	sbd_init(&sbd, &opts, &dinfo, fd);
>> +	warn_of_destruction(opts.device);
>>
>> -	if (asprintf(&fdpath, "/proc/%d/fd/%d", getpid(), fd) < 0) {
>> -		perror(_("Failed to build string"));
>> +	error = probe_contents(fd);
>> +	if (error)
>>   		exit(EXIT_FAILURE);
>> -	}
>>
>> -	if (!opts.override) {
>> -		islnk = is_symlink(opts.device, &absname);
>> -		printf(_("This will destroy any data on %s.\n"), islnk ? absname : opts.device);
>> -		free(absname);
>> -		check_dev_content(fdpath);
>> -		opts.confirm = 1;
>> -	}
>> -	free(fdpath);
>> +	sbd_init(&sbd, &opts, &dinfo, fd);
>>
>>   	if (opts.confirm && !opts.override)
>>   		are_you_sure();
>
>



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Cluster-devel] [PATCH 5/6] mkfs.gfs2: Add a struct to store device info
  2013-05-14 10:45 [Cluster-devel] [PATCH 0/6] More mkfs.gfs2 reworking Andrew Price
                   ` (3 preceding siblings ...)
  2013-05-14 10:45 ` [Cluster-devel] [PATCH 4/6] mkfs.gfs2: Use libblkid for checking contents Andrew Price
@ 2013-05-14 10:45 ` Andrew Price
  2013-05-14 10:45 ` [Cluster-devel] [PATCH 6/6] libgfs2: Clarify gfs2_compute_bitstructs's parameters Andrew Price
  2013-05-14 12:21 ` [Cluster-devel] [PATCH 0/6] More mkfs.gfs2 reworking Bob Peterson
  6 siblings, 0 replies; 15+ messages in thread
From: Andrew Price @ 2013-05-14 10:45 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Adds a struct mkfs_dev to store the fd, topology and stat info for the
target. Not using lgfs2_device_info as we now probe the topology with
libblkid so that functionality should eventually disappear from libgfs2.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/mkfs/main_mkfs.c | 173 ++++++++++++++++++++++++++++++++++----------------
 1 file changed, 118 insertions(+), 55 deletions(-)

diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
index 91c89d3..aadd501 100644
--- a/gfs2/mkfs/main_mkfs.c
+++ b/gfs2/mkfs/main_mkfs.c
@@ -113,6 +113,27 @@ struct mkfs_opts {
 	unsigned confirm:1;
 };
 
+/**
+ * Values probed by libblkid:
+ *  alignment_offset: offset, in bytes, of the start of the dev from its natural alignment
+ *  logical_sector_size: smallest addressable unit
+ *  minimum_io_size: device's preferred unit of I/O. RAID stripe unit.
+ *  optimal_io_size: biggest I/O we can submit without incurring a penalty. RAID stripe width.
+ *  physical_sector_size: the smallest unit we can write atomically
+ */
+struct mkfs_dev {
+	int fd;
+	struct stat stat;
+	uint64_t size;
+	unsigned long alignment_offset;
+	unsigned long logical_sector_size;
+	unsigned long minimum_io_size;
+	unsigned long optimal_io_size;
+	unsigned long physical_sector_size;
+
+	unsigned int got_topol:1;
+};
+
 static void opts_init(struct mkfs_opts *opts)
 {
 	memset(opts, 0, sizeof(*opts));
@@ -408,25 +429,26 @@ static void are_you_sure(void)
 		free(line);
 }
 
-static unsigned choose_blocksize(struct mkfs_opts *opts, struct lgfs2_dev_info *dinfo)
+static unsigned choose_blocksize(struct mkfs_opts *opts, const struct mkfs_dev *dev)
 {
 	unsigned int x;
 	unsigned int bsize = opts->bsize;
 
-	if (!opts->got_bsize) {
-		if (S_ISREG(dinfo->stat.st_mode))
-			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. */
-		else if (dinfo->io_optimal_size <= getpagesize() && dinfo->io_optimal_size >= dinfo->io_min_size)
-			bsize = dinfo->io_optimal_size;
-		/* See if physical_block_size (the smallest unit we can write
-		   without incurring read-modify-write penalty) is suitable. */
-		else if (dinfo->physical_block_size <= getpagesize() && dinfo->physical_block_size >= GFS2_DEFAULT_BSIZE)
-			bsize = dinfo->physical_block_size;
-		else
-			bsize = GFS2_DEFAULT_BSIZE;
+	if (dev->got_topol && opts->debug) {
+		printf("alignment_offset: %lu\n", dev->alignment_offset);
+		printf("logical_sector_size: %lu\n", dev->logical_sector_size);
+		printf("minimum_io_size: %lu\n", dev->minimum_io_size);
+		printf("optimal_io_size: %lu\n", dev->optimal_io_size);
+		printf("physical_sector_size: %lu\n", dev->physical_sector_size);
+	}
 
+	if (!opts->got_bsize && dev->got_topol) {
+		if (dev->optimal_io_size <= getpagesize() &&
+		    dev->optimal_io_size >= dev->minimum_io_size)
+			bsize = dev->optimal_io_size;
+		else if (dev->physical_sector_size <= getpagesize() &&
+		         dev->physical_sector_size >= GFS2_DEFAULT_BSIZE)
+			bsize = dev->physical_sector_size;
 	}
 
 	/* Block sizes must be a power of two from 512 to 65536 */
@@ -438,15 +460,15 @@ static unsigned choose_blocksize(struct mkfs_opts *opts, struct lgfs2_dev_info *
 		die( _("Block size must be a power of two between 512 and %d\n"),
 		       getpagesize());
 
-	if (bsize < dinfo->logical_block_size) {
+	if (bsize < dev->logical_sector_size) {
 		die( _("Error: Block size %d is less than minimum logical "
-		       "block size (%d).\n"), bsize, dinfo->logical_block_size);
+		       "block size (%lu).\n"), bsize, dev->logical_sector_size);
 	}
 
-	if (bsize < dinfo->physical_block_size) {
+	if (bsize < dev->physical_sector_size) {
 		printf( _("Warning: Block size %d is inefficient because it "
-			  "is less than the physical block size (%d).\n"),
-			  bsize, dinfo->physical_block_size);
+			  "is less than the physical block size (%lu).\n"),
+			  bsize, dev->physical_sector_size);
 		opts->confirm = 1;
 	}
 	return bsize;
@@ -454,6 +476,11 @@ static unsigned choose_blocksize(struct mkfs_opts *opts, struct lgfs2_dev_info *
 
 static void opts_check(struct mkfs_opts *opts)
 {
+	if (!opts->got_device) {
+		fprintf(stderr, _("No device specified. Use -h for help\n"));
+		exit(1);
+	}
+
 	if (!opts->expert)
 		test_locking(opts->lockproto, opts->locktable);
 	if (opts->expert) {
@@ -607,7 +634,7 @@ static int place_rgrps(struct gfs2_sbd *sdp, const struct mkfs_opts *opts)
 	return 0;
 }
 
-static void sbd_init(struct gfs2_sbd *sdp, struct mkfs_opts *opts, struct lgfs2_dev_info *dinfo, int fd)
+static void sbd_init(struct gfs2_sbd *sdp, struct mkfs_opts *opts, struct mkfs_dev *dev)
 {
 	memset(sdp, 0, sizeof(struct gfs2_sbd));
 	sdp->time = time(NULL);
@@ -616,20 +643,20 @@ static void sbd_init(struct gfs2_sbd *sdp, struct mkfs_opts *opts, struct lgfs2_
 	sdp->qcsize = opts->qcsize;
 	sdp->jsize = opts->jsize;
 	sdp->md.journals = opts->journals;
-	sdp->device_fd = fd;
-	sdp->bsize = choose_blocksize(opts, dinfo);
+	sdp->device_fd = dev->fd;
+	sdp->bsize = choose_blocksize(opts, dev);
 
 	if (compute_constants(sdp)) {
 		perror(_("Failed to compute file system constants"));
 		exit(1);
 	}
-	sdp->device.length = dinfo->size / sdp->bsize;
+	sdp->device.length = dev->size / sdp->bsize;
 	if (opts->got_fssize) {
 		if (opts->fssize > sdp->device.length) {
 			fprintf(stderr, _("Specified size is bigger than the device."));
-			die("%s %.2f %s (%llu %s)\n", _("Device size:"),
-			       dinfo->size / ((float)(1 << 30)), _("GB"),
-			       (unsigned long long)dinfo->size / sdp->bsize, _("blocks"));
+			die("%s %.2f %s (%"PRIu64" %s)\n", _("Device size:"),
+			       dev->size / ((float)(1 << 30)), _("GB"),
+			       dev->size / sdp->bsize, _("blocks"));
 		}
 		/* TODO: Check if the fssize is too small, somehow */
 		sdp->device.length = opts->fssize;
@@ -651,18 +678,23 @@ static void sbd_init(struct gfs2_sbd *sdp, struct mkfs_opts *opts, struct lgfs2_
 	}
 }
 
-static int probe_contents(int fd)
+static int probe_contents(struct mkfs_dev *dev)
 {
 	int ret;
 	const char *contents;
 	blkid_probe pr = blkid_new_probe();
-	if (pr == NULL || blkid_probe_set_device(pr, fd, 0, 0) != 0
+	if (pr == NULL || blkid_probe_set_device(pr, dev->fd, 0, 0) != 0
 	               || blkid_probe_enable_superblocks(pr, TRUE) != 0
 	               || blkid_probe_enable_partitions(pr, TRUE) != 0) {
 		fprintf(stderr, _("Failed to create probe\n"));
 		return -1;
 	}
 
+	if (!S_ISREG(dev->stat.st_mode) && blkid_probe_enable_topology(pr, TRUE) != 0) {
+		fprintf(stderr, _("Failed to create probe\n"));
+		return -1;
+	}
+
 	ret = blkid_do_fullprobe(pr);
 	if (ret == -1) {
 		fprintf(stderr, _("Failed to probe device\n"));
@@ -678,53 +710,84 @@ static int probe_contents(int fd)
 		printf(_("It appears to contain a partition table (%s).\n"), contents);
 	}
 
-	blkid_free_probe(pr);
+	if (!S_ISREG(dev->stat.st_mode)) {
+		blkid_topology tp = blkid_probe_get_topology(pr);
+		if (tp != NULL) {
+			dev->alignment_offset = blkid_topology_get_alignment_offset(tp);
+			dev->logical_sector_size = blkid_topology_get_logical_sector_size(tp);
+			dev->minimum_io_size = blkid_topology_get_minimum_io_size(tp);
+			dev->optimal_io_size = blkid_topology_get_optimal_io_size(tp);
+			dev->physical_sector_size = blkid_topology_get_physical_sector_size(tp);
+			dev->got_topol = 1;
+		}
+	}
 
+	blkid_free_probe(pr);
 	return 0;
 }
 
+static void open_dev(const char *path, struct mkfs_dev *dev)
+{
+	int error;
+
+	memset(dev, 0, sizeof(*dev));
+	dev->fd = open(path, O_RDWR | O_CLOEXEC);
+	if (dev->fd < 0) {
+		perror(path);
+		exit(1);
+	}
+
+	error = fstat(dev->fd, &dev->stat);
+	if (error < 0) {
+		perror(path);
+		exit(1);
+	}
+
+	if (S_ISREG(dev->stat.st_mode)) {
+		dev->size = dev->stat.st_size;
+	} else if (S_ISBLK(dev->stat.st_mode)) {
+		dev->size = lseek(dev->fd, 0, SEEK_END);
+		if (dev->size < 1) {
+			fprintf(stderr, _("Device '%s' is too small\n"), path);
+			exit(1);
+		}
+	} else {
+		fprintf(stderr, _("'%s' is not a block device or regular file\n"), path);
+		exit(1);
+	}
+
+	error = probe_contents(dev);
+	if (error)
+		exit(1);
+}
+
 void main_mkfs(int argc, char *argv[])
 {
 	struct gfs2_sbd sbd;
 	struct mkfs_opts opts;
-	struct lgfs2_dev_info dinfo;
+	struct mkfs_dev dev;
 	int error;
 	unsigned char uuid[16];
-	int fd;
 
 	opts_init(&opts);
 	opts_get(argc, argv, &opts);
+	opts_check(&opts);
 
-	if (!opts.got_device) {
-		fprintf(stderr, _("No device specified. Use -h for help\n"));
-		exit(1);
-	}
-
-	fd = open(opts.device, O_RDWR | O_CLOEXEC);
-	if (fd < 0){
-		perror(opts.device);
-		exit(EXIT_FAILURE);
-	}
+	open_dev(opts.device, &dev);
 
-	if (lgfs2_get_dev_info(fd, &dinfo) < 0) {
-		perror(opts.device);
-		exit(EXIT_FAILURE);
+	if (S_ISREG(dev.stat.st_mode)) {
+		opts.got_bsize = 1; /* Use default block size for regular files */
 	}
 
-	opts_check(&opts);
 	warn_of_destruction(opts.device);
 
-	error = probe_contents(fd);
-	if (error)
-		exit(EXIT_FAILURE);
-
-	sbd_init(&sbd, &opts, &dinfo, fd);
+	sbd_init(&sbd, &opts, &dev);
 
 	if (opts.confirm && !opts.override)
 		are_you_sure();
 
-	if (!S_ISREG(dinfo.stat.st_mode) && opts.discard)
-		discard_blocks(fd, sbd.bsize * sbd.device.length, opts.debug);
+	if (!S_ISREG(dev.stat.st_mode) && opts.discard)
+		discard_blocks(dev.fd, sbd.bsize * sbd.device.length, opts.debug);
 
 	error = place_rgrps(&sbd, &opts);
 	if (error) {
@@ -777,18 +840,18 @@ void main_mkfs(int argc, char *argv[])
 	inode_put(&sbd.md.statfs);
 
 	gfs2_rgrp_free(&sbd.rgtree);
-	error = fsync(fd);
+	error = fsync(dev.fd);
 	if (error){
 		perror(opts.device);
 		exit(EXIT_FAILURE);
 	}
 
-	error = close(fd);
+	error = close(dev.fd);
 	if (error){
 		perror(opts.device);
 		exit(EXIT_FAILURE);
 	}
 
 	if (!opts.quiet)
-		print_results(&sbd, dinfo.size, &opts, uuid);
+		print_results(&sbd, dev.size, &opts, uuid);
 }
-- 
1.8.1.4



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Cluster-devel] [PATCH 6/6] libgfs2: Clarify gfs2_compute_bitstructs's parameters
  2013-05-14 10:45 [Cluster-devel] [PATCH 0/6] More mkfs.gfs2 reworking Andrew Price
                   ` (4 preceding siblings ...)
  2013-05-14 10:45 ` [Cluster-devel] [PATCH 5/6] mkfs.gfs2: Add a struct to store device info Andrew Price
@ 2013-05-14 10:45 ` Andrew Price
  2013-05-14 10:54   ` Steven Whitehouse
  2013-05-14 12:21 ` [Cluster-devel] [PATCH 0/6] More mkfs.gfs2 reworking Bob Peterson
  6 siblings, 1 reply; 15+ messages in thread
From: Andrew Price @ 2013-05-14 10:45 UTC (permalink / raw)
  To: cluster-devel.redhat.com

gfs2_compute_bitstructs accepted an sdp and only used it to look up the
block size. Replace the sdp parameter with a bsize parameter to make it
easier to reason through code which uses this function.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/convert/gfs2_convert.c |  2 +-
 gfs2/fsck/rgrepair.c        |  6 +++---
 gfs2/libgfs2/fs_geometry.c  |  2 +-
 gfs2/libgfs2/libgfs2.h      |  2 +-
 gfs2/libgfs2/rgrp.c         | 10 +++++-----
 gfs2/libgfs2/super.c        |  2 +-
 gfs2/mkfs/main_mkfs.c       |  2 +-
 7 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/gfs2/convert/gfs2_convert.c b/gfs2/convert/gfs2_convert.c
index d286512..d4bad1d 100644
--- a/gfs2/convert/gfs2_convert.c
+++ b/gfs2/convert/gfs2_convert.c
@@ -1898,7 +1898,7 @@ static int journ_space_to_rg(struct gfs2_sbd *sdp)
 			rgd->bh[x] = bget(sdp, rgd->ri.ri_addr + x);
 			memset(rgd->bh[x]->b_data, 0, sdp->bsize);
 		}
-		if (gfs2_compute_bitstructs(sdp, rgd)) {
+		if (gfs2_compute_bitstructs(sdp->sd_sb.sb_bsize, rgd)) {
 			log_crit(_("gfs2_convert: Error converting bitmaps.\n"));
 			exit(-1);
 		}
diff --git a/gfs2/fsck/rgrepair.c b/gfs2/fsck/rgrepair.c
index 27368a2..1ebdc70 100644
--- a/gfs2/fsck/rgrepair.c
+++ b/gfs2/fsck/rgrepair.c
@@ -503,7 +503,7 @@ static int gfs2_rindex_rebuild(struct gfs2_sbd *sdp, int *num_rgs,
 				break; /* end of bitmap, so call it quits. */
 		} /* for subsequent bitmaps */
 		
-		gfs2_compute_bitstructs(sdp, calc_rgd);
+		gfs2_compute_bitstructs(sdp->sd_sb.sb_bsize, calc_rgd);
 		calc_rgd->ri.ri_data0 = calc_rgd->ri.ri_addr +
 			calc_rgd->ri.ri_length;
 		if (prev_rgd) {
@@ -732,7 +732,7 @@ static int expect_rindex_sanity(struct gfs2_sbd *sdp, int *num_rgs)
 		memcpy(&exp->rg, &rgd->rg, sizeof(exp->rg));
 		exp->bits = NULL;
 		exp->bh = NULL;
-		gfs2_compute_bitstructs(sdp, exp);
+		gfs2_compute_bitstructs(sdp->sd_sb.sb_bsize, exp);
 	}
 	sdp->rgrps = *num_rgs;
 	return 0;
@@ -948,7 +948,7 @@ int rg_repair(struct gfs2_sbd *sdp, int trust_lvl, int *rg_count, int *sane)
 			}
 			else
 				log_err( _("rindex not fixed.\n"));
-			gfs2_compute_bitstructs(sdp, actual);
+			gfs2_compute_bitstructs(sdp->sd_sb.sb_bsize, actual);
 			rindex_modified = FALSE;
 		}
 		e = enext;
diff --git a/gfs2/libgfs2/fs_geometry.c b/gfs2/libgfs2/fs_geometry.c
index e716127..932a2e6 100644
--- a/gfs2/libgfs2/fs_geometry.c
+++ b/gfs2/libgfs2/fs_geometry.c
@@ -207,7 +207,7 @@ void build_rgrps(struct gfs2_sbd *sdp, int do_write)
 		rl->rg.rg_header.mh_format = GFS2_FORMAT_RG;
 		rl->rg.rg_free = rgblocks;
 
-		if (gfs2_compute_bitstructs(sdp, rl)) {
+		if (gfs2_compute_bitstructs(sdp->sd_sb.sb_bsize, rl)) {
 			fprintf(stderr, "%s: Unable to build resource groups "
 				"with these characteristics.\n", __FUNCTION__);
 			exit(-1);
diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index e069e9d..997e23f 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -725,7 +725,7 @@ extern int gfs2_find_jhead(struct gfs2_inode *ip, struct gfs2_log_header *head);
 extern int clean_journal(struct gfs2_inode *ip, struct gfs2_log_header *head);
 
 /* rgrp.c */
-extern int gfs2_compute_bitstructs(struct gfs2_sbd *sdp, struct rgrp_tree *rgd);
+extern int gfs2_compute_bitstructs(const uint32_t bsize, struct rgrp_tree *rgd);
 extern struct rgrp_tree *gfs2_blk2rgrpd(struct gfs2_sbd *sdp, uint64_t blk);
 extern uint64_t gfs2_rgrp_read(struct gfs2_sbd *sdp, struct rgrp_tree *rgd);
 extern void gfs2_rgrp_relse(struct rgrp_tree *rgd);
diff --git a/gfs2/libgfs2/rgrp.c b/gfs2/libgfs2/rgrp.c
index f7dc01e..f2b8304 100644
--- a/gfs2/libgfs2/rgrp.c
+++ b/gfs2/libgfs2/rgrp.c
@@ -12,11 +12,11 @@
 
 /**
  * gfs2_compute_bitstructs - Compute the bitmap sizes
- * @rgd: The resource group descriptor
- *
+ * bsize: Block size
+ * rgd: The resource group descriptor
  * Returns: 0 on success, -1 on error
  */
-int gfs2_compute_bitstructs(struct gfs2_sbd *sdp, struct rgrp_tree *rgd)
+int gfs2_compute_bitstructs(const uint32_t bsize, struct rgrp_tree *rgd)
 {
 	struct gfs2_bitmap *bits;
 	uint32_t length = rgd->ri.ri_length;
@@ -49,7 +49,7 @@ int gfs2_compute_bitstructs(struct gfs2_sbd *sdp, struct rgrp_tree *rgd)
 			bits->bi_len = bytes;
 		}
 		else if (x == 0){
-			bytes = sdp->sd_sb.sb_bsize - sizeof(struct gfs2_rgrp);
+			bytes = bsize - sizeof(struct gfs2_rgrp);
 			bits->bi_offset = sizeof(struct gfs2_rgrp);
 			bits->bi_start = 0;
 			bits->bi_len = bytes;
@@ -61,7 +61,7 @@ int gfs2_compute_bitstructs(struct gfs2_sbd *sdp, struct rgrp_tree *rgd)
 			bits->bi_len = bytes;
 		}
 		else{
-			bytes = sdp->sd_sb.sb_bsize - sizeof(struct gfs2_meta_header);
+			bytes = bsize - sizeof(struct gfs2_meta_header);
 			bits->bi_offset = sizeof(struct gfs2_meta_header);
 			bits->bi_start = rgd->ri.ri_bitbytes - bytes_left;
 			bits->bi_len = bytes;
diff --git a/gfs2/libgfs2/super.c b/gfs2/libgfs2/super.c
index 21c9f7b..eb97c40 100644
--- a/gfs2/libgfs2/super.c
+++ b/gfs2/libgfs2/super.c
@@ -188,7 +188,7 @@ int rindex_read(struct gfs2_sbd *sdp, int fd, int *count1, int *sane)
 			prev_rgd->length = rgrp_size(prev_rgd);
 		}
 
-		if(gfs2_compute_bitstructs(sdp, rgd))
+		if(gfs2_compute_bitstructs(sdp->sd_sb.sb_bsize, rgd))
 			*sane = 0;
 
 		(*count1)++;
diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
index aadd501..76b34b8 100644
--- a/gfs2/mkfs/main_mkfs.c
+++ b/gfs2/mkfs/main_mkfs.c
@@ -613,7 +613,7 @@ static int place_rgrps(struct gfs2_sbd *sdp, const struct mkfs_opts *opts)
 
 		/* TODO: This call allocates buffer heads and bitmap pointers
 		 * in rgt. We really shouldn't need to do that. */
-		err = gfs2_compute_bitstructs(sdp, rgt);
+		err = gfs2_compute_bitstructs(sdp->bsize, rgt);
 		if (err != 0) {
 			fprintf(stderr, _("Could not compute bitmaps. "
 			        "Check resource group and block size options.\n"));
-- 
1.8.1.4



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Cluster-devel] [PATCH 6/6] libgfs2: Clarify gfs2_compute_bitstructs's parameters
  2013-05-14 10:45 ` [Cluster-devel] [PATCH 6/6] libgfs2: Clarify gfs2_compute_bitstructs's parameters Andrew Price
@ 2013-05-14 10:54   ` Steven Whitehouse
  0 siblings, 0 replies; 15+ messages in thread
From: Steven Whitehouse @ 2013-05-14 10:54 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Hi,

Other patches all look good too. Thanks,

Steve.

On Tue, 2013-05-14 at 11:45 +0100, Andrew Price wrote:
> gfs2_compute_bitstructs accepted an sdp and only used it to look up the
> block size. Replace the sdp parameter with a bsize parameter to make it
> easier to reason through code which uses this function.
> 
> Signed-off-by: Andrew Price <anprice@redhat.com>
> ---
>  gfs2/convert/gfs2_convert.c |  2 +-
>  gfs2/fsck/rgrepair.c        |  6 +++---
>  gfs2/libgfs2/fs_geometry.c  |  2 +-
>  gfs2/libgfs2/libgfs2.h      |  2 +-
>  gfs2/libgfs2/rgrp.c         | 10 +++++-----
>  gfs2/libgfs2/super.c        |  2 +-
>  gfs2/mkfs/main_mkfs.c       |  2 +-
>  7 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/gfs2/convert/gfs2_convert.c b/gfs2/convert/gfs2_convert.c
> index d286512..d4bad1d 100644
> --- a/gfs2/convert/gfs2_convert.c
> +++ b/gfs2/convert/gfs2_convert.c
> @@ -1898,7 +1898,7 @@ static int journ_space_to_rg(struct gfs2_sbd *sdp)
>  			rgd->bh[x] = bget(sdp, rgd->ri.ri_addr + x);
>  			memset(rgd->bh[x]->b_data, 0, sdp->bsize);
>  		}
> -		if (gfs2_compute_bitstructs(sdp, rgd)) {
> +		if (gfs2_compute_bitstructs(sdp->sd_sb.sb_bsize, rgd)) {
>  			log_crit(_("gfs2_convert: Error converting bitmaps.\n"));
>  			exit(-1);
>  		}
> diff --git a/gfs2/fsck/rgrepair.c b/gfs2/fsck/rgrepair.c
> index 27368a2..1ebdc70 100644
> --- a/gfs2/fsck/rgrepair.c
> +++ b/gfs2/fsck/rgrepair.c
> @@ -503,7 +503,7 @@ static int gfs2_rindex_rebuild(struct gfs2_sbd *sdp, int *num_rgs,
>  				break; /* end of bitmap, so call it quits. */
>  		} /* for subsequent bitmaps */
>  		
> -		gfs2_compute_bitstructs(sdp, calc_rgd);
> +		gfs2_compute_bitstructs(sdp->sd_sb.sb_bsize, calc_rgd);
>  		calc_rgd->ri.ri_data0 = calc_rgd->ri.ri_addr +
>  			calc_rgd->ri.ri_length;
>  		if (prev_rgd) {
> @@ -732,7 +732,7 @@ static int expect_rindex_sanity(struct gfs2_sbd *sdp, int *num_rgs)
>  		memcpy(&exp->rg, &rgd->rg, sizeof(exp->rg));
>  		exp->bits = NULL;
>  		exp->bh = NULL;
> -		gfs2_compute_bitstructs(sdp, exp);
> +		gfs2_compute_bitstructs(sdp->sd_sb.sb_bsize, exp);
>  	}
>  	sdp->rgrps = *num_rgs;
>  	return 0;
> @@ -948,7 +948,7 @@ int rg_repair(struct gfs2_sbd *sdp, int trust_lvl, int *rg_count, int *sane)
>  			}
>  			else
>  				log_err( _("rindex not fixed.\n"));
> -			gfs2_compute_bitstructs(sdp, actual);
> +			gfs2_compute_bitstructs(sdp->sd_sb.sb_bsize, actual);
>  			rindex_modified = FALSE;
>  		}
>  		e = enext;
> diff --git a/gfs2/libgfs2/fs_geometry.c b/gfs2/libgfs2/fs_geometry.c
> index e716127..932a2e6 100644
> --- a/gfs2/libgfs2/fs_geometry.c
> +++ b/gfs2/libgfs2/fs_geometry.c
> @@ -207,7 +207,7 @@ void build_rgrps(struct gfs2_sbd *sdp, int do_write)
>  		rl->rg.rg_header.mh_format = GFS2_FORMAT_RG;
>  		rl->rg.rg_free = rgblocks;
>  
> -		if (gfs2_compute_bitstructs(sdp, rl)) {
> +		if (gfs2_compute_bitstructs(sdp->sd_sb.sb_bsize, rl)) {
>  			fprintf(stderr, "%s: Unable to build resource groups "
>  				"with these characteristics.\n", __FUNCTION__);
>  			exit(-1);
> diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
> index e069e9d..997e23f 100644
> --- a/gfs2/libgfs2/libgfs2.h
> +++ b/gfs2/libgfs2/libgfs2.h
> @@ -725,7 +725,7 @@ extern int gfs2_find_jhead(struct gfs2_inode *ip, struct gfs2_log_header *head);
>  extern int clean_journal(struct gfs2_inode *ip, struct gfs2_log_header *head);
>  
>  /* rgrp.c */
> -extern int gfs2_compute_bitstructs(struct gfs2_sbd *sdp, struct rgrp_tree *rgd);
> +extern int gfs2_compute_bitstructs(const uint32_t bsize, struct rgrp_tree *rgd);
>  extern struct rgrp_tree *gfs2_blk2rgrpd(struct gfs2_sbd *sdp, uint64_t blk);
>  extern uint64_t gfs2_rgrp_read(struct gfs2_sbd *sdp, struct rgrp_tree *rgd);
>  extern void gfs2_rgrp_relse(struct rgrp_tree *rgd);
> diff --git a/gfs2/libgfs2/rgrp.c b/gfs2/libgfs2/rgrp.c
> index f7dc01e..f2b8304 100644
> --- a/gfs2/libgfs2/rgrp.c
> +++ b/gfs2/libgfs2/rgrp.c
> @@ -12,11 +12,11 @@
>  
>  /**
>   * gfs2_compute_bitstructs - Compute the bitmap sizes
> - * @rgd: The resource group descriptor
> - *
> + * bsize: Block size
> + * rgd: The resource group descriptor
>   * Returns: 0 on success, -1 on error
>   */
> -int gfs2_compute_bitstructs(struct gfs2_sbd *sdp, struct rgrp_tree *rgd)
> +int gfs2_compute_bitstructs(const uint32_t bsize, struct rgrp_tree *rgd)
>  {
>  	struct gfs2_bitmap *bits;
>  	uint32_t length = rgd->ri.ri_length;
> @@ -49,7 +49,7 @@ int gfs2_compute_bitstructs(struct gfs2_sbd *sdp, struct rgrp_tree *rgd)
>  			bits->bi_len = bytes;
>  		}
>  		else if (x == 0){
> -			bytes = sdp->sd_sb.sb_bsize - sizeof(struct gfs2_rgrp);
> +			bytes = bsize - sizeof(struct gfs2_rgrp);
>  			bits->bi_offset = sizeof(struct gfs2_rgrp);
>  			bits->bi_start = 0;
>  			bits->bi_len = bytes;
> @@ -61,7 +61,7 @@ int gfs2_compute_bitstructs(struct gfs2_sbd *sdp, struct rgrp_tree *rgd)
>  			bits->bi_len = bytes;
>  		}
>  		else{
> -			bytes = sdp->sd_sb.sb_bsize - sizeof(struct gfs2_meta_header);
> +			bytes = bsize - sizeof(struct gfs2_meta_header);
>  			bits->bi_offset = sizeof(struct gfs2_meta_header);
>  			bits->bi_start = rgd->ri.ri_bitbytes - bytes_left;
>  			bits->bi_len = bytes;
> diff --git a/gfs2/libgfs2/super.c b/gfs2/libgfs2/super.c
> index 21c9f7b..eb97c40 100644
> --- a/gfs2/libgfs2/super.c
> +++ b/gfs2/libgfs2/super.c
> @@ -188,7 +188,7 @@ int rindex_read(struct gfs2_sbd *sdp, int fd, int *count1, int *sane)
>  			prev_rgd->length = rgrp_size(prev_rgd);
>  		}
>  
> -		if(gfs2_compute_bitstructs(sdp, rgd))
> +		if(gfs2_compute_bitstructs(sdp->sd_sb.sb_bsize, rgd))
>  			*sane = 0;
>  
>  		(*count1)++;
> diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
> index aadd501..76b34b8 100644
> --- a/gfs2/mkfs/main_mkfs.c
> +++ b/gfs2/mkfs/main_mkfs.c
> @@ -613,7 +613,7 @@ static int place_rgrps(struct gfs2_sbd *sdp, const struct mkfs_opts *opts)
>  
>  		/* TODO: This call allocates buffer heads and bitmap pointers
>  		 * in rgt. We really shouldn't need to do that. */
> -		err = gfs2_compute_bitstructs(sdp, rgt);
> +		err = gfs2_compute_bitstructs(sdp->bsize, rgt);
>  		if (err != 0) {
>  			fprintf(stderr, _("Could not compute bitmaps. "
>  			        "Check resource group and block size options.\n"));




^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Cluster-devel] [PATCH 0/6] More mkfs.gfs2 reworking
  2013-05-14 10:45 [Cluster-devel] [PATCH 0/6] More mkfs.gfs2 reworking Andrew Price
                   ` (5 preceding siblings ...)
  2013-05-14 10:45 ` [Cluster-devel] [PATCH 6/6] libgfs2: Clarify gfs2_compute_bitstructs's parameters Andrew Price
@ 2013-05-14 12:21 ` Bob Peterson
  6 siblings, 0 replies; 15+ messages in thread
From: Bob Peterson @ 2013-05-14 12:21 UTC (permalink / raw)
  To: cluster-devel.redhat.com

----- Original Message -----
| The stripe alignment work is ongoing but these patches are ready to go so I
| thought I'd get them posted.
| 
| Andrew Price (6):
|   mkfs.gfs2: Add options for stripe size and width
|   libgfs2: Remove 'writes' field from gfs2_sbd
|   mkfs.gfs2: Link to libblkid
|   mkfs.gfs2: Use libblkid for checking contents
|   mkfs.gfs2: Add a struct to store device info
|   libgfs2: Clarify gfs2_compute_bitstructs's parameters

Hi,

The patches look good.

Regards,

Bob Peterson
Red Hat File Systems



^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2013-05-14 12:53 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-14 10:45 [Cluster-devel] [PATCH 0/6] More mkfs.gfs2 reworking Andrew Price
2013-05-14 10:45 ` [Cluster-devel] [PATCH 1/6] mkfs.gfs2: Add options for stripe size and width Andrew Price
2013-05-14 10:49   ` Steven Whitehouse
2013-05-14 11:21     ` Andrew Price
2013-05-14 12:53     ` [Cluster-devel] [PATCH 1/6 v2] " Andrew Price
2013-05-14 12:52       ` Steven Whitehouse
2013-05-14 10:45 ` [Cluster-devel] [PATCH 2/6] libgfs2: Remove 'writes' field from gfs2_sbd Andrew Price
2013-05-14 10:45 ` [Cluster-devel] [PATCH 3/6] mkfs.gfs2: Link to libblkid Andrew Price
2013-05-14 10:45 ` [Cluster-devel] [PATCH 4/6] mkfs.gfs2: Use libblkid for checking contents Andrew Price
2013-05-14 10:52   ` Steven Whitehouse
2013-05-14 11:00     ` Andrew Price
2013-05-14 10:45 ` [Cluster-devel] [PATCH 5/6] mkfs.gfs2: Add a struct to store device info Andrew Price
2013-05-14 10:45 ` [Cluster-devel] [PATCH 6/6] libgfs2: Clarify gfs2_compute_bitstructs's parameters Andrew Price
2013-05-14 10:54   ` Steven Whitehouse
2013-05-14 12:21 ` [Cluster-devel] [PATCH 0/6] More mkfs.gfs2 reworking Bob Peterson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).