dm raid: use dm_arg_set API in constructor

- use dm_arg_set API in ctr and its callees parse_raid_params() and dev_parms()

- introduce _in_range() function to check a value is in a [ min, max ] range;
  this is to support more callers in parsing parameters etc. in the future

- correct comment on MAX_RAID_DEVICES

Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>

1 files changed, 84 insertions, 61 deletions

diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
index 6982e23681be..01aa511ebe44 100644
--- a/drivers/md/dm-raid.c
+++ b/drivers/md/dm-raid.c
@@ -17,7 +17,7 @@
 #include <linux/device-mapper.h>
 
 #define DM_MSG_PREFIX "raid"
-#define	MAX_RAID_DEVICES	253 /* raid4/5/6 limit */
+#define	MAX_RAID_DEVICES	253 /* md-raid kernel limit */
 
 static bool devices_handle_discard_safely = false;
 
@@ -95,6 +95,12 @@ static struct raid_type {
 	{"raid6_nc", "RAID6 (N continue)",		2, 4, 6, ALGORITHM_ROTATING_N_CONTINUE}
 };
 
+/* True, if @v is in inclusive range [@min, @max] */
+static bool _in_range(long v, long min, long max)
+{
+	return v >= min && v <= max;
+}
+
 static char *raid10_md_layout_to_format(int layout)
 {
 	/*
@@ -135,7 +141,7 @@ static int raid10_format_to_md_layout(char *format, unsigned copies)
 	return (f << 8) | n;
 }
 
-static struct raid_type *get_raid_type(char *name)
+static struct raid_type *get_raid_type(const char *name)
 {
 	int i;
 
@@ -220,14 +226,20 @@ static void context_free(struct raid_set *rs)
  * This code parses those words.  If there is a failure,
  * the caller must use context_free to unwind the operations.
  */
-static int dev_parms(struct raid_set *rs, char **argv)
+static int parse_dev_parms(struct raid_set *rs, struct dm_arg_set *as)
 {
 	int i;
 	int rebuild = 0;
 	int metadata_available = 0;
 	int r = 0;
+	const char *arg;
 
-	for (i = 0; i < rs->md.raid_disks; i++, argv += 2) {
+	/* Put off the number of raid devices argument to get to dev pairs */
+	arg = dm_shift_arg(as);
+	if (!arg)
+		return -EINVAL;
+
+	for (i = 0; i < rs->md.raid_disks; i++) {
 		rs->dev[i].rdev.raid_disk = i;
 
 		rs->dev[i].meta_dev = NULL;
@@ -240,8 +252,12 @@ static int dev_parms(struct raid_set *rs, char **argv)
 		rs->dev[i].rdev.data_offset = 0;
 		rs->dev[i].rdev.mddev = &rs->md;
 
-		if (strcmp(argv[0], "-")) {
-			r = dm_get_device(rs->ti, argv[0],
+		arg = dm_shift_arg(as);
+		if (!arg)
+			return -EINVAL;
+
+		if (strcmp(arg, "-")) {
+			r = dm_get_device(rs->ti, arg,
 					    dm_table_get_mode(rs->ti->table),
 					    &rs->dev[i].meta_dev);
 			rs->ti->error = "RAID metadata device lookup failure";
@@ -253,7 +269,11 @@ static int dev_parms(struct raid_set *rs, char **argv)
 				return -ENOMEM;
 		}
 
-		if (!strcmp(argv[1], "-")) {
+		arg = dm_shift_arg(as);
+		if (!arg)
+			return -EINVAL;
+
+		if (!strcmp(arg, "-")) {
 			if (!test_bit(In_sync, &rs->dev[i].rdev.flags) &&
 			    (!rs->dev[i].rdev.recovery_offset)) {
 				rs->ti->error = "Drive designated for rebuild not specified";
@@ -267,7 +287,7 @@ static int dev_parms(struct raid_set *rs, char **argv)
 			continue;
 		}
 
-		r = dm_get_device(rs->ti, argv[1],
+		r = dm_get_device(rs->ti, arg,
 				    dm_table_get_mode(rs->ti->table),
 				    &rs->dev[i].data_dev);
 		if (r) {
@@ -492,25 +512,30 @@ too_many:
  *    [raid10_copies <# copies>]        Number of copies.  (Default: 2)
  *    [raid10_format <near|far|offset>] Layout algorithm.  (Default: near)
  */
-static int parse_raid_params(struct raid_set *rs, char **argv,
+static int parse_raid_params(struct raid_set *rs, struct dm_arg_set *as,
 			     unsigned num_raid_params)
 {
 	char *raid10_format = "near";
 	unsigned raid10_copies = 2;
 	unsigned i;
-	unsigned long value, region_size = 0;
+	unsigned value, region_size = 0;
 	sector_t sectors_per_dev = rs->ti->len;
 	sector_t max_io_len;
-	char *key;
+	const char *arg, *key;
+
+	arg = dm_shift_arg(as);
+	num_raid_params--; /* Account for chunk_size argument */
+
+	if (kstrtouint(arg, 10, &value) < 0) {
+		rs->ti->error = "Bad numerical argument given for chunk_size";
+		return -EINVAL;
+	}
 
 	/*
 	 * First, parse the in-order required arguments
 	 * "chunk_size" is the only argument of this type.
 	 */
-	if ((kstrtoul(argv[0], 10, &value) < 0)) {
-		rs->ti->error = "Bad chunk size";
-		return -EINVAL;
-	} else if (rs->raid_type->level == 1) {
+	if (rs->raid_type->level == 1) {
 		if (value)
 			DMERR("Ignoring chunk size parameter for RAID 1");
 		value = 0;
@@ -523,8 +548,6 @@ static int parse_raid_params(struct raid_set *rs, char **argv,
 	}
 
 	rs->md.new_chunk_sectors = rs->md.chunk_sectors = value;
-	argv++;
-	num_raid_params--;
 
 	/*
 	 * We set each individual device as In_sync with a completed
@@ -552,12 +575,18 @@ static int parse_raid_params(struct raid_set *rs, char **argv,
 	 * Second, parse the unordered optional arguments
 	 */
 	for (i = 0; i < num_raid_params; i++) {
-		if (!strcasecmp(argv[i], "nosync")) {
+		arg = dm_shift_arg(as);
+		if (!arg) {
+			rs->ti->error = "Not enough raid parameters given";
+			return -EINVAL;
+		}
+
+		if (!strcasecmp(arg, "nosync")) {
 			rs->md.recovery_cp = MaxSector;
 			rs->ctr_flags |= CTR_FLAG_NOSYNC;
 			continue;
 		}
-		if (!strcasecmp(argv[i], "sync")) {
+		if (!strcasecmp(arg, "sync")) {
 			rs->md.recovery_cp = 0;
 			rs->ctr_flags |= CTR_FLAG_SYNC;
 			continue;
@@ -569,7 +598,9 @@ static int parse_raid_params(struct raid_set *rs, char **argv,
 			return -EINVAL;
 		}
 
-		key = argv[i++];
+		key = arg;
+		arg = dm_shift_arg(as);
+		i++; /* Account for the argument pairs */
 
 		/* Parameters that take a string value are checked here. */
 		if (!strcasecmp(key, "raid10_format")) {
@@ -577,18 +608,18 @@ static int parse_raid_params(struct raid_set *rs, char **argv,
 				rs->ti->error = "'raid10_format' is an invalid parameter for this RAID type";
 				return -EINVAL;
 			}
-			if (strcmp("near", argv[i]) &&
-			    strcmp("far", argv[i]) &&
-			    strcmp("offset", argv[i])) {
+			if (strcmp("near", arg) &&
+			    strcmp("far", arg) &&
+			    strcmp("offset", arg)) {
 				rs->ti->error = "Invalid 'raid10_format' value given";
 				return -EINVAL;
 			}
-			raid10_format = argv[i];
+			raid10_format = (char *) arg;
 			rs->ctr_flags |= CTR_FLAG_RAID10_FORMAT;
 			continue;
 		}
 
-		if (kstrtoul(argv[i], 10, &value) < 0) {
+		if (kstrtouint(arg, 10, &value) < 0) {
 			rs->ti->error = "Bad numerical argument given in raid params";
 			return -EINVAL;
 		}
@@ -1223,61 +1254,53 @@ static int raid_ctr(struct dm_target *ti, unsigned argc, char **argv)
 {
 	int r;
 	struct raid_type *rt;
-	unsigned long num_raid_params, num_raid_devs;
+	unsigned num_raid_params, num_raid_devs;
 	struct raid_set *rs = NULL;
-
-	/* Must have at least <raid_type> <#raid_params> */
-	if (argc < 2) {
-		ti->error = "Too few arguments";
+	const char *arg;
+	struct dm_arg_set as = { argc, argv }, as_nrd;
+	struct dm_arg _args[] = {
+		{ 0, as.argc, "Cannot understand number of raid parameters" },
+		{ 1, 254, "Cannot understand number of raid devices parameters" }
+	};
+
+	/* Must have <raid_type> */
+	arg = dm_shift_arg(&as);
+	if (!arg) {
+		ti->error = "No arguments";
 		return -EINVAL;
 	}
 
-	/* raid type */
-	rt = get_raid_type(argv[0]);
+	rt = get_raid_type(arg);
 	if (!rt) {
 		ti->error = "Unrecognised raid_type";
 		return -EINVAL;
 	}
-	argc--;
-	argv++;
-
-	/* number of RAID parameters */
-	if (kstrtoul(argv[0], 10, &num_raid_params) < 0) {
-		ti->error = "Cannot understand number of RAID parameters";
-		return -EINVAL;
-	}
-	argc--;
-	argv++;
 
-	/* Skip over RAID params for now and find out # of devices */
-	if (num_raid_params >= argc) {
-		ti->error = "Arguments do not agree with counts given";
-		return -EINVAL;
-	}
+	/* Must have <#raid_params> */
+	if (dm_read_arg_group(_args, &as, &num_raid_params, &ti->error))
+                return -EINVAL;
 
-	if ((kstrtoul(argv[num_raid_params], 10, &num_raid_devs) < 0) ||
-	    (num_raid_devs > MAX_RAID_DEVICES)) {
-		ti->error = "Cannot understand number of raid devices";
-		return -EINVAL;
-	}
+	/* number of raid device tupples <meta_dev data_dev> */
+	as_nrd = as;
+	dm_consume_args(&as_nrd, num_raid_params);
+	_args[1].max = (as_nrd.argc - 1) / 2;
+	if (dm_read_arg(_args + 1, &as_nrd, &num_raid_devs, &ti->error))
+                return -EINVAL;
 
-	argc -= num_raid_params + 1; /* +1: we already have num_raid_devs */
-	if (argc != (num_raid_devs * 2)) {
-		ti->error = "Supplied RAID devices does not match the count given";
-		return -EINVAL;
+	if (!_in_range(num_raid_devs, 1, MAX_RAID_DEVICES)) {
+		ti->error = "Invalid number of supplied raid devices";
+                return -EINVAL;
 	}
 
-	rs = context_alloc(ti, rt, (unsigned)num_raid_devs);
+	rs = context_alloc(ti, rt, num_raid_devs);
 	if (IS_ERR(rs))
 		return PTR_ERR(rs);
 
-	r = parse_raid_params(rs, argv, (unsigned)num_raid_params);
+	r = parse_raid_params(rs, &as, num_raid_params);
 	if (r)
 		goto bad;
 
-	argv += num_raid_params + 1;
-
-	r = dev_parms(rs, argv);
+	r = parse_dev_parms(rs, &as);
 	if (r)
 		goto bad;