bdev/raid: Factor out delete raid bdev config operation

Change-Id: I71f98a7cfd49f391ce9880c42f53072c6264ef1d
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/420326
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Kunal Sablok <kunal.sablok@intel.com>

lib/bdev/raid/bdev_raid.c

@@ -773,21 +773,17 @@ static const struct spdk_bdev_fn_table g_raid_bdev_fn_table = {
 
 /*
  * brief:
- * raid_bdev_free is the raid bdev function table function pointer. This is
+ * raid_bdev_config_cleanup function is used to free memory for one raid_bdev in configuration
- * called on bdev free path
  * params:
- * none
+ * raid_bdev_config - pointer to raid_bdev_config structure
  * returns:
  * none
  */
-static void
+void
-raid_bdev_free(void)
+raid_bdev_config_cleanup(struct raid_bdev_config *raid_cfg)
 {
-	struct raid_bdev_config *raid_cfg, *tmp;
 	uint32_t i;
 
-	SPDK_DEBUGLOG(SPDK_LOG_BDEV_RAID, "raid_bdev_free\n");
-	TAILQ_FOREACH_SAFE(raid_cfg, &g_spdk_raid_config.raid_bdev_config_head, link, tmp) {
 	TAILQ_REMOVE(&g_spdk_raid_config.raid_bdev_config_head, raid_cfg, link);
 	g_spdk_raid_config.total_raid_bdev--;
 
@@ -799,6 +795,25 @@ raid_bdev_free(void)
 	}
 	free(raid_cfg->name);
 	free(raid_cfg);
+}
+
+/*
+ * brief:
+ * raid_bdev_free is the raid bdev function table function pointer. This is
+ * called on bdev free path
+ * params:
+ * none
+ * returns:
+ * none
+ */
+static void
+raid_bdev_free(void)
+{
+	struct raid_bdev_config *raid_cfg, *tmp;
+
+	SPDK_DEBUGLOG(SPDK_LOG_BDEV_RAID, "raid_bdev_free\n");
+	TAILQ_FOREACH_SAFE(raid_cfg, &g_spdk_raid_config.raid_bdev_config_head, link, tmp) {
+		raid_bdev_config_cleanup(raid_cfg);
 	}
 }
 
@@ -837,7 +852,6 @@ raid_bdev_parse_raid(struct spdk_conf_section *conf_section)
 	const char *base_bdev_name;
 	uint32_t i, j;
 	struct raid_bdev_config *raid_bdev_config, *tmp;
-	int rc = -1;
 
 	raid_name = spdk_conf_section_get_val(conf_section, "Name");
 	if (raid_name == NULL) {
@@ -890,9 +904,9 @@ raid_bdev_parse_raid(struct spdk_conf_section *conf_section)
 
 	raid_bdev_config->base_bdev = calloc(num_base_bdevs, sizeof(*raid_bdev_config->base_bdev));
 	if (raid_bdev_config->base_bdev == NULL) {
+		raid_bdev_config_cleanup(raid_bdev_config);
 		SPDK_ERRLOG("unable to allocate memory\n");
-		rc = -ENOMEM;
+		return -ENOMEM;
-		goto error;
 	}
 
 	for (i = 0; true; i++) {
@@ -901,19 +915,19 @@ raid_bdev_parse_raid(struct spdk_conf_section *conf_section)
 			break;
 		}
 		if (i >= raid_bdev_config->num_base_bdevs) {
+			raid_bdev_config_cleanup(raid_bdev_config);
 			SPDK_ERRLOG("Number of devices mentioned is more than count\n");
-			rc = -1;
+			return -1;
-			goto error;
 		}
 
 		TAILQ_FOREACH(tmp, &g_spdk_raid_config.raid_bdev_config_head, link) {
 			for (j = 0; j < tmp->num_base_bdevs; j++) {
 				if (tmp->base_bdev[j].bdev_name != NULL) {
 					if (!strcmp(tmp->base_bdev[j].bdev_name, base_bdev_name)) {
+						raid_bdev_config_cleanup(raid_bdev_config);
 						SPDK_ERRLOG("duplicate base bdev name %s mentioned\n",
 							    base_bdev_name);
-						rc = -EEXIST;
+						return -EEXIST;
-						goto error;
 					}
 				}
 			}
@@ -921,32 +935,19 @@ raid_bdev_parse_raid(struct spdk_conf_section *conf_section)
 
 		raid_bdev_config->base_bdev[i].bdev_name = strdup(base_bdev_name);
 		if (raid_bdev_config->base_bdev[i].bdev_name == NULL) {
+			raid_bdev_config_cleanup(raid_bdev_config);
 			SPDK_ERRLOG("unable to allocate memory\n");
-			rc = -ENOMEM;
+			return -ENOMEM;
-			goto error;
 		}
 	}
 
 	if (i != raid_bdev_config->num_base_bdevs) {
+		raid_bdev_config_cleanup(raid_bdev_config);
 		SPDK_ERRLOG("Number of devices mentioned is less than count\n");
-		rc = -1;
+		return -1;
-		goto error;
 	}
 
 	return 0;
-
-error:
-	g_spdk_raid_config.total_raid_bdev--;
-	TAILQ_REMOVE(&g_spdk_raid_config.raid_bdev_config_head, raid_bdev_config, link);
-	if (raid_bdev_config->base_bdev) {
-		for (i = 0; i < raid_bdev_config->num_base_bdevs; i++) {
-			free(raid_bdev_config->base_bdev[i].bdev_name);
-		}
-		free(raid_bdev_config->base_bdev);
-	}
-	free(raid_bdev_config->name);
-	free(raid_bdev_config);
-	return rc;
 }
 
 /*

lib/bdev/raid/bdev_raid.h

@@ -228,5 +228,6 @@ extern struct raid_config                   g_spdk_raid_config;
 
 void raid_bdev_remove_base_bdev(void *ctx);
 int raid_bdev_add_base_device(struct spdk_bdev *bdev);
+void raid_bdev_config_cleanup(struct raid_bdev_config *raid_cfg);
 
 #endif // SPDK_BDEV_RAID_INTERNAL_H

lib/bdev/raid/bdev_raid_rpc.c

@@ -244,32 +244,6 @@ static const struct spdk_json_object_decoder rpc_construct_raid_bdev_decoders[]
 	{"base_bdevs", offsetof(struct rpc_construct_raid_bdev, base_bdevs), decode_base_bdevs},
 };
 
-/*
- * brief:
- * raid_bdev_config_cleanup function is used to free memory for one raid_bdev in configuration
- * params:
- * raid_bdev_config - pointer to raid_bdev_config structure
- * returns:
- * none
- */
-static void
-raid_bdev_config_cleanup(struct raid_bdev_config *raid_cfg)
-{
-	uint32_t i;
-
-	TAILQ_REMOVE(&g_spdk_raid_config.raid_bdev_config_head, raid_cfg, link);
-	g_spdk_raid_config.total_raid_bdev--;
-
-	if (raid_cfg->base_bdev) {
-		for (i = 0; i < raid_cfg->num_base_bdevs; i++) {
-			free(raid_cfg->base_bdev[i].bdev_name);
-		}
-		free(raid_cfg->base_bdev);
-	}
-	free(raid_cfg->name);
-	free(raid_cfg);
-}
-
 /*
  * brief:
  * check_and_remove_raid_bdev function free base bdev descriptors, unclaim the base
@@ -516,8 +490,6 @@ raid_bdev_config_destroy_check_raid_bdev_exists(void *arg)
 static void
 raid_bdev_config_destroy(struct raid_bdev_config *raid_cfg)
 {
-	uint8_t i;
-
 	assert(raid_cfg != NULL);
 	if (raid_cfg->raid_bdev_ctxt != NULL) {
 		/*
@@ -528,16 +500,7 @@ raid_bdev_config_destroy(struct raid_bdev_config *raid_cfg)
 		return;
 	}
 
-	TAILQ_REMOVE(&g_spdk_raid_config.raid_bdev_config_head, raid_cfg, link);
+	raid_bdev_config_cleanup(raid_cfg);
-	g_spdk_raid_config.total_raid_bdev--;
-
-	/* Destroy raid bdev config and cleanup */
-	for (i = 0; i < raid_cfg->num_base_bdevs; i++) {
-		free(raid_cfg->base_bdev[i].bdev_name);
-	}
-	free(raid_cfg->base_bdev);
-	free(raid_cfg->name);
-	free(raid_cfg);
 }
 
 /*