[Cluster-devel] [PATCH 1/6] mkfs.gfs2: Add options for stripe size and width

[Andrew Price <anprice@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.
>
>