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;