From: Heinz Mauelshagen Date: Thu, 19 May 2016 16:49:25 +0000 (+0200) Subject: dm raid: use dm_arg_set API in constructor X-Git-Url: https://git.karo-electronics.de/?a=commitdiff_plain;h=92c83d79b07ec1c53e0c74b8a7988799e00856db;p=linux-beck.git 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 Signed-off-by: Mike Snitzer --- 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 #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 ] 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_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 */ + 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 */ + 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;