Commit a3004632 authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Jim Harris
Browse files

bdev/raid: Change construct_raid_bdev RPC to be consistent to other bdev modules 



Change the behavior of construct_raid_bdev RPC to be consistent with
other bdev modules.

Create a config of raid bdev first. Then create a raid bdev based
on the config.

If both succeed, the config and the raid bdev are not removed even if
any failure occur later in construct_raid_bdev RPC. Otherwise, both
are removed and return failure.

During iteration of adding base bdevs in construct_raid_bdev RPC,
- skip any nonexistent bdev and move to the next base bdev.
- if adding any base bdev fails, move to the next bdev.
- if adding base bdevs don't fail but any base bdev doesn't exist,
  the raid bdev is in the configuring state and return success.
- if adding base bdev fails, the raid bdev is in the configuring state
  and return failure.
- if all adding base bdevs succeed, configure the raid bdev. If configuring
  the raid bdev fails, move the raid bdev to the offline state and return
  failure. If configuring the raid bdev succeed, return success.

check_and_remove_raid_bdev() becomes unused in raid_bdev_rpc.c but
is still necessary in UT. Hence move this function to UT.

In UT, finding a raid bdev config by name becomes necessary. Hence
factor out the iteration to a function and use the function in UT.

Change-Id: Ifa36967bdc987d97030e3a4e36684cb37b329d4e
Signed-off-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/423622


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: default avatarKunal Sablok <kunal.sablok@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
parent 50b51bc8

lib/bdev/raid/bdev_raid.c

+26 −6
Original line number Diff line number Diff line
@@ -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,13 +692,12 @@ 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)) {

	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) {

		SPDK_ERRLOG("Invalid strip size %d\n", strip_size);

lib/bdev/raid/bdev_raid_rpc.c

+20 −50
Original line number Diff line number Diff line
@@ -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",

			SPDK_DEBUGLOG(SPDK_LOG_BDEV_RAID, "base bdev %s doesn't exist now\n",

				      req.base_bdevs.base_bdevs[i]);

			free_rpc_construct_raid_bdev(&req);

			return;

			continue;

		}



		/*
@@ -324,18 +288,24 @@ 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);

		_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) {

			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));

						     "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);

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

+29 −3
Original line number Diff line number Diff line
@@ -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;