diff --git a/lib/bdev/raid/bdev_raid.c b/lib/bdev/raid/bdev_raid.c index abc4710d6..871be970e 100644 --- a/lib/bdev/raid/bdev_raid.c +++ b/lib/bdev/raid/bdev_raid.c @@ -42,7 +42,9 @@ #include "spdk/string.h" /* raid bdev config as read from config file */ -struct raid_config g_spdk_raid_config; +struct raid_config g_spdk_raid_config = { + .raid_bdev_config_head = TAILQ_HEAD_INITIALIZER(g_spdk_raid_config.raid_bdev_config_head), +}; /* * List of raid bdev in configured list, these raid bdevs are registered with @@ -781,25 +783,22 @@ static const struct spdk_bdev_fn_table g_raid_bdev_fn_table = { static void raid_bdev_free(void) { + struct raid_bdev_config *raid_cfg, *tmp; + uint32_t i; + SPDK_DEBUGLOG(SPDK_LOG_BDEV_RAID, "raid_bdev_free\n"); - for (uint32_t raid_bdev = 0; raid_bdev < g_spdk_raid_config.total_raid_bdev; raid_bdev++) { - if (g_spdk_raid_config.raid_bdev_config[raid_bdev].base_bdev) { - for (uint32_t i = 0; i < g_spdk_raid_config.raid_bdev_config[raid_bdev].num_base_bdevs; - i++) { - free(g_spdk_raid_config.raid_bdev_config[raid_bdev].base_bdev[i].bdev_name); + 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--; + + if (raid_cfg->base_bdev) { + for (i = 0; i < raid_cfg->num_base_bdevs; i++) { + free(raid_cfg->base_bdev[i].bdev_name); } - free(g_spdk_raid_config.raid_bdev_config[raid_bdev].base_bdev); - g_spdk_raid_config.raid_bdev_config[raid_bdev].base_bdev = NULL; + free(raid_cfg->base_bdev); } - free(g_spdk_raid_config.raid_bdev_config[raid_bdev].name); - } - if (g_spdk_raid_config.raid_bdev_config) { - if (g_spdk_raid_config.raid_bdev_config->raid_bdev_ctxt) { - g_spdk_raid_config.raid_bdev_config->raid_bdev_ctxt->raid_bdev.raid_bdev_config = NULL; - } - free(g_spdk_raid_config.raid_bdev_config); - g_spdk_raid_config.raid_bdev_config = NULL; - g_spdk_raid_config.total_raid_bdev = 0; + free(raid_cfg->name); + free(raid_cfg); } } @@ -836,9 +835,9 @@ raid_bdev_parse_raid(struct spdk_conf_section *conf_section) int num_base_bdevs; int raid_level; const char *base_bdev_name; - uint32_t i; - void *temp_ptr; - struct raid_bdev_config *raid_bdev_config; + 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) { @@ -864,35 +863,36 @@ raid_bdev_parse_raid(struct spdk_conf_section *conf_section) SPDK_DEBUGLOG(SPDK_LOG_BDEV_RAID, "%s %d %d %d\n", raid_name, strip_size, num_base_bdevs, raid_level); - for (i = 0; i < g_spdk_raid_config.total_raid_bdev; i++) { - if (!strcmp(g_spdk_raid_config.raid_bdev_config[i].name, raid_name)) { + TAILQ_FOREACH(tmp, &g_spdk_raid_config.raid_bdev_config_head, link) { + if (!strcmp(tmp->name, raid_name)) { SPDK_ERRLOG("Duplicate raid bdev name found in config file %s\n", raid_name); return -1; } } - temp_ptr = realloc(g_spdk_raid_config.raid_bdev_config, - sizeof(struct raid_bdev_config) * (g_spdk_raid_config.total_raid_bdev + 1)); - if (temp_ptr == NULL) { + + raid_bdev_config = calloc(1, sizeof(*raid_bdev_config)); + if (raid_bdev_config == NULL) { SPDK_ERRLOG("unable to allocate memory\n"); - return -1; + return -ENOMEM; } - g_spdk_raid_config.raid_bdev_config = temp_ptr; - raid_bdev_config = &g_spdk_raid_config.raid_bdev_config[g_spdk_raid_config.total_raid_bdev]; - memset(raid_bdev_config, 0, sizeof(*raid_bdev_config)); raid_bdev_config->name = strdup(raid_name); if (!raid_bdev_config->name) { + free(raid_bdev_config); SPDK_ERRLOG("unable to allocate memory\n"); - return -1; + return -ENOMEM; } raid_bdev_config->strip_size = strip_size; raid_bdev_config->num_base_bdevs = num_base_bdevs; raid_bdev_config->raid_level = raid_level; + TAILQ_INSERT_TAIL(&g_spdk_raid_config.raid_bdev_config_head, raid_bdev_config, link); g_spdk_raid_config.total_raid_bdev++; + raid_bdev_config->base_bdev = calloc(num_base_bdevs, sizeof(*raid_bdev_config->base_bdev)); if (raid_bdev_config->base_bdev == NULL) { SPDK_ERRLOG("unable to allocate memory\n"); - return -1; + rc = -ENOMEM; + goto error; } for (i = 0; true; i++) { @@ -902,28 +902,51 @@ raid_bdev_parse_raid(struct spdk_conf_section *conf_section) } if (i >= raid_bdev_config->num_base_bdevs) { SPDK_ERRLOG("Number of devices mentioned is more than count\n"); - return -1; + rc = -1; + goto error; } - for (uint32_t j = 0; j < g_spdk_raid_config.total_raid_bdev; j++) { - for (uint32_t k = 0; k < g_spdk_raid_config.raid_bdev_config[j].num_base_bdevs; - k++) { - if (g_spdk_raid_config.raid_bdev_config[j].base_bdev[k].bdev_name != NULL) { - if (!strcmp(g_spdk_raid_config.raid_bdev_config[j].base_bdev[k].bdev_name, - base_bdev_name)) { - SPDK_ERRLOG("duplicate base bdev name %s mentioned\n", base_bdev_name); - return -1; + + 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)) { + SPDK_ERRLOG("duplicate base bdev name %s mentioned\n", + base_bdev_name); + rc = -EEXIST; + goto error; } } } } + raid_bdev_config->base_bdev[i].bdev_name = strdup(base_bdev_name); + if (raid_bdev_config->base_bdev[i].bdev_name == NULL) { + SPDK_ERRLOG("unable to allocate memory\n"); + rc = -ENOMEM; + goto error; + } } if (i != raid_bdev_config->num_base_bdevs) { SPDK_ERRLOG("Number of devices mentioned is less than count\n"); - return -1; + rc = -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; } /* @@ -1005,21 +1028,22 @@ static bool raid_bdev_can_claim_bdev(const char *bdev_name, struct raid_bdev_config **raid_bdev_config, uint32_t *base_bdev_slot) { - bool rv = false; + bool rv = false; + struct raid_bdev_config *raid_cfg; + uint32_t i; - for (uint32_t i = 0; i < g_spdk_raid_config.total_raid_bdev && !rv; i++) { - for (uint32_t j = 0; j < g_spdk_raid_config.raid_bdev_config[i].num_base_bdevs; - j++) { + TAILQ_FOREACH(raid_cfg, &g_spdk_raid_config.raid_bdev_config_head, link) { + for (i = 0; i < raid_cfg->num_base_bdevs; i++) { /* * Check if the base bdev name is part of raid bdev configuration. * If match is found then return true and the slot information where * this base bdev should be inserted in raid bdev */ - if (!strcmp(bdev_name, g_spdk_raid_config.raid_bdev_config[i].base_bdev[j].bdev_name)) { - *raid_bdev_config = &g_spdk_raid_config.raid_bdev_config[i]; - *base_bdev_slot = j; + if (!strcmp(bdev_name, raid_cfg->base_bdev[i].bdev_name)) { + *raid_bdev_config = raid_cfg; + *base_bdev_slot = i; rv = true; - break; + break;; } } } @@ -1054,7 +1078,6 @@ raid_bdev_init(void) { int ret; - memset(&g_spdk_raid_config, 0, sizeof(g_spdk_raid_config)); TAILQ_INIT(&g_spdk_raid_bdev_configured_list); TAILQ_INIT(&g_spdk_raid_bdev_configuring_list); TAILQ_INIT(&g_spdk_raid_bdev_list); diff --git a/lib/bdev/raid/bdev_raid.h b/lib/bdev/raid/bdev_raid.h index 260649dae..d2dbf66f1 100644 --- a/lib/bdev/raid/bdev_raid.h +++ b/lib/bdev/raid/bdev_raid.h @@ -185,6 +185,8 @@ struct raid_bdev_config { /* raid level */ uint8_t raid_level; + + TAILQ_ENTRY(raid_bdev_config) link; }; /* @@ -193,7 +195,7 @@ struct raid_bdev_config { */ struct raid_config { /* raid bdev context from config file */ - struct raid_bdev_config *raid_bdev_config; + TAILQ_HEAD(, raid_bdev_config) raid_bdev_config_head; /* total raid bdev from config file */ uint8_t total_raid_bdev; diff --git a/lib/bdev/raid/bdev_raid_rpc.c b/lib/bdev/raid/bdev_raid_rpc.c index 10a48ab9f..44f68a97a 100644 --- a/lib/bdev/raid/bdev_raid_rpc.c +++ b/lib/bdev/raid/bdev_raid_rpc.c @@ -247,29 +247,18 @@ static const struct spdk_json_object_decoder rpc_construct_raid_bdev_decoders[] * brief: * raid_bdev_config_cleanup function is used to free memory for one raid_bdev in configuration * params: - * none + * raid_bdev_config - pointer to raid_bdev_config structure * returns: * none */ static void -raid_bdev_config_cleanup(void) +raid_bdev_config_cleanup(struct raid_bdev_config *raid_cfg) { - void *temp_ptr; - + TAILQ_REMOVE(&g_spdk_raid_config.raid_bdev_config_head, raid_cfg, link); g_spdk_raid_config.total_raid_bdev--; - if (g_spdk_raid_config.total_raid_bdev == 0) { - free(g_spdk_raid_config.raid_bdev_config); - g_spdk_raid_config.raid_bdev_config = NULL; - } else { - temp_ptr = realloc(g_spdk_raid_config.raid_bdev_config, - sizeof(struct raid_bdev_config) * (g_spdk_raid_config.total_raid_bdev)); - if (temp_ptr != NULL) { - g_spdk_raid_config.raid_bdev_config = temp_ptr; - } else { - SPDK_ERRLOG("Config memory allocation failed\n"); - assert(0); - } - } + + free(raid_cfg->base_bdev); + free(raid_cfg); } /* @@ -339,7 +328,6 @@ spdk_rpc_construct_raid_bdev(struct spdk_jsonrpc_request *request, struct rpc_construct_raid_bdev req = {}; struct spdk_json_write_ctx *w; struct raid_bdev_ctxt *raid_bdev_ctxt; - void *temp_ptr; struct raid_base_bdev_config *base_bdevs; struct raid_bdev_config *raid_bdev_config; struct spdk_bdev *base_bdev; @@ -380,26 +368,20 @@ spdk_rpc_construct_raid_bdev(struct spdk_jsonrpc_request *request, return; } - /* Insert the new raid bdev config entry */ - temp_ptr = realloc(g_spdk_raid_config.raid_bdev_config, - sizeof(struct raid_bdev_config) * (g_spdk_raid_config.total_raid_bdev + 1)); - if (temp_ptr == NULL) { + /* Allocate the new raid bdev config entry */ + raid_bdev_config = calloc(1, sizeof(*raid_bdev_config)); + if (raid_bdev_config == NULL) { free(base_bdevs); spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR, spdk_strerror(ENOMEM)); free_rpc_construct_raid_bdev(&req); return; } - g_spdk_raid_config.raid_bdev_config = temp_ptr; - for (size_t i = 0; i < g_spdk_raid_config.total_raid_bdev; i++) { - g_spdk_raid_config.raid_bdev_config[i].raid_bdev_ctxt->raid_bdev.raid_bdev_config = - &g_spdk_raid_config.raid_bdev_config[i]; - } - raid_bdev_config = &g_spdk_raid_config.raid_bdev_config[g_spdk_raid_config.total_raid_bdev]; - memset(raid_bdev_config, 0, sizeof(*raid_bdev_config)); + raid_bdev_config->name = req.name; raid_bdev_config->strip_size = req.strip_size; raid_bdev_config->num_base_bdevs = req.base_bdevs.num_base_bdevs; raid_bdev_config->raid_level = req.raid_level; + TAILQ_INSERT_TAIL(&g_spdk_raid_config.raid_bdev_config_head, raid_bdev_config, link); g_spdk_raid_config.total_raid_bdev++; raid_bdev_config->base_bdev = base_bdevs; for (size_t i = 0; i < raid_bdev_config->num_base_bdevs; i++) { @@ -410,10 +392,8 @@ spdk_rpc_construct_raid_bdev(struct spdk_jsonrpc_request *request, /* Check if base_bdev exists already, if not fail the command */ base_bdev = spdk_bdev_get_by_name(req.base_bdevs.base_bdevs[i]); if (base_bdev == NULL) { - check_and_remove_raid_bdev(&g_spdk_raid_config.raid_bdev_config[g_spdk_raid_config.total_raid_bdev - - 1]); - raid_bdev_config_cleanup(); - free(base_bdevs); + check_and_remove_raid_bdev(raid_bdev_config); + raid_bdev_config_cleanup(raid_bdev_config); spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR, "base bdev not found"); free_rpc_construct_raid_bdev(&req); return; @@ -425,10 +405,8 @@ spdk_rpc_construct_raid_bdev(struct spdk_jsonrpc_request *request, * by some other module */ if (raid_bdev_add_base_device(base_bdev)) { - check_and_remove_raid_bdev(&g_spdk_raid_config.raid_bdev_config[g_spdk_raid_config.total_raid_bdev - - 1]); - raid_bdev_config_cleanup(); - free(base_bdevs); + check_and_remove_raid_bdev(raid_bdev_config); + raid_bdev_config_cleanup(raid_bdev_config); spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR, "base bdev can't be added because of either memory allocation failed or not able to claim"); free_rpc_construct_raid_bdev(&req); @@ -436,6 +414,7 @@ spdk_rpc_construct_raid_bdev(struct spdk_jsonrpc_request *request, } } + w = spdk_jsonrpc_begin_result(request); if (w == NULL) { return; @@ -512,10 +491,7 @@ raid_bdev_config_destroy_check_raid_bdev_exists(void *arg) static void raid_bdev_config_destroy(struct raid_bdev_config *raid_cfg) { - void *temp_ptr; - uint8_t i; - struct raid_bdev_config *raid_cfg_next; - uint8_t slot; + uint8_t i; assert(raid_cfg != NULL); if (raid_cfg->raid_bdev_ctxt != NULL) { @@ -527,46 +503,16 @@ raid_bdev_config_destroy(struct raid_bdev_config *raid_cfg) return; } + TAILQ_REMOVE(&g_spdk_raid_config.raid_bdev_config_head, raid_cfg, link); + g_spdk_raid_config.total_raid_bdev--; + /* Destroy raid bdev config and cleanup */ - for (uint8_t j = 0; j < raid_cfg->num_base_bdevs; j++) { - free(raid_cfg->base_bdev[j].bdev_name); + 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); - slot = raid_cfg - g_spdk_raid_config.raid_bdev_config; - assert(slot < g_spdk_raid_config.total_raid_bdev); - if (slot != g_spdk_raid_config.total_raid_bdev - 1) { - i = slot; - while (i < g_spdk_raid_config.total_raid_bdev - 1) { - raid_cfg = &g_spdk_raid_config.raid_bdev_config[i]; - raid_cfg_next = &g_spdk_raid_config.raid_bdev_config[i + 1]; - raid_cfg->base_bdev = raid_cfg_next->base_bdev; - raid_cfg->raid_bdev_ctxt = raid_cfg_next->raid_bdev_ctxt; - raid_cfg->name = raid_cfg_next->name; - raid_cfg->strip_size = raid_cfg_next->strip_size; - raid_cfg->num_base_bdevs = raid_cfg_next->num_base_bdevs; - raid_cfg->raid_level = raid_cfg_next->raid_level; - i++; - } - } - temp_ptr = realloc(g_spdk_raid_config.raid_bdev_config, - sizeof(struct raid_bdev_config) * (g_spdk_raid_config.total_raid_bdev - 1)); - if (temp_ptr != NULL) { - g_spdk_raid_config.raid_bdev_config = temp_ptr; - g_spdk_raid_config.total_raid_bdev--; - for (i = 0; i < g_spdk_raid_config.total_raid_bdev; i++) { - g_spdk_raid_config.raid_bdev_config[i].raid_bdev_ctxt->raid_bdev.raid_bdev_config = - &g_spdk_raid_config.raid_bdev_config[i]; - } - } else { - if (g_spdk_raid_config.total_raid_bdev == 1) { - g_spdk_raid_config.total_raid_bdev--; - g_spdk_raid_config.raid_bdev_config = NULL; - } else { - SPDK_ERRLOG("Config memory allocation failed\n"); - assert(0); - } - } + free(raid_cfg); } /* @@ -596,9 +542,8 @@ spdk_rpc_destroy_raid_bdev(struct spdk_jsonrpc_request *request, const struct sp } /* Find raid bdev config for this raid bdev */ - for (uint32_t i = 0; i < g_spdk_raid_config.total_raid_bdev; i++) { - if (strcmp(g_spdk_raid_config.raid_bdev_config[i].name, req.name) == 0) { - raid_bdev_config = &g_spdk_raid_config.raid_bdev_config[i]; + TAILQ_FOREACH(raid_bdev_config, &g_spdk_raid_config.raid_bdev_config_head, link) { + if (strcmp(raid_bdev_config->name, req.name) == 0) { break; } } diff --git a/test/unit/lib/bdev/bdev_raid.c/bdev_raid_ut.c b/test/unit/lib/bdev/bdev_raid.c/bdev_raid_ut.c index a65186495..69b5834b0 100644 --- a/test/unit/lib/bdev/bdev_raid.c/bdev_raid_ut.c +++ b/test/unit/lib/bdev/bdev_raid.c/bdev_raid_ut.c @@ -718,12 +718,13 @@ verify_io(struct spdk_bdev_io *bdev_io, uint8_t num_base_drives, static void verify_raid_config_present(const char *name, bool presence) { - uint32_t i; + struct raid_bdev_config *raid_cfg; bool cfg_found; cfg_found = false; - for (i = 0; i < g_spdk_raid_config.total_raid_bdev; i++) { - if (strcmp(name, g_spdk_raid_config.raid_bdev_config[i].name) == 0) { + + TAILQ_FOREACH(raid_cfg, &g_spdk_raid_config.raid_bdev_config_head, link) { + if (strcmp(name, raid_cfg->name) == 0) { cfg_found = true; break; } @@ -761,12 +762,11 @@ static void verify_raid_config(struct rpc_construct_raid_bdev *r, bool presence) { struct raid_bdev_config *raid_cfg = NULL; - uint32_t i, j; + uint32_t i; int val; - for (i = 0; i < g_spdk_raid_config.total_raid_bdev; i++) { - if (strcmp(r->name, g_spdk_raid_config.raid_bdev_config[i].name) == 0) { - raid_cfg = &g_spdk_raid_config.raid_bdev_config[i]; + TAILQ_FOREACH(raid_cfg, &g_spdk_raid_config.raid_bdev_config_head, link) { + if (strcmp(r->name, raid_cfg->name) == 0) { if (presence == false) { break; } @@ -774,8 +774,8 @@ verify_raid_config(struct rpc_construct_raid_bdev *r, bool presence) CU_ASSERT(raid_cfg->strip_size == r->strip_size); CU_ASSERT(raid_cfg->num_base_bdevs == r->base_bdevs.num_base_bdevs); CU_ASSERT(raid_cfg->raid_level == r->raid_level); - for (j = 0; j < raid_cfg->num_base_bdevs; j++) { - val = strcmp(raid_cfg->base_bdev[j].bdev_name, r->base_bdevs.base_bdevs[j]); + for (i = 0; i < raid_cfg->num_base_bdevs; i++) { + val = strcmp(raid_cfg->base_bdev[i].bdev_name, r->base_bdevs.base_bdevs[i]); CU_ASSERT(val == 0); } break;