diff --git a/lib/bdev/raid/bdev_raid.c b/lib/bdev/raid/bdev_raid.c index 4995d462c..dbc6b37f1 100644 --- a/lib/bdev/raid/bdev_raid.c +++ b/lib/bdev/raid/bdev_raid.c @@ -654,6 +654,27 @@ raid_bdev_free(void) } } +/* brief + * raid_bdev_config_find_by_name is a helper function to find raid bdev config + * by name as key. + * + * params: + * raid_name - name for raid bdev. + */ +static struct raid_bdev_config * +raid_bdev_config_find_by_name(const char *raid_name) +{ + struct raid_bdev_config *raid_cfg; + + TAILQ_FOREACH(raid_cfg, &g_spdk_raid_config.raid_bdev_config_head, link) { + if (!strcmp(raid_cfg->name, raid_name)) { + return raid_cfg; + } + } + + return raid_cfg; +} + /* * brief * raid_bdev_config_add function adds config for newly created raid bdev. @@ -671,12 +692,11 @@ raid_bdev_config_add(const char *raid_name, int strip_size, int num_base_bdevs, { struct raid_bdev_config *raid_cfg; - TAILQ_FOREACH(raid_cfg, &g_spdk_raid_config.raid_bdev_config_head, link) { - if (!strcmp(raid_cfg->name, raid_name)) { - SPDK_ERRLOG("Duplicate raid bdev name found in config file %s\n", - raid_name); - return -EEXIST; - } + raid_cfg = raid_bdev_config_find_by_name(raid_name); + if (raid_cfg != NULL) { + SPDK_ERRLOG("Duplicate raid bdev name found in config file %s\n", + raid_name); + return -EEXIST; } if (spdk_u32_is_pow2(strip_size) == false) { diff --git a/lib/bdev/raid/bdev_raid_rpc.c b/lib/bdev/raid/bdev_raid_rpc.c index c8d84749e..6623ee954 100644 --- a/lib/bdev/raid/bdev_raid_rpc.c +++ b/lib/bdev/raid/bdev_raid_rpc.c @@ -212,38 +212,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: - * check_and_remove_raid_bdev function free base bdev descriptors, unclaim the base - * bdevs and free the raid. This function is used to cleanup when raid is not - * able to successfully create during constructing the raid via RPC - * params: - * raid_bdev_config - pointer to raid_bdev_config structure - * returns: - * NULL - raid not present - * non NULL - raid present, returns raid_bdev - */ -static void -check_and_remove_raid_bdev(struct raid_bdev_config *raid_cfg) -{ - struct raid_bdev *raid_bdev; - - /* Get the raid structured allocated if exists */ - raid_bdev = raid_cfg->raid_bdev; - if (raid_bdev == NULL) { - return; - } - - for (uint32_t i = 0; i < raid_bdev->num_base_bdevs; i++) { - assert(raid_bdev->base_bdev_info != NULL); - if (raid_bdev->base_bdev_info[i].bdev) { - raid_bdev_free_base_bdev_resource(raid_bdev, i); - } - } - assert(raid_bdev->num_base_bdevs_discovered == 0); - raid_bdev_cleanup(raid_bdev); -} - /* * brief: * spdk_rpc_construct_raid_bdev function is the RPC for construct_raids. It takes @@ -262,7 +230,7 @@ spdk_rpc_construct_raid_bdev(struct spdk_jsonrpc_request *request, struct spdk_json_write_ctx *w; struct raid_bdev_config *raid_cfg; struct spdk_bdev *base_bdev; - int rc; + int rc, _rc; if (spdk_json_decode_object(params, rpc_construct_raid_bdev_decoders, SPDK_COUNTOF(rpc_construct_raid_bdev_decoders), @@ -310,13 +278,9 @@ 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(raid_cfg); - raid_bdev_config_cleanup(raid_cfg); - spdk_jsonrpc_send_error_response_fmt(request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR, - "base bdev %s not found", - req.base_bdevs.base_bdevs[i]); - free_rpc_construct_raid_bdev(&req); - return; + SPDK_DEBUGLOG(SPDK_LOG_BDEV_RAID, "base bdev %s doesn't exist now\n", + req.base_bdevs.base_bdevs[i]); + continue; } /* @@ -324,19 +288,25 @@ spdk_rpc_construct_raid_bdev(struct spdk_jsonrpc_request *request, * command. This might be because this base_bdev may already be claimed * by some other module */ - rc = raid_bdev_add_base_device(raid_cfg, base_bdev, i); - if (rc != 0) { - check_and_remove_raid_bdev(raid_cfg); - raid_bdev_config_cleanup(raid_cfg); - spdk_jsonrpc_send_error_response_fmt(request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR, - "Failed to add base bdev %s to RAID bdev %s: %s", - req.base_bdevs.base_bdevs[i], req.name, - spdk_strerror(-rc)); - free_rpc_construct_raid_bdev(&req); - return; + _rc = raid_bdev_add_base_device(raid_cfg, base_bdev, i); + if (_rc != 0) { + SPDK_ERRLOG("Failed to add base bdev %s to RAID bdev %s: %s\n", + req.base_bdevs.base_bdevs[i], req.name, + spdk_strerror(-_rc)); + if (rc == 0) { + rc = _rc; + } } } + if (rc != 0) { + spdk_jsonrpc_send_error_response_fmt(request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR, + "Failed to add base bdev to RAID bdev %s: %s", + req.name, spdk_strerror(-rc)); + free_rpc_construct_raid_bdev(&req); + return; + } + free_rpc_construct_raid_bdev(&req); w = spdk_jsonrpc_begin_result(request); 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 bcd50eb96..1ac2fc687 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 @@ -155,6 +155,27 @@ base_bdevs_cleanup(void) } } +static void +check_and_remove_raid_bdev(struct raid_bdev_config *raid_cfg) +{ + struct raid_bdev *raid_bdev; + + /* Get the raid structured allocated if exists */ + raid_bdev = raid_cfg->raid_bdev; + if (raid_bdev == NULL) { + return; + } + + for (uint32_t i = 0; i < raid_bdev->num_base_bdevs; i++) { + assert(raid_bdev->base_bdev_info != NULL); + if (raid_bdev->base_bdev_info[i].bdev) { + raid_bdev_free_base_bdev_resource(raid_bdev, i); + } + } + assert(raid_bdev->num_base_bdevs_discovered == 0); + raid_bdev_cleanup(raid_bdev); +} + /* Reset globals */ static void reset_globals(void) @@ -1090,6 +1111,7 @@ test_construct_raid_invalid_args(void) { struct rpc_construct_raid_bdev req; struct rpc_destroy_raid_bdev destroy_req; + struct raid_bdev_config *raid_cfg; set_globals(); rpc_req = &req; @@ -1175,10 +1197,14 @@ test_construct_raid_invalid_args(void) g_rpc_err = 0; g_json_decode_obj_construct = 1; spdk_rpc_construct_raid_bdev(NULL, NULL); - CU_ASSERT(g_rpc_err == 1); + CU_ASSERT(g_rpc_err == 0); free_test_req(&req); - verify_raid_config_present("raid2", false); - verify_raid_bdev_present("raid2", false); + verify_raid_config_present("raid2", true); + verify_raid_bdev_present("raid2", true); + raid_cfg = raid_bdev_config_find_by_name("raid2"); + SPDK_CU_ASSERT_FATAL(raid_cfg != NULL); + check_and_remove_raid_bdev(raid_cfg); + raid_bdev_config_cleanup(raid_cfg); create_test_req(&req, "raid2", g_max_base_drives, false); g_rpc_err = 0;