From 702108d194e3649f69afcd2661282a0157c71e54 Mon Sep 17 00:00:00 2001 From: Heinz Mauelshagen Date: Thu, 19 May 2016 18:49:26 +0200 Subject: [PATCH] dm raid: cleanup / provide infrastructure Provide necessary infrastructure to handle ctr flags and their names and cleanup setting ti->error: - comment constructor flags - introduce constructor flag manipulation - introduce ti_error_*() functions to simplify setting the error message (use in other targets?) - introduce array to hold ctr flag <-> flag name mapping - introduce argument name by flag functions for that array - use those functions throughout the ctr call path Signed-off-by: Heinz Mauelshagen Signed-off-by: Mike Snitzer --- drivers/md/dm-raid.c | 424 +++++++++++++++++++++++-------------------- 1 file changed, 228 insertions(+), 196 deletions(-) diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c index 01aa511ebe44..ab7aa7d83364 100644 --- a/drivers/md/dm-raid.c +++ b/drivers/md/dm-raid.c @@ -1,6 +1,6 @@ /* * Copyright (C) 2010-2011 Neil Brown - * Copyright (C) 2010-2015 Red Hat, Inc. All rights reserved. + * Copyright (C) 2010-2016 Red Hat, Inc. All rights reserved. * * This file is released under the GPL. */ @@ -47,18 +47,22 @@ struct raid_dev { /* * Flags for rs->ctr_flags field. + * + * 1 = no flag value + * 2 = flag with value */ -#define CTR_FLAG_SYNC 0x1 -#define CTR_FLAG_NOSYNC 0x2 -#define CTR_FLAG_REBUILD 0x4 -#define CTR_FLAG_DAEMON_SLEEP 0x8 -#define CTR_FLAG_MIN_RECOVERY_RATE 0x10 -#define CTR_FLAG_MAX_RECOVERY_RATE 0x20 -#define CTR_FLAG_MAX_WRITE_BEHIND 0x40 -#define CTR_FLAG_STRIPE_CACHE 0x80 -#define CTR_FLAG_REGION_SIZE 0x100 -#define CTR_FLAG_RAID10_COPIES 0x200 -#define CTR_FLAG_RAID10_FORMAT 0x400 +#define CTR_FLAG_SYNC 0x1 /* 1 */ /* Not with raid0! */ +#define CTR_FLAG_NOSYNC 0x2 /* 1 */ /* Not with raid0! */ +#define CTR_FLAG_REBUILD 0x4 /* 2 */ /* Not with raid0! */ +#define CTR_FLAG_DAEMON_SLEEP 0x8 /* 2 */ /* Not with raid0! */ +#define CTR_FLAG_MIN_RECOVERY_RATE 0x10 /* 2 */ /* Not with raid0! */ +#define CTR_FLAG_MAX_RECOVERY_RATE 0x20 /* 2 */ /* Not with raid0! */ +#define CTR_FLAG_MAX_WRITE_BEHIND 0x40 /* 2 */ /* Only with raid1! */ +#define CTR_FLAG_WRITE_MOSTLY 0x80 /* 2 */ /* Only with raid1! */ +#define CTR_FLAG_STRIPE_CACHE 0x100 /* 2 */ /* Only with raid4/5/6! */ +#define CTR_FLAG_REGION_SIZE 0x200 /* 2 */ /* Not with raid0! */ +#define CTR_FLAG_RAID10_COPIES 0x400 /* 2 */ /* Only with raid10 */ +#define CTR_FLAG_RAID10_FORMAT 0x800 /* 2 */ /* Only with raid10 */ struct raid_set { struct dm_target *ti; @@ -101,6 +105,83 @@ static bool _in_range(long v, long min, long max) return v >= min && v <= max; } +/* ctr flag bit manipulation... */ +/* Set single @flag in @flags */ +static void _set_flag(uint32_t flag, uint32_t *flags) +{ + WARN_ON_ONCE(hweight32(flag) != 1); + *flags |= flag; +} + +/* Test single @flag in @flags */ +static bool _test_flag(uint32_t flag, uint32_t flags) +{ + WARN_ON_ONCE(hweight32(flag) != 1); + return (flag & flags) ? true : false; +} + +/* Return true if single @flag is set in @*flags, else set it and return false */ +static bool _test_and_set_flag(uint32_t flag, uint32_t *flags) +{ + if (_test_flag(flag, *flags)) + return true; + + _set_flag(flag, flags); + return false; +} +/* ...ctr and runtime flag bit manipulation */ + +/* All table line arguments are defined here */ +static struct arg_name_flag { + const uint32_t flag; + const char *name; +} _arg_name_flags[] = { + { CTR_FLAG_SYNC, "sync"}, + { CTR_FLAG_NOSYNC, "nosync"}, + { CTR_FLAG_REBUILD, "rebuild"}, + { CTR_FLAG_DAEMON_SLEEP, "daemon_sleep"}, + { CTR_FLAG_MIN_RECOVERY_RATE, "min_recovery_rate"}, + { CTR_FLAG_MAX_RECOVERY_RATE, "max_recovery_rate"}, + { CTR_FLAG_MAX_WRITE_BEHIND, "max_write_behind"}, + { CTR_FLAG_WRITE_MOSTLY, "writemostly"}, + { CTR_FLAG_STRIPE_CACHE, "stripe_cache"}, + { CTR_FLAG_REGION_SIZE, "region_size"}, + { CTR_FLAG_RAID10_COPIES, "raid10_copies"}, + { CTR_FLAG_RAID10_FORMAT, "raid10_format"}, +}; + +/* Return argument name string for given @flag */ +static const char *_argname_by_flag(const uint32_t flag) +{ + if (hweight32(flag) == 1) { + struct arg_name_flag *anf = _arg_name_flags + ARRAY_SIZE(_arg_name_flags); + + while (anf-- > _arg_name_flags) + if (_test_flag(flag, anf->flag)) + return anf->name; + + } else + DMERR("%s called with more than one flag!", __func__); + + return NULL; +} + +/* + * Convenience functions to set ti->error to @errmsg and + * return @r in order to shorten code in a lot of places + */ +static int ti_error_ret(struct dm_target *ti, const char *errmsg, int r) +{ + ti->error = (char *) errmsg; + return r; +} + +static int ti_error_einval(struct dm_target *ti, const char *errmsg) +{ + return ti_error_ret(ti, errmsg, -EINVAL); +} +/* END: convenience functions to set ti->error to @errmsg... */ + static char *raid10_md_layout_to_format(int layout) { /* @@ -157,16 +238,12 @@ static struct raid_set *context_alloc(struct dm_target *ti, struct raid_type *ra unsigned i; struct raid_set *rs; - if (raid_devs <= raid_type->parity_devs) { - ti->error = "Insufficient number of devices"; - return ERR_PTR(-EINVAL); - } + if (raid_devs <= raid_type->parity_devs) + return ERR_PTR(ti_error_einval(ti, "Insufficient number of devices")); rs = kzalloc(sizeof(*rs) + raid_devs * sizeof(rs->dev[0]), GFP_KERNEL); - if (!rs) { - ti->error = "Cannot allocate raid context"; - return ERR_PTR(-ENOMEM); - } + if (!rs) + return ERR_PTR(ti_error_ret(ti, "Cannot allocate raid context", -ENOMEM)); mddev_init(&rs->md); @@ -226,7 +303,7 @@ 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 parse_dev_parms(struct raid_set *rs, struct dm_arg_set *as) +static int parse_dev_params(struct raid_set *rs, struct dm_arg_set *as) { int i; int rebuild = 0; @@ -260,13 +337,12 @@ static int parse_dev_parms(struct raid_set *rs, struct dm_arg_set *as) 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"; if (r) - return r; + return ti_error_ret(rs->ti, "RAID metadata device lookup failure", r); rs->dev[i].rdev.sb_page = alloc_page(GFP_KERNEL); if (!rs->dev[i].rdev.sb_page) - return -ENOMEM; + return ti_error_ret(rs->ti, "Failed to allocate superblock page", -ENOMEM); } arg = dm_shift_arg(as); @@ -275,14 +351,11 @@ static int parse_dev_parms(struct raid_set *rs, struct dm_arg_set *as) 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"; - return -EINVAL; - } + (!rs->dev[i].rdev.recovery_offset)) + return ti_error_einval(rs->ti, "Drive designated for rebuild not specified"); - rs->ti->error = "No data device supplied with metadata device"; if (rs->dev[i].meta_dev) - return -EINVAL; + return ti_error_einval(rs->ti, "No data device supplied with metadata device"); continue; } @@ -290,10 +363,8 @@ static int parse_dev_parms(struct raid_set *rs, struct dm_arg_set *as) r = dm_get_device(rs->ti, arg, dm_table_get_mode(rs->ti->table), &rs->dev[i].data_dev); - if (r) { - rs->ti->error = "RAID device lookup failure"; - return r; - } + if (r) + return ti_error_ret(rs->ti, "RAID device lookup failure", r); if (rs->dev[i].meta_dev) { metadata_available = 1; @@ -322,8 +393,7 @@ static int parse_dev_parms(struct raid_set *rs, struct dm_arg_set *as) * User could specify 'nosync' option if desperate. */ DMERR("Unable to rebuild drive while array is not in-sync"); - rs->ti->error = "RAID device lookup failure"; - return -EINVAL; + return ti_error_einval(rs->ti, "Unable to rebuild drive while array is not in-sync"); } return 0; @@ -360,27 +430,20 @@ static int validate_region_size(struct raid_set *rs, unsigned long region_size) /* * Validate user-supplied value. */ - if (region_size > rs->ti->len) { - rs->ti->error = "Supplied region size is too large"; - return -EINVAL; - } + if (region_size > rs->ti->len) + return ti_error_einval(rs->ti, "Supplied region size is too large"); if (region_size < min_region_size) { DMERR("Supplied region_size (%lu sectors) below minimum (%lu)", region_size, min_region_size); - rs->ti->error = "Supplied region size is too small"; - return -EINVAL; + return ti_error_einval(rs->ti, "Supplied region size is too small"); } - if (!is_power_of_2(region_size)) { - rs->ti->error = "Region size is not a power of 2"; - return -EINVAL; - } + if (!is_power_of_2(region_size)) + return ti_error_einval(rs->ti, "Region size is not a power of 2"); - if (region_size < rs->md.chunk_sectors) { - rs->ti->error = "Region size is smaller than the chunk size"; - return -EINVAL; - } + if (region_size < rs->md.chunk_sectors) + return ti_error_einval(rs->ti, "Region size is smaller than the chunk size"); } /* @@ -522,14 +585,13 @@ static int parse_raid_params(struct raid_set *rs, struct dm_arg_set *as, sector_t sectors_per_dev = rs->ti->len; sector_t max_io_len; const char *arg, *key; + struct raid_dev *rd; 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; - } + if (kstrtouint(arg, 10, &value) < 0) + return ti_error_einval(rs->ti, "Bad numerical argument given for chunk_size"); /* * First, parse the in-order required arguments @@ -539,13 +601,10 @@ static int parse_raid_params(struct raid_set *rs, struct dm_arg_set *as, if (value) DMERR("Ignoring chunk size parameter for RAID 1"); value = 0; - } else if (!is_power_of_2(value)) { - rs->ti->error = "Chunk size must be a power of 2"; - return -EINVAL; - } else if (value < 8) { - rs->ti->error = "Chunk size value is too small"; - return -EINVAL; - } + } else if (!is_power_of_2(value)) + return ti_error_einval(rs->ti, "Chunk size must be a power of 2"); + else if (value < 8) + return ti_error_einval(rs->ti, "Chunk size value is too small"); rs->md.new_chunk_sectors = rs->md.chunk_sectors = value; @@ -576,144 +635,134 @@ static int parse_raid_params(struct raid_set *rs, struct dm_arg_set *as, */ for (i = 0; i < num_raid_params; i++) { arg = dm_shift_arg(as); - if (!arg) { - rs->ti->error = "Not enough raid parameters given"; - return -EINVAL; - } + if (!arg) + return ti_error_einval(rs->ti, "Not enough raid parameters given"); if (!strcasecmp(arg, "nosync")) { rs->md.recovery_cp = MaxSector; - rs->ctr_flags |= CTR_FLAG_NOSYNC; + _set_flag(CTR_FLAG_NOSYNC, &rs->ctr_flags); continue; } if (!strcasecmp(arg, "sync")) { rs->md.recovery_cp = 0; - rs->ctr_flags |= CTR_FLAG_SYNC; + _set_flag(CTR_FLAG_SYNC, &rs->ctr_flags); continue; } - /* The rest of the optional arguments come in key/value pairs */ - if ((i + 1) >= num_raid_params) { - rs->ti->error = "Wrong number of raid parameters given"; - return -EINVAL; - } - key = arg; arg = dm_shift_arg(as); i++; /* Account for the argument pairs */ + if (!arg) + return ti_error_einval(rs->ti, "Wrong number of raid parameters given"); - /* Parameters that take a string value are checked here. */ - if (!strcasecmp(key, "raid10_format")) { - if (rs->raid_type->level != 10) { - rs->ti->error = "'raid10_format' is an invalid parameter for this RAID type"; - return -EINVAL; - } + /* + * Parameters that take a string value are checked here. + */ + + if (!strcasecmp(key, _argname_by_flag(CTR_FLAG_RAID10_FORMAT))) { + if (_test_and_set_flag(CTR_FLAG_RAID10_FORMAT, &rs->ctr_flags)) + return ti_error_einval(rs->ti, "Only one raid10_format argument pair allowed"); + if (rs->raid_type->level != 10) + return ti_error_einval(rs->ti, "'raid10_format' is an invalid parameter for this RAID type"); if (strcmp("near", arg) && strcmp("far", arg) && - strcmp("offset", arg)) { - rs->ti->error = "Invalid 'raid10_format' value given"; - return -EINVAL; - } + strcmp("offset", arg)) + return ti_error_einval(rs->ti, "Invalid 'raid10_format' value given"); + raid10_format = (char *) arg; - rs->ctr_flags |= CTR_FLAG_RAID10_FORMAT; continue; } - if (kstrtouint(arg, 10, &value) < 0) { - rs->ti->error = "Bad numerical argument given in raid params"; - return -EINVAL; - } + if (kstrtouint(arg, 10, &value) < 0) + return ti_error_einval(rs->ti, "Bad numerical argument given in raid params"); + + if (!strcasecmp(key, _argname_by_flag(CTR_FLAG_REBUILD))) { + /* + * "rebuild" is being passed in by userspace to provide + * indexes of replaced devices and to set up additional + * devices on raid level takeover. + */ + if (!_in_range(value, 0, rs->md.raid_disks - 1)) + return ti_error_einval(rs->ti, "Invalid rebuild index given"); + + rd = rs->dev + value; + clear_bit(In_sync, &rd->rdev.flags); + clear_bit(Faulty, &rd->rdev.flags); + rd->rdev.recovery_offset = 0; + _set_flag(CTR_FLAG_REBUILD, &rs->ctr_flags); + } else if (!strcasecmp(key, _argname_by_flag(CTR_FLAG_WRITE_MOSTLY))) { + if (rs->raid_type->level != 1) + return ti_error_einval(rs->ti, "write_mostly option is only valid for RAID1"); + + if (!_in_range(value, 0, rs->md.raid_disks - 1)) + return ti_error_einval(rs->ti, "Invalid write_mostly index given"); - /* Parameters that take a numeric value are checked here */ - if (!strcasecmp(key, "rebuild")) { - if (value >= rs->md.raid_disks) { - rs->ti->error = "Invalid rebuild index given"; - return -EINVAL; - } - clear_bit(In_sync, &rs->dev[value].rdev.flags); - rs->dev[value].rdev.recovery_offset = 0; - rs->ctr_flags |= CTR_FLAG_REBUILD; - } else if (!strcasecmp(key, "write_mostly")) { - if (rs->raid_type->level != 1) { - rs->ti->error = "write_mostly option is only valid for RAID1"; - return -EINVAL; - } - if (value >= rs->md.raid_disks) { - rs->ti->error = "Invalid write_mostly drive index given"; - return -EINVAL; - } set_bit(WriteMostly, &rs->dev[value].rdev.flags); - } else if (!strcasecmp(key, "max_write_behind")) { - if (rs->raid_type->level != 1) { - rs->ti->error = "max_write_behind option is only valid for RAID1"; - return -EINVAL; - } - rs->ctr_flags |= CTR_FLAG_MAX_WRITE_BEHIND; + _set_flag(CTR_FLAG_WRITE_MOSTLY, &rs->ctr_flags); + } else if (!strcasecmp(key, _argname_by_flag(CTR_FLAG_MAX_WRITE_BEHIND))) { + if (rs->raid_type->level != 1) + return ti_error_einval(rs->ti, "max_write_behind option is only valid for RAID1"); + + if (_test_and_set_flag(CTR_FLAG_MAX_WRITE_BEHIND, &rs->ctr_flags)) + return ti_error_einval(rs->ti, "Only one max_write_behind argument pair allowed"); /* * In device-mapper, we specify things in sectors, but * MD records this value in kB */ value /= 2; - if (value > COUNTER_MAX) { - rs->ti->error = "Max write-behind limit out of range"; - return -EINVAL; - } + if (value > COUNTER_MAX) + return ti_error_einval(rs->ti, "Max write-behind limit out of range"); + rs->md.bitmap_info.max_write_behind = value; - } else if (!strcasecmp(key, "daemon_sleep")) { - rs->ctr_flags |= CTR_FLAG_DAEMON_SLEEP; - if (!value || (value > MAX_SCHEDULE_TIMEOUT)) { - rs->ti->error = "daemon sleep period out of range"; - return -EINVAL; - } + } else if (!strcasecmp(key, _argname_by_flag(CTR_FLAG_DAEMON_SLEEP))) { + if (_test_and_set_flag(CTR_FLAG_DAEMON_SLEEP, &rs->ctr_flags)) + return ti_error_einval(rs->ti, "Only one daemon_sleep argument pair allowed"); + if (!value || (value > MAX_SCHEDULE_TIMEOUT)) + return ti_error_einval(rs->ti, "daemon sleep period out of range"); rs->md.bitmap_info.daemon_sleep = value; - } else if (!strcasecmp(key, "stripe_cache")) { - rs->ctr_flags |= CTR_FLAG_STRIPE_CACHE; - + } else if (!strcasecmp(key, _argname_by_flag(CTR_FLAG_STRIPE_CACHE))) { + if (_test_and_set_flag(CTR_FLAG_STRIPE_CACHE, &rs->ctr_flags)) + return ti_error_einval(rs->ti, "Only one stripe_cache argument pair allowed"); /* * In device-mapper, we specify things in sectors, but * MD records this value in kB */ value /= 2; - if ((rs->raid_type->level != 5) && - (rs->raid_type->level != 6)) { - rs->ti->error = "Inappropriate argument: stripe_cache"; - return -EINVAL; - } - if (raid5_set_cache_size(&rs->md, (int)value)) { - rs->ti->error = "Bad stripe_cache size"; - return -EINVAL; - } - } else if (!strcasecmp(key, "min_recovery_rate")) { - rs->ctr_flags |= CTR_FLAG_MIN_RECOVERY_RATE; - if (value > INT_MAX) { - rs->ti->error = "min_recovery_rate out of range"; - return -EINVAL; - } + if (!_in_range(rs->raid_type->level, 4, 6)) + return ti_error_einval(rs->ti, "Inappropriate argument: stripe_cache"); + if (raid5_set_cache_size(&rs->md, (int)value)) + return ti_error_einval(rs->ti, "Bad stripe_cache size"); + + } else if (!strcasecmp(key, _argname_by_flag(CTR_FLAG_MIN_RECOVERY_RATE))) { + if (_test_and_set_flag(CTR_FLAG_MIN_RECOVERY_RATE, &rs->ctr_flags)) + return ti_error_einval(rs->ti, "Only one min_recovery_rate argument pair allowed"); + if (value > INT_MAX) + return ti_error_einval(rs->ti, "min_recovery_rate out of range"); rs->md.sync_speed_min = (int)value; - } else if (!strcasecmp(key, "max_recovery_rate")) { - rs->ctr_flags |= CTR_FLAG_MAX_RECOVERY_RATE; - if (value > INT_MAX) { - rs->ti->error = "max_recovery_rate out of range"; - return -EINVAL; - } + } else if (!strcasecmp(key, _argname_by_flag(CTR_FLAG_MAX_RECOVERY_RATE))) { + if (_test_and_set_flag(CTR_FLAG_MIN_RECOVERY_RATE, &rs->ctr_flags)) + return ti_error_einval(rs->ti, "Only one max_recovery_rate argument pair allowed"); + if (value > INT_MAX) + return ti_error_einval(rs->ti, "max_recovery_rate out of range"); rs->md.sync_speed_max = (int)value; - } else if (!strcasecmp(key, "region_size")) { - rs->ctr_flags |= CTR_FLAG_REGION_SIZE; + } else if (!strcasecmp(key, _argname_by_flag(CTR_FLAG_REGION_SIZE))) { + if (_test_and_set_flag(CTR_FLAG_REGION_SIZE, &rs->ctr_flags)) + return ti_error_einval(rs->ti, "Only one region_size argument pair allowed"); + region_size = value; - } else if (!strcasecmp(key, "raid10_copies") && - (rs->raid_type->level == 10)) { - if ((value < 2) || (value > 0xFF)) { - rs->ti->error = "Bad value for 'raid10_copies'"; - return -EINVAL; - } - rs->ctr_flags |= CTR_FLAG_RAID10_COPIES; + } else if (!strcasecmp(key, _argname_by_flag(CTR_FLAG_RAID10_COPIES))) { + if (_test_and_set_flag(CTR_FLAG_RAID10_COPIES, &rs->ctr_flags)) + return ti_error_einval(rs->ti, "Only one raid10_copies argument pair allowed"); + + if (!_in_range(value, 2, rs->md.raid_disks)) + return ti_error_einval(rs->ti, "Bad value for 'raid10_copies'"); + raid10_copies = value; } else { DMERR("Unable to parse RAID parameter: %s", key); - rs->ti->error = "Unable to parse RAID parameters"; - return -EINVAL; + return ti_error_einval(rs->ti, "Unable to parse RAID parameters"); } } @@ -729,19 +778,15 @@ static int parse_raid_params(struct raid_set *rs, struct dm_arg_set *as, return -EINVAL; if (rs->raid_type->level == 10) { - if (raid10_copies > rs->md.raid_disks) { - rs->ti->error = "Not enough devices to satisfy specification"; - return -EINVAL; - } + if (raid10_copies > rs->md.raid_disks) + return ti_error_einval(rs->ti, "Not enough devices to satisfy specification"); /* * If the format is not "near", we only support * two copies at the moment. */ - if (strcmp("near", raid10_format) && (raid10_copies > 2)) { - rs->ti->error = "Too many copies for given RAID10 format."; - return -EINVAL; - } + if (strcmp("near", raid10_format) && (raid10_copies > 2)) + return ti_error_einval(rs->ti, "Too many copies for given RAID10 format."); /* (Len * #mirrors) / #devices */ sectors_per_dev = rs->ti->len * raid10_copies; @@ -752,10 +797,9 @@ static int parse_raid_params(struct raid_set *rs, struct dm_arg_set *as, rs->md.new_layout = rs->md.layout; } else if ((!rs->raid_type->level || rs->raid_type->level > 1) && sector_div(sectors_per_dev, - (rs->md.raid_disks - rs->raid_type->parity_devs))) { - rs->ti->error = "Target length not divisible by number of data devices"; - return -EINVAL; - } + (rs->md.raid_disks - rs->raid_type->parity_devs))) + return ti_error_einval(rs->ti, "Target length not divisible by number of data devices"); + rs->md.dev_sectors = sectors_per_dev; /* Assume there are no metadata devices until the drives are parsed */ @@ -1035,11 +1079,9 @@ static int super_init_validation(struct mddev *mddev, struct md_rdev *rdev) if (!test_bit(FirstUse, &r->flags) && (r->raid_disk >= 0)) { role = le32_to_cpu(sb2->array_position); if (role != r->raid_disk) { - if (rs->raid_type->level != 1) { - rs->ti->error = "Cannot change device " - "positions in RAID array"; - return -EINVAL; - } + if (rs->raid_type->level != 1) + return ti_error_einval(rs->ti, "Cannot change device " + "positions in RAID array"); DMINFO("RAID1 device #%d now at position #%d", role, r->raid_disk); } @@ -1170,18 +1212,15 @@ static int analyse_superblocks(struct dm_target *ti, struct raid_set *rs) if (!freshest) return 0; - if (validate_raid_redundancy(rs)) { - rs->ti->error = "Insufficient redundancy to activate array"; - return -EINVAL; - } + if (validate_raid_redundancy(rs)) + return ti_error_einval(rs->ti, "Insufficient redundancy to activate array"); /* * Validation of the freshest device provides the source of * validation for the remaining devices. */ - ti->error = "Unable to assemble array: Invalid superblocks"; if (super_validate(rs, freshest)) - return -EINVAL; + return ti_error_einval(rs->ti, "Unable to assemble array: Invalid superblocks"); rdev_for_each(rdev, mddev) if ((rdev != freshest) && super_validate(rs, rdev)) @@ -1265,16 +1304,12 @@ static int raid_ctr(struct dm_target *ti, unsigned argc, char **argv) /* Must have */ arg = dm_shift_arg(&as); - if (!arg) { - ti->error = "No arguments"; - return -EINVAL; - } + if (!arg) + return ti_error_einval(rs->ti, "No arguments"); rt = get_raid_type(arg); - if (!rt) { - ti->error = "Unrecognised raid_type"; - return -EINVAL; - } + if (!rt) + return ti_error_einval(rs->ti, "Unrecognised raid_type"); /* Must have <#raid_params> */ if (dm_read_arg_group(_args, &as, &num_raid_params, &ti->error)) @@ -1287,10 +1322,8 @@ static int raid_ctr(struct dm_target *ti, unsigned argc, char **argv) if (dm_read_arg(_args + 1, &as_nrd, &num_raid_devs, &ti->error)) return -EINVAL; - if (!_in_range(num_raid_devs, 1, MAX_RAID_DEVICES)) { - ti->error = "Invalid number of supplied raid devices"; - return -EINVAL; - } + if (!_in_range(num_raid_devs, 1, MAX_RAID_DEVICES)) + return ti_error_einval(rs->ti, "Invalid number of supplied raid devices"); rs = context_alloc(ti, rt, num_raid_devs); if (IS_ERR(rs)) @@ -1300,7 +1333,7 @@ static int raid_ctr(struct dm_target *ti, unsigned argc, char **argv) if (r) goto bad; - r = parse_dev_parms(rs, &as); + r = parse_dev_params(rs, &as); if (r) goto bad; @@ -1330,8 +1363,7 @@ static int raid_ctr(struct dm_target *ti, unsigned argc, char **argv) } if (ti->len != rs->md.array_sectors) { - ti->error = "Array size does not match requested target length"; - r = -EINVAL; + r = ti_error_einval(ti, "Array size does not match requested target length"); goto size_mismatch; } rs->callbacks.congested_fn = raid_is_congested; @@ -1751,7 +1783,7 @@ static void raid_resume(struct dm_target *ti) static struct target_type raid_target = { .name = "raid", - .version = {1, 8, 0}, + .version = {1, 8, 1}, .module = THIS_MODULE, .ctr = raid_ctr, .dtr = raid_dtr, -- 2.34.1