dm raid: check constructor arguments for invalid raid level/argument combinations
authorHeinz Mauelshagen <heinzm@redhat.com>
Thu, 19 May 2016 16:49:27 +0000 (18:49 +0200)
committerMike Snitzer <snitzer@redhat.com>
Mon, 13 Jun 2016 18:40:27 +0000 (14:40 -0400)
Reject invalid flag combinations to avoid potential data corruption or
failing raid set construction:

 - add definitions for constructor flag combinations and invalid flags
   per level

 - add bool test functions for the various raid types
   (also will be used by future reshaping enhancements)

 - introduce rs_check_for_invalid_flags() and _invalid_flags()
   to perform the validity checks

Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
drivers/md/dm-raid.c

index ab7aa7d83364ca4496b6f14d3352fe94158850e8..ebb64eb66defdb8b7dd48e534638802a8f84c9fe 100644 (file)
@@ -64,6 +64,61 @@ struct raid_dev {
 #define CTR_FLAG_RAID10_COPIES     0x400 /* 2 */ /* Only with raid10 */
 #define CTR_FLAG_RAID10_FORMAT     0x800 /* 2 */ /* Only with raid10 */
 
+/*
+ * Definitions of various constructor flags to
+ * be used in checks of valid / invalid flags
+ * per raid level.
+ */
+/* Define all any sync flags */
+#define        CTR_FLAGS_ANY_SYNC              (CTR_FLAG_SYNC | CTR_FLAG_NOSYNC)
+
+/* Define flags for options without argument (e.g. 'nosync') */
+#define        CTR_FLAG_OPTIONS_NO_ARGS        CTR_FLAGS_ANY_SYNC
+
+/* Define flags for options with one argument (e.g. 'delta_disks +2') */
+#define CTR_FLAG_OPTIONS_ONE_ARG (CTR_FLAG_REBUILD | \
+                                 CTR_FLAG_WRITE_MOSTLY | \
+                                 CTR_FLAG_DAEMON_SLEEP | \
+                                 CTR_FLAG_MIN_RECOVERY_RATE | \
+                                 CTR_FLAG_MAX_RECOVERY_RATE | \
+                                 CTR_FLAG_MAX_WRITE_BEHIND | \
+                                 CTR_FLAG_STRIPE_CACHE | \
+                                 CTR_FLAG_REGION_SIZE | \
+                                 CTR_FLAG_RAID10_COPIES | \
+                                 CTR_FLAG_RAID10_FORMAT)
+
+/* All ctr optional arguments */
+#define ALL_CTR_FLAGS          (CTR_FLAG_OPTIONS_NO_ARGS | \
+                                CTR_FLAG_OPTIONS_ONE_ARG)
+
+/* Invalid options definitions per raid level... */
+
+/* "raid0" does not accept any options */
+#define RAID0_INVALID_FLAGS ALL_CTR_FLAGS
+
+/* "raid1" does not accept stripe cache or any raid10 options */
+#define RAID1_INVALID_FLAGS    (CTR_FLAG_STRIPE_CACHE | \
+                                CTR_FLAG_RAID10_COPIES | \
+                                CTR_FLAG_RAID10_FORMAT)
+
+/* "raid10" does not accept any raid1 or stripe cache options */
+#define RAID10_INVALID_FLAGS   (CTR_FLAG_WRITE_MOSTLY | \
+                                CTR_FLAG_MAX_WRITE_BEHIND | \
+                                CTR_FLAG_STRIPE_CACHE)
+/*
+ * "raid4/5/6" do not accept any raid1 or raid10 specific options
+ *
+ * "raid6" does not accept "nosync", because it is not guaranteed
+ * that both parity and q-syndrome are being written properly with
+ * any writes
+ */
+#define RAID45_INVALID_FLAGS   (CTR_FLAG_WRITE_MOSTLY | \
+                                CTR_FLAG_MAX_WRITE_BEHIND | \
+                                CTR_FLAG_RAID10_FORMAT | \
+                                CTR_FLAG_RAID10_COPIES)
+#define RAID6_INVALID_FLAGS    (CTR_FLAG_NOSYNC | RAID45_INVALID_FLAGS)
+/* ...invalid options definitions per raid level */
+
 struct raid_set {
        struct dm_target *ti;
 
@@ -166,6 +221,41 @@ static const char *_argname_by_flag(const uint32_t flag)
        return NULL;
 }
 
+/*
+ * bool helpers to test for various raid levels of a raid type
+ */
+
+/* Return true, if raid type in @rt is raid0 */
+static bool rt_is_raid0(struct raid_type *rt)
+{
+       return !rt->level;
+}
+
+/* Return true, if raid type in @rt is raid1 */
+static bool rt_is_raid1(struct raid_type *rt)
+{
+       return rt->level == 1;
+}
+
+/* Return true, if raid type in @rt is raid10 */
+static bool rt_is_raid10(struct raid_type *rt)
+{
+       return rt->level == 10;
+}
+
+/* Return true, if raid type in @rt is raid4/5 */
+static bool rt_is_raid45(struct raid_type *rt)
+{
+       return _in_range(rt->level, 4, 5);
+}
+
+/* Return true, if raid type in @rt is raid6 */
+static bool rt_is_raid6(struct raid_type *rt)
+{
+       return rt->level == 6;
+}
+/* END: raid level bools */
+
 /*
  * Convenience functions to set ti->error to @errmsg and
  * return @r in order to shorten code in a lot of places
@@ -182,6 +272,44 @@ static int ti_error_einval(struct dm_target *ti, const char *errmsg)
 }
 /* END: convenience functions to set ti->error to @errmsg... */
 
+/* Return invalid ctr flags for the raid level of @rs */
+static uint32_t _invalid_flags(struct raid_set *rs)
+{
+       if (rt_is_raid0(rs->raid_type))
+               return RAID0_INVALID_FLAGS;
+       else if (rt_is_raid1(rs->raid_type))
+               return RAID1_INVALID_FLAGS;
+       else if (rt_is_raid10(rs->raid_type))
+               return RAID10_INVALID_FLAGS;
+       else if (rt_is_raid45(rs->raid_type))
+               return RAID45_INVALID_FLAGS;
+       else if (rt_is_raid6(rs->raid_type))
+               return RAID6_INVALID_FLAGS;
+
+       return ~0;
+}
+
+/*
+ * Check for any invalid flags set on @rs defined by bitset @invalid_flags
+ *
+ * Has to be called after parsing of the ctr flags!
+ */
+static int rs_check_for_invalid_flags(struct raid_set *rs)
+{
+       unsigned int ctr_flags = rs->ctr_flags, flag = 0;
+       const uint32_t invalid_flags = _invalid_flags(rs);
+
+       while ((ctr_flags &= ~flag)) {
+               flag = 1 << __ffs(ctr_flags);
+
+               if (_test_flag(flag, rs->ctr_flags) &&
+                   _test_flag(flag, invalid_flags))
+                       return ti_error_einval(rs->ti, "Invalid flag combined");
+       }
+
+       return 0;
+}
+
 static char *raid10_md_layout_to_format(int layout)
 {
        /*
@@ -806,7 +934,8 @@ static int parse_raid_params(struct raid_set *rs, struct dm_arg_set *as,
        rs->md.persistent = 0;
        rs->md.external = 1;
 
-       return 0;
+       /* Check, if any invalid ctr arguments have been passed in for the raid level */
+       return rs_check_for_invalid_flags(rs);
 }
 
 static void do_table_event(struct work_struct *ws)