module/raid: make raid_level an enum instead of plain integer

Also add functions to convert it to/from string.

Signed-off-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
Change-Id: I6f7964d832c308b815150fff39eb3dc5c5ae5853
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471079
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom SPDK FC-NVMe CI <spdk-ci.pdl@broadcom.com>

module/bdev/raid/bdev_raid.c

@@ -580,7 +580,7 @@ raid_bdev_dump_info_json(void *ctx, struct spdk_json_write_ctx *w)
 	spdk_json_write_named_uint32(w, "strip_size", raid_bdev->strip_size);
 	spdk_json_write_named_uint32(w, "strip_size_kb", raid_bdev->strip_size_kb);
 	spdk_json_write_named_uint32(w, "state", raid_bdev->state);
-	spdk_json_write_named_uint32(w, "raid_level", raid_bdev->raid_level);
+	spdk_json_write_named_string(w, "raid_level", raid_bdev_level_to_str(raid_bdev->level));
 	spdk_json_write_named_uint32(w, "destruct_called", raid_bdev->destruct_called);
 	spdk_json_write_named_uint32(w, "num_base_bdevs", raid_bdev->num_base_bdevs);
 	spdk_json_write_named_uint32(w, "num_base_bdevs_discovered", raid_bdev->num_base_bdevs_discovered);
@@ -622,7 +622,7 @@ raid_bdev_write_config_json(struct spdk_bdev *bdev, struct spdk_json_write_ctx *
 	spdk_json_write_named_object_begin(w, "params");
 	spdk_json_write_named_string(w, "name", bdev->name);
 	spdk_json_write_named_uint32(w, "strip_size", raid_bdev->strip_size_kb);
-	spdk_json_write_named_uint32(w, "raid_level", raid_bdev->raid_level);
+	spdk_json_write_named_string(w, "raid_level", raid_bdev_level_to_str(raid_bdev->level));
 
 	spdk_json_write_named_array_begin(w, "base_bdevs");
 	for (i = 0; i < raid_bdev->num_base_bdevs; i++) {
@@ -722,12 +722,12 @@ raid_bdev_config_find_by_name(const char *raid_name)
  * raid_name - name for raid bdev.
  * strip_size - strip size in KB
  * num_base_bdevs - number of base bdevs.
- * raid_level - raid level, only raid level 0 is supported.
+ * level - raid level, only raid level 0 is supported.
  * _raid_cfg - Pointer to newly added configuration
  */
 int
 raid_bdev_config_add(const char *raid_name, uint32_t strip_size, uint8_t num_base_bdevs,
-		     uint8_t raid_level, struct raid_bdev_config **_raid_cfg)
+		     enum raid_level level, struct raid_bdev_config **_raid_cfg)
 {
 	struct raid_bdev_config *raid_cfg;
 
@@ -748,9 +748,9 @@ raid_bdev_config_add(const char *raid_name, uint32_t strip_size, uint8_t num_bas
 		return -EINVAL;
 	}
 
-	if (raid_level != RAID0) {
+	if (level != RAID0) {
 		SPDK_ERRLOG("invalid raid level %u, only raid level 0 is supported\n",
-			    raid_level);
+			    level);
 		return -EINVAL;
 	}
 
@@ -768,7 +768,7 @@ raid_bdev_config_add(const char *raid_name, uint32_t strip_size, uint8_t num_bas
 	}
 	raid_cfg->strip_size = strip_size;
 	raid_cfg->num_base_bdevs = num_base_bdevs;
-	raid_cfg->raid_level = raid_level;
+	raid_cfg->level = level;
 
 	raid_cfg->base_bdev = calloc(num_base_bdevs, sizeof(*raid_cfg->base_bdev));
 	if (raid_cfg->base_bdev == NULL) {
@@ -825,6 +825,43 @@ raid_bdev_config_add_base_bdev(struct raid_bdev_config *raid_cfg, const char *ba
 
 	return 0;
 }
+
+static struct {
+	const char *name;
+	enum raid_level value;
+} g_raid_level_names[] = {
+	{ "raid0", RAID0 },
+	{ "0", RAID0 },
+	{ }
+};
+
+enum raid_level raid_bdev_parse_raid_level(const char *str)
+{
+	unsigned int i;
+
+	for (i = 0; g_raid_level_names[i].name != NULL; i++) {
+		if (strcasecmp(g_raid_level_names[i].name, str) == 0) {
+			return g_raid_level_names[i].value;
+		}
+	}
+
+	return INVALID_RAID_LEVEL;
+}
+
+const char *
+raid_bdev_level_to_str(enum raid_level level)
+{
+	unsigned int i;
+
+	for (i = 0; g_raid_level_names[i].name != NULL; i++) {
+		if (g_raid_level_names[i].value == level) {
+			return g_raid_level_names[i].name;
+		}
+	}
+
+	return "";
+}
+
 /*
  * brief:
  * raid_bdev_parse_raid is used to parse the raid bdev from config file based on
@@ -855,7 +892,9 @@ raid_bdev_parse_raid(struct spdk_conf_section *conf_section)
 {
 	const char *raid_name;
 	uint32_t strip_size;
-	uint8_t num_base_bdevs, raid_level;
+	uint8_t num_base_bdevs;
+	const char *raid_level_str;
+	enum raid_level level;
 	const char *base_bdev_name;
 	struct raid_bdev_config *raid_cfg;
 	int rc, i, val;
@@ -878,16 +917,21 @@ raid_bdev_parse_raid(struct spdk_conf_section *conf_section)
 	}
 	num_base_bdevs = val;
 
-	val = spdk_conf_section_get_intval(conf_section, "RaidLevel");
+	raid_level_str = spdk_conf_section_get_val(conf_section, "RaidLevel");
-	if (val < 0) {
+	if (raid_level_str == NULL) {
+		SPDK_ERRLOG("Missing RaidLevel\n");
+		return -EINVAL;
+	}
+	level = raid_bdev_parse_raid_level(raid_level_str);
+	if (level == INVALID_RAID_LEVEL) {
+		SPDK_ERRLOG("Invalid RaidLevel\n");
 		return -EINVAL;
 	}
-	raid_level = val;
 
 	SPDK_DEBUGLOG(SPDK_LOG_BDEV_RAID, "%s %" PRIu32 " %u %u\n",
-		      raid_name, strip_size, num_base_bdevs, raid_level);
+		      raid_name, strip_size, num_base_bdevs, level);
 
-	rc = raid_bdev_config_add(raid_name, strip_size, num_base_bdevs, raid_level,
+	rc = raid_bdev_config_add(raid_name, strip_size, num_base_bdevs, level,
 				  &raid_cfg);
 	if (rc != 0) {
 		SPDK_ERRLOG("Failed to add raid bdev config\n");
@@ -1037,9 +1081,10 @@ raid_bdev_get_running_config(FILE *fp)
 			"  Name %s\n"
 			"  StripSize %" PRIu32 "\n"
 			"  NumDevices %u\n"
-			"  RaidLevel %hhu\n",
+			"  RaidLevel %s\n",
 			index, raid_bdev->bdev.name, raid_bdev->strip_size_kb,
-			raid_bdev->num_base_bdevs, raid_bdev->raid_level);
+			raid_bdev->num_base_bdevs,
+			raid_bdev_level_to_str(raid_bdev->level));
 		fprintf(fp,
 			"  Devices ");
 		for (i = 0; i < raid_bdev->num_base_bdevs; i++) {
@@ -1173,13 +1218,13 @@ raid_bdev_create(struct raid_bdev_config *raid_cfg)
 	raid_bdev->strip_size_kb = raid_cfg->strip_size;
 	raid_bdev->state = RAID_BDEV_STATE_CONFIGURING;
 	raid_bdev->config = raid_cfg;
-	raid_bdev->raid_level = raid_cfg->raid_level;
+	raid_bdev->level = raid_cfg->level;
 
-	switch (raid_bdev->raid_level) {
+	switch (raid_bdev->level) {
 	case RAID0:
 		break;
 	default:
-		SPDK_ERRLOG("invalid raid level %u\n", raid_bdev->raid_level);
+		SPDK_ERRLOG("invalid raid level %u\n", raid_bdev->level);
 		free(raid_bdev);
 		return -EINVAL;
 	}

module/bdev/raid/bdev_raid.h

@@ -36,7 +36,10 @@
 
 #include "spdk/bdev_module.h"
 
-#define RAID0 0
+enum raid_level {
+	INVALID_RAID_LEVEL	= -1,
+	RAID0			= 0,
+};
 
 /*
  * Raid state describes the state of the raid. This raid bdev can be either in
@@ -143,7 +146,7 @@ struct raid_bdev {
 	uint8_t				num_base_bdevs_discovered;
 
 	/* Raid Level of this raid bdev */
-	uint8_t				raid_level;
+	enum raid_level			level;
 
 	/* Set to true if destruct is called for this raid bdev */
 	bool				destruct_called;
@@ -181,7 +184,7 @@ struct raid_bdev_config {
 	uint8_t				num_base_bdevs;
 
 	/* raid level */
-	uint8_t				raid_level;
+	enum raid_level			level;
 
 	TAILQ_ENTRY(raid_bdev_config)	link;
 };
@@ -229,11 +232,13 @@ int raid_bdev_add_base_devices(struct raid_bdev_config *raid_cfg);
 void raid_bdev_remove_base_devices(struct raid_bdev_config *raid_cfg,
 				   raid_bdev_destruct_cb cb_fn, void *cb_ctx);
 int raid_bdev_config_add(const char *raid_name, uint32_t strip_size, uint8_t num_base_bdevs,
-			 uint8_t raid_level, struct raid_bdev_config **_raid_cfg);
+			 enum raid_level level, struct raid_bdev_config **_raid_cfg);
 int raid_bdev_config_add_base_bdev(struct raid_bdev_config *raid_cfg,
 				   const char *base_bdev_name, uint8_t slot);
 void raid_bdev_config_cleanup(struct raid_bdev_config *raid_cfg);
 struct raid_bdev_config *raid_bdev_config_find_by_name(const char *raid_name);
+enum raid_level raid_bdev_parse_raid_level(const char *str);
+const char *raid_bdev_level_to_str(enum raid_level level);
 
 void
 raid0_start_rw_request(struct spdk_io_channel *ch, struct spdk_bdev_io *bdev_io);

module/bdev/raid/bdev_raid_rpc.c

@@ -165,7 +165,7 @@ struct rpc_bdev_raid_create {
 	uint32_t                             strip_size_kb;
 
 	/* RAID raid level */
-	uint8_t                              raid_level;
+	enum raid_level                      level;
 
 	/* Base bdevs information */
 	struct rpc_bdev_raid_create_base_bdevs base_bdevs;
@@ -188,6 +188,30 @@ free_rpc_bdev_raid_create(struct rpc_bdev_raid_create *req)
 	}
 }
 
+/*
+ * Decoder function for RPC bdev_raid_create to decode raid level
+ */
+static int
+decode_raid_level(const struct spdk_json_val *val, void *out)
+{
+	int ret;
+	char *str = NULL;
+	enum raid_level level;
+
+	ret = spdk_json_decode_string(val, &str);
+	if (ret == 0) {
+		level = raid_bdev_parse_raid_level(str);
+		if (level == INVALID_RAID_LEVEL) {
+			ret = -EINVAL;
+		} else {
+			*(enum raid_level *)out = level;
+		}
+	}
+
+	free(str);
+	return ret;
+}
+
 /*
  * Decoder function for RPC bdev_raid_create to decode base bdevs list
  */
@@ -207,7 +231,7 @@ static const struct spdk_json_object_decoder rpc_bdev_raid_create_decoders[] = {
 	{"name", offsetof(struct rpc_bdev_raid_create, name), spdk_json_decode_string},
 	{"strip_size", offsetof(struct rpc_bdev_raid_create, strip_size), spdk_json_decode_uint32, true},
 	{"strip_size_kb", offsetof(struct rpc_bdev_raid_create, strip_size_kb), spdk_json_decode_uint32, true},
-	{"raid_level", offsetof(struct rpc_bdev_raid_create, raid_level), spdk_json_decode_uint32},
+	{"raid_level", offsetof(struct rpc_bdev_raid_create, level), decode_raid_level},
 	{"base_bdevs", offsetof(struct rpc_bdev_raid_create, base_bdevs), decode_base_bdevs},
 };
 
@@ -251,7 +275,7 @@ spdk_rpc_bdev_raid_create(struct spdk_jsonrpc_request *request,
 	}
 
 	rc = raid_bdev_config_add(req.name, req.strip_size_kb, req.base_bdevs.num_base_bdevs,
-				  req.raid_level,
+				  req.level,
 				  &raid_cfg);
 	if (rc != 0) {
 		spdk_jsonrpc_send_error_response_fmt(request, rc,

scripts/rpc.py

@@ -1448,7 +1448,7 @@ Format: 'user:u1 secret:s1 muser:mu1 msecret:ms1,user:u2 secret:s2 muser:mu2 mse
     p.add_argument('-n', '--name', help='raid bdev name', required=True)
     p.add_argument('-s', '--strip-size', help='strip size in KB (deprecated)', type=int)
     p.add_argument('-z', '--strip-size_kb', help='strip size in KB', type=int)
-    p.add_argument('-r', '--raid-level', help='raid level, only raid level 0 is supported', type=int, required=True)
+    p.add_argument('-r', '--raid-level', help='raid level, only raid level 0 is supported', required=True)
     p.add_argument('-b', '--base-bdevs', help='base bdevs name, whitespace separated list in quotes', required=True)
     p.set_defaults(func=bdev_raid_create)
 

scripts/spdkcli/ui_node.py

@@ -843,7 +843,6 @@ class UIRaidBdev(UIBdev):
         for u in base_bdevs.strip().split(" "):
             base_bdevs_array.append(u)
 
-        raid_level = self.ui_eval_param(raid_level, "number", None)
         strip_size_kb = self.ui_eval_param(strip_size_kb, "number", None)
 
         ret_name = self.get_root().bdev_raid_create(name=name,

test/unit/lib/bdev/bdev_raid.c/bdev_raid_ut.c

@@ -124,8 +124,6 @@ DEFINE_STUB(spdk_json_decode_array, int, (const struct spdk_json_val *values,
 		spdk_json_decode_fn decode_func,
 		void *out, size_t max_size, size_t *out_size, size_t stride), 0);
 DEFINE_STUB(spdk_json_write_name, int, (struct spdk_json_write_ctx *w, const char *name), 0);
-DEFINE_STUB(spdk_json_write_named_string, int, (struct spdk_json_write_ctx *w,
-		const char *name, const char *val), 0);
 DEFINE_STUB(spdk_json_write_object_begin, int, (struct spdk_json_write_ctx *w), 0);
 DEFINE_STUB(spdk_json_write_named_object_begin, int, (struct spdk_json_write_ctx *w,
 		const char *name), 0);
@@ -387,8 +385,6 @@ int spdk_json_write_named_uint32(struct spdk_json_write_ctx *w, const char *name
 		CU_ASSERT(req->strip_size_kb == val);
 	} else if (strcmp(name, "blocklen_shift") == 0) {
 		CU_ASSERT(spdk_u32log2(g_block_len) == val);
-	} else if (strcmp(name, "raid_level") == RAID0) {
-		CU_ASSERT(req->raid_level == val);
 	} else if (strcmp(name, "num_base_bdevs") == 0) {
 		CU_ASSERT(req->base_bdevs.num_base_bdevs == val);
 	} else if (strcmp(name, "state") == 0) {
@@ -401,6 +397,15 @@ int spdk_json_write_named_uint32(struct spdk_json_write_ctx *w, const char *name
 	return 0;
 }
 
+int spdk_json_write_named_string(struct spdk_json_write_ctx *w, const char *name, const char *val)
+{
+	struct rpc_bdev_raid_create *req = g_rpc_req;
+	if (strcmp(name, "raid_level") == 0) {
+		CU_ASSERT(strcmp(val, raid_bdev_level_to_str(req->level)) == 0);
+	}
+	return 0;
+}
+
 void
 spdk_for_each_thread(spdk_msg_fn fn, void *ctx, spdk_msg_fn cpl)
 {
@@ -485,6 +490,8 @@ spdk_conf_section_get_val(struct spdk_conf_section *sp, const char *key)
 	if (g_config_level_create) {
 		if (strcmp(key, "Name") == 0) {
 			return req->name;
+		} else if (strcmp(key, "RaidLevel") == 0) {
+			return (char *)raid_bdev_level_to_str(req->level);
 		}
 	}
 
@@ -501,8 +508,6 @@ spdk_conf_section_get_intval(struct spdk_conf_section *sp, const char *key)
 			return req->strip_size_kb;
 		} else if (strcmp(key, "NumDevices") == 0) {
 			return req->base_bdevs.num_base_bdevs;
-		} else if (strcmp(key, "RaidLevel") == 0) {
-			return req->raid_level;
 		}
 	}
 
@@ -554,7 +559,7 @@ spdk_json_decode_object(const struct spdk_json_val *values,
 		_out->name = strdup(req->name);
 		SPDK_CU_ASSERT_FATAL(_out->name != NULL);
 		_out->strip_size_kb = req->strip_size_kb;
-		_out->raid_level = req->raid_level;
+		_out->level = req->level;
 		_out->base_bdevs.num_base_bdevs = req->base_bdevs.num_base_bdevs;
 		for (i = 0; i < req->base_bdevs.num_base_bdevs; i++) {
 			_out->base_bdevs.base_bdevs[i] = strdup(req->base_bdevs.base_bdevs[i]);
@@ -901,7 +906,7 @@ verify_raid_config(struct rpc_bdev_raid_create *r, bool presence)
 			CU_ASSERT(raid_cfg->raid_bdev != NULL);
 			CU_ASSERT(raid_cfg->strip_size == r->strip_size_kb);
 			CU_ASSERT(raid_cfg->num_base_bdevs == r->base_bdevs.num_base_bdevs);
-			CU_ASSERT(raid_cfg->raid_level == r->raid_level);
+			CU_ASSERT(raid_cfg->level == r->level);
 			if (raid_cfg->base_bdev != NULL) {
 				for (i = 0; i < raid_cfg->num_base_bdevs; i++) {
 					val = strcmp(raid_cfg->base_bdev[i].name,
@@ -945,7 +950,7 @@ verify_raid_bdev(struct rpc_bdev_raid_create *r, bool presence, uint32_t raid_st
 			CU_ASSERT(pbdev->state == raid_state);
 			CU_ASSERT(pbdev->num_base_bdevs == r->base_bdevs.num_base_bdevs);
 			CU_ASSERT(pbdev->num_base_bdevs_discovered == r->base_bdevs.num_base_bdevs);
-			CU_ASSERT(pbdev->raid_level == r->raid_level);
+			CU_ASSERT(pbdev->level == r->level);
 			CU_ASSERT(pbdev->destruct_called == false);
 			for (i = 0; i < pbdev->num_base_bdevs; i++) {
 				if (pbdev->base_bdev_info && pbdev->base_bdev_info[i].bdev) {
@@ -1068,7 +1073,7 @@ create_test_req(struct rpc_bdev_raid_create *r, const char *raid_name,
 	r->name = strdup(raid_name);
 	SPDK_CU_ASSERT_FATAL(r->name != NULL);
 	r->strip_size_kb = (g_strip_size * g_block_len) / 1024;
-	r->raid_level = RAID0;
+	r->level = RAID0;
 	r->base_bdevs.num_base_bdevs = g_max_base_drives;
 	for (i = 0; i < g_max_base_drives; i++, bbdev_idx++) {
 		snprintf(name, 16, "%s%u%s", "Nvme", bbdev_idx, "n1");
@@ -1218,7 +1223,7 @@ test_create_raid_invalid_args(void)
 	verify_raid_config_present("raid1", false);
 	verify_raid_bdev_present("raid1", false);
 	create_raid_bdev_create_req(&req, "raid1", 0, true, 0);
-	req.raid_level = 1;
+	req.level = INVALID_RAID_LEVEL;
 	spdk_rpc_bdev_raid_create(NULL, NULL);
 	CU_ASSERT(g_rpc_err == 1);
 	free_test_req(&req);
@@ -2120,14 +2125,14 @@ test_create_raid_from_config_invalid_params(void)
 	verify_raid_bdev_present("raid1", false);
 
 	create_raid_bdev_create_config(&req, "raid1", 0, false);
-	req.raid_level = 1;
+	req.level = INVALID_RAID_LEVEL;
 	CU_ASSERT(raid_bdev_init() != 0);
 	free_test_req(&req);
 	verify_raid_config_present("raid1", false);
 	verify_raid_bdev_present("raid1", false);
 
 	create_raid_bdev_create_config(&req, "raid1", 0, false);
-	req.raid_level = 1;
+	req.level = INVALID_RAID_LEVEL;
 	CU_ASSERT(raid_bdev_init() != 0);
 	free_test_req(&req);
 	verify_raid_config_present("raid1", false);
@@ -2208,6 +2213,24 @@ test_context_size(void)
 	CU_ASSERT(raid_bdev_get_ctx_size() == sizeof(struct raid_bdev_io));
 }
 
+static void
+test_raid_level_conversions(void)
+{
+	const char *raid_str;
+
+	CU_ASSERT(raid_bdev_parse_raid_level("abcd123") == INVALID_RAID_LEVEL);
+	CU_ASSERT(raid_bdev_parse_raid_level("0") == RAID0);
+	CU_ASSERT(raid_bdev_parse_raid_level("raid0") == RAID0);
+	CU_ASSERT(raid_bdev_parse_raid_level("RAID0") == RAID0);
+
+	raid_str = raid_bdev_level_to_str(INVALID_RAID_LEVEL);
+	CU_ASSERT(raid_str != NULL && strlen(raid_str) == 0);
+	raid_str = raid_bdev_level_to_str(1234);
+	CU_ASSERT(raid_str != NULL && strlen(raid_str) == 0);
+	raid_str = raid_bdev_level_to_str(RAID0);
+	CU_ASSERT(raid_str != NULL && strcmp(raid_str, "raid0") == 0);
+}
+
 int main(int argc, char **argv)
 {
 	CU_pSuite       suite = NULL;
@@ -2243,7 +2266,8 @@ int main(int argc, char **argv)
 		CU_add_test(suite, "test_create_raid_from_config_invalid_params",
 			    test_create_raid_from_config_invalid_params) == NULL ||
 		CU_add_test(suite, "test_raid_json_dump_info", test_raid_json_dump_info) == NULL ||
-		CU_add_test(suite, "test_context_size", test_context_size) == NULL
+		CU_add_test(suite, "test_context_size", test_context_size) == NULL ||
+		CU_add_test(suite, "test_raid_level_conversions", test_raid_level_conversions) == NULL
 	) {
 		CU_cleanup_registry();
 		return CU_get_error();