Commit 4219407a authored by Tomasz Zawadzki's avatar Tomasz Zawadzki Committed by Jim Harris
Browse files

lvol: destroy_lvol_bdev implementation



This patch fixes lvol delete behaviour.
First, we look if there are any dependencies that disallow lvol deletion.
If there are any (i.e. dependent clones) we fail.
Otherwise we delete lvol and unregister associated bdev.

destroy_bdev no longer deletes lvol.

Fixes #345

Signed-off-by: default avatarPiotr Pelplinski <piotr.pelplinski@intel.com>
Signed-off-by: default avatarTomasz Zawadzki <tomasz.zawadzki@intel.com>
Change-Id: I99e6abded2ed3ae2742103f81fc7eb937ad1cab4
Reviewed-on: https://review.gerrithub.io/407402


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
parent 848daf27
Loading
Loading
Loading
Loading
+6 −0
Original line number Diff line number Diff line
@@ -205,6 +205,12 @@ void
spdk_lvol_rename(struct spdk_lvol *lvol, const char *new_name,
		 spdk_lvol_op_complete cb_fn, void *cb_arg);

/**
 * \brief Returns if it is possible to delete an lvol (i.e. lvol is not a snapshot that have at least one clone).
 * \param lvol Handle to lvol
 */
bool spdk_lvol_deletable(struct spdk_lvol *lvol);

/**
 * Close lvol and remove information about lvol from its lvolstore.
 *
+0 −1
Original line number Diff line number Diff line
@@ -105,7 +105,6 @@ struct spdk_lvol {
	char				name[SPDK_LVOL_NAME_MAX];
	struct spdk_uuid		uuid;
	char				uuid_str[SPDK_UUID_STRING_LEN];
	bool				close_only;
	bool				thin_provision;
	struct spdk_bdev		*bdev;
	int				ref_count;
+118 −66
Original line number Diff line number Diff line
@@ -330,6 +330,41 @@ _vbdev_lvs_remove_cb(void *cb_arg, int lvserrno)
	free(req);
}

static void
_vbdev_lvs_remove_lvol_cb(void *cb_arg, int lvolerrno)
{
	struct lvol_store_bdev *lvs_bdev = cb_arg;
	struct spdk_lvol_store *lvs = lvs_bdev->lvs;
	struct spdk_lvol *lvol;

	if (lvolerrno != 0) {
		SPDK_DEBUGLOG(SPDK_LOG_VBDEV_LVOL, "Lvol removed with errno %d\n", lvolerrno);
	}

	if (TAILQ_EMPTY(&lvs->lvols)) {
		return;
	}

	lvol = TAILQ_FIRST(&lvs->lvols);
	while (lvol != NULL) {
		if (spdk_lvol_deletable(lvol)) {
			vbdev_lvol_destroy(lvol, _vbdev_lvs_remove_lvol_cb, lvs_bdev);
			return;
		}
		lvol = TAILQ_NEXT(lvol, link);
	}

	/* If no lvol is deletable, that means there is circular dependency. */
	SPDK_ERRLOG("Lvols left in lvs, but unable to delete.\n");
	assert(false);
}

static void
_vbdev_lvs_unregister_empty_cb(void *cb_arg, int bdeverrno)
{
	SPDK_DEBUGLOG(SPDK_LOG_VBDEV_LVOL, "Lvol unregistered with errno %d\n", bdeverrno);
}

static void
_vbdev_lvs_remove(struct spdk_lvol_store *lvs, spdk_lvs_op_complete cb_fn, void *cb_arg,
		  bool destroy)
@@ -383,9 +418,12 @@ _vbdev_lvs_remove(struct spdk_lvol_store *lvs, spdk_lvs_op_complete cb_fn, void
		lvs->destruct_req->cb_fn = _vbdev_lvs_remove_cb;
		lvs->destruct_req->cb_arg = lvs_bdev;
		lvs->destruct = destroy;
		if (destroy) {
			_vbdev_lvs_remove_lvol_cb(lvs_bdev, 0);
		} else {
			TAILQ_FOREACH_SAFE(lvol, &lvs->lvols, link, tmp) {
			lvol->close_only = !destroy;
			spdk_bdev_unregister(lvol->bdev, NULL, NULL);
				spdk_bdev_unregister(lvol->bdev, _vbdev_lvs_unregister_empty_cb, NULL);
			}
		}
	}
}
@@ -477,63 +515,24 @@ vbdev_get_lvol_store_by_name(const char *name)
	return NULL;
}

static void
_vbdev_lvol_close_cb(void *cb_arg, int lvserrno)
{
	struct spdk_lvol_store *lvs;

	if (cb_arg == NULL) {
		/*
		 * This close cb is from unload/destruct - so do not continue to check
		 *  the lvol open counts.
		 */
		return;
	}

	lvs = cb_arg;

	if (lvs->lvols_opened >= lvs->lvol_count) {
		SPDK_INFOLOG(SPDK_LOG_VBDEV_LVOL, "Opening lvols finished\n");
		spdk_bdev_module_examine_done(&g_lvol_if);
	}
}

static void
_vbdev_lvol_destroy_cb(void *cb_arg, int lvserrno)
{
	struct spdk_bdev *bdev = cb_arg;

	if (lvserrno == -EBUSY) {
		/* TODO: Handle reporting error to spdk_bdev_unregister */
	}

	SPDK_INFOLOG(SPDK_LOG_VBDEV_LVOL, "Lvol destroyed\n");

	spdk_bdev_destruct_done(bdev, lvserrno);
	free(bdev->name);
	free(bdev);
}
struct vbdev_lvol_destroy_ctx {
	struct spdk_lvol *lvol;
	spdk_lvol_op_complete cb_fn;
	void *cb_arg;
};

static void
_vbdev_lvol_destroy_after_close_cb(void *cb_arg, int lvserrno)
_vbdev_lvol_unregister_cb(void *ctx, int lvolerrno)
{
	struct spdk_lvol *lvol = cb_arg;
	struct spdk_bdev *bdev = lvol->bdev;
	struct spdk_bdev *bdev = ctx;

	if (lvserrno != 0) {
		SPDK_INFOLOG(SPDK_LOG_VBDEV_LVOL, "Could not close Lvol %s\n", lvol->unique_id);
		spdk_bdev_destruct_done(bdev, lvserrno);
	spdk_bdev_destruct_done(bdev, lvolerrno);
	free(bdev->name);
	free(bdev);
		return;
	}

	SPDK_INFOLOG(SPDK_LOG_VBDEV_LVOL, "Lvol %s closed, begin destroying\n", lvol->unique_id);
	spdk_lvol_destroy(lvol, _vbdev_lvol_destroy_cb, bdev);
}

static int
vbdev_lvol_destruct(void *ctx)
vbdev_lvol_unregister(void *ctx)
{
	struct spdk_lvol *lvol = ctx;
	char *alias;
@@ -547,14 +546,7 @@ vbdev_lvol_destruct(void *ctx)
	} else {
		SPDK_ERRLOG("Cannot alloc memory for alias\n");
	}

	if (lvol->close_only) {
		free(lvol->bdev->name);
		free(lvol->bdev);
		spdk_lvol_close(lvol, _vbdev_lvol_close_cb, NULL);
	} else {
		spdk_lvol_close(lvol, _vbdev_lvol_destroy_after_close_cb, lvol);
	}
	spdk_lvol_close(lvol, _vbdev_lvol_unregister_cb, lvol->bdev);

	/* return 1 to indicate we have an operation that must finish asynchronously before the
	 *  lvol is closed
@@ -562,14 +554,63 @@ vbdev_lvol_destruct(void *ctx)
	return 1;
}

static void
_vbdev_lvol_destroy_cb(void *cb_arg, int bdeverrno)
{
	struct vbdev_lvol_destroy_ctx *ctx = cb_arg;
	struct spdk_lvol *lvol = ctx->lvol;

	if (bdeverrno < 0) {
		SPDK_INFOLOG(SPDK_LOG_VBDEV_LVOL, "Could not unregister bdev during lvol (%s) destroy\n",
			     lvol->unique_id);
		ctx->cb_fn(ctx->cb_arg, bdeverrno);
		free(ctx);
		return;
	}

	spdk_lvol_destroy(lvol, ctx->cb_fn, ctx->cb_arg);
	free(ctx);
}

void
vbdev_lvol_destroy(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_arg)
{
	/*
	 * TODO: This should call spdk_lvol_destroy() directly, and the bdev unregister path
	 * should be changed so that it does not destroy the lvol.
	 */
	spdk_bdev_unregister(lvol->bdev, cb_fn, cb_arg);
	struct vbdev_lvol_destroy_ctx *ctx;
	char *alias;

	assert(lvol != NULL);
	assert(cb_fn != NULL);

	/* Check if it is possible to delete lvol */
	if (spdk_lvol_deletable(lvol) == false) {
		/* throw an error */
		SPDK_ERRLOG("Cannot delete lvol\n");
		cb_fn(cb_arg, -EPERM);
		return;
	}

	ctx = calloc(1, sizeof(*ctx));
	if (!ctx) {
		cb_fn(cb_arg, -ENOMEM);
		return;
	}

	ctx->lvol = lvol;
	ctx->cb_fn = cb_fn;
	ctx->cb_arg = cb_arg;

	alias = spdk_sprintf_alloc("%s/%s", lvol->lvol_store->name, lvol->name);
	if (alias != NULL) {
		spdk_bdev_alias_del(lvol->bdev, alias);
		free(alias);
	} else {
		SPDK_ERRLOG("Cannot alloc memory for alias\n");
		cb_fn(cb_arg, -ENOMEM);
		free(ctx);
		return;
	}

	spdk_bdev_unregister(lvol->bdev, _vbdev_lvol_destroy_cb, ctx);
}

static char *
@@ -853,7 +894,7 @@ vbdev_lvol_submit_request(struct spdk_io_channel *ch, struct spdk_bdev_io *bdev_
}

static struct spdk_bdev_fn_table vbdev_lvol_fn_table = {
	.destruct		= vbdev_lvol_destruct,
	.destruct		= vbdev_lvol_unregister,
	.io_type_supported	= vbdev_lvol_io_type_supported,
	.submit_request		= vbdev_lvol_submit_request,
	.get_io_channel		= vbdev_lvol_get_io_channel,
@@ -1124,6 +1165,17 @@ _vbdev_lvs_examine_failed(void *cb_arg, int lvserrno)
	spdk_bdev_module_examine_done(&g_lvol_if);
}

static void
_vbdev_lvol_examine_close_cb(void *cb_arg, int lvserrno)
{
	struct spdk_lvol_store *lvs = cb_arg;

	if (lvs->lvols_opened >= lvs->lvol_count) {
		SPDK_INFOLOG(SPDK_LOG_VBDEV_LVOL, "Opening lvols finished\n");
		spdk_bdev_module_examine_done(&g_lvol_if);
	}
}

static void
_vbdev_lvs_examine_finish(void *cb_arg, struct spdk_lvol *lvol, int lvolerrno)
{
@@ -1144,7 +1196,7 @@ _vbdev_lvs_examine_finish(void *cb_arg, struct spdk_lvol *lvol, int lvolerrno)
		SPDK_ERRLOG("Cannot create bdev for lvol %s\n", lvol->unique_id);
		TAILQ_REMOVE(&lvs->lvols, lvol, link);
		lvs->lvol_count--;
		spdk_lvol_close(lvol, _vbdev_lvol_close_cb, lvs);
		spdk_lvol_close(lvol, _vbdev_lvol_examine_close_cb, lvs);
		SPDK_INFOLOG(SPDK_LOG_VBDEV_LVOL, "Opening lvol %s failed\n", lvol->unique_id);
		return;
	}
+21 −16
Original line number Diff line number Diff line
@@ -84,6 +84,13 @@ _spdk_lvs_free(struct spdk_lvol_store *lvs)
	free(lvs);
}

static void
_spdk_lvol_free(struct spdk_lvol *lvol)
{
	free(lvol->unique_id);
	free(lvol);
}

static void
_spdk_lvol_open_cb(void *cb_arg, struct spdk_blob *blob, int lvolerrno)
{
@@ -192,7 +199,6 @@ _spdk_load_next_lvol(void *cb_arg, struct spdk_blob *blob, int lvolerrno)
	lvol->blob = blob;
	lvol->blob_id = blob_id;
	lvol->lvol_store = lvs;
	lvol->close_only = false;

	rc = spdk_blob_get_xattr_value(blob, "uuid", (const void **)&attr, &value_len);
	if (rc != 0 || value_len != SPDK_UUID_STRING_LEN || attr[SPDK_UUID_STRING_LEN - 1] != '\0' ||
@@ -218,8 +224,7 @@ _spdk_load_next_lvol(void *cb_arg, struct spdk_blob *blob, int lvolerrno)
	rc = spdk_blob_get_xattr_value(blob, "name", (const void **)&attr, &value_len);
	if (rc != 0 || value_len > SPDK_LVOL_NAME_MAX) {
		SPDK_ERRLOG("Cannot assign lvol name\n");
		free(lvol->unique_id);
		free(lvol);
		_spdk_lvol_free(lvol);
		req->lvserrno = -EINVAL;
		goto invalid;
	}
@@ -765,8 +770,7 @@ spdk_lvs_unload(struct spdk_lvol_store *lvs, spdk_lvs_op_complete cb_fn,

	TAILQ_FOREACH_SAFE(lvol, &lvs->lvols, link, tmp) {
		TAILQ_REMOVE(&lvs->lvols, lvol, link);
		free(lvol->unique_id);
		free(lvol);
		_spdk_lvol_free(lvol);
	}

	lvs_req = calloc(1, sizeof(*lvs_req));
@@ -877,8 +881,7 @@ _spdk_lvol_close_blob_cb(void *cb_arg, int lvolerrno)

	if (lvolerrno < 0) {
		SPDK_ERRLOG("Could not close blob on lvol\n");
		free(lvol->unique_id);
		free(lvol);
		_spdk_lvol_free(lvol);
		goto end;
	}

@@ -905,6 +908,15 @@ end:
	free(req);
}

bool
spdk_lvol_deletable(struct spdk_lvol *lvol)
{
	size_t count;

	spdk_blob_get_clones(lvol->lvol_store->blobstore, lvol->blob_id, NULL, &count);
	return (count == 0);
}

static void
_spdk_lvol_delete_blob_cb(void *cb_arg, int lvolerrno)
{
@@ -913,8 +925,6 @@ _spdk_lvol_delete_blob_cb(void *cb_arg, int lvolerrno)

	if (lvolerrno < 0) {
		SPDK_ERRLOG("Could not delete blob on lvol\n");
		free(lvol->unique_id);
		free(lvol);
		goto end;
	}

@@ -928,10 +938,8 @@ _spdk_lvol_delete_blob_cb(void *cb_arg, int lvolerrno)

	SPDK_INFOLOG(SPDK_LOG_LVOL, "Lvol %s deleted\n", lvol->unique_id);

	free(lvol->unique_id);
	free(lvol);

end:
	_spdk_lvol_free(lvol);
	req->cb_fn(req->cb_arg, lvolerrno);
	free(req);
}
@@ -945,8 +953,7 @@ _spdk_lvol_destroy_cb(void *cb_arg, int lvolerrno)

	if (lvolerrno < 0) {
		SPDK_ERRLOG("Could not close blob on lvol\n");
		free(lvol->unique_id);
		free(lvol);
		_spdk_lvol_free(lvol);
		req->cb_fn(req->cb_arg, lvolerrno);
		free(req);
		return;
@@ -1096,10 +1103,8 @@ spdk_lvol_create(struct spdk_lvol_store *lvs, const char *name, uint64_t sz,
		SPDK_ERRLOG("Cannot alloc memory for lvol base pointer\n");
		return -ENOMEM;
	}

	lvol->lvol_store = lvs;
	num_clusters = divide_round_up(sz, spdk_bs_get_cluster_size(bs));
	lvol->close_only = false;
	lvol->thin_provision = thin_provision;
	snprintf(lvol->name, sizeof(lvol->name), "%s", name);
	TAILQ_INSERT_TAIL(&lvol->lvol_store->pending_lvols, lvol, link);
+1 −1
Original line number Diff line number Diff line
@@ -13,7 +13,7 @@ rpc_py="python $rootdir/scripts/rpc.py"
function remove_backends()
{
	echo "INFO: Removing lvol bdev"
	$rpc_py delete_bdev "lvs_0/lbd_0"
	$rpc_py destroy_lvol_bdev "lvs_0/lbd_0"

	echo "INFO: Removing lvol stores"
	$rpc_py destroy_lvol_store -l lvs_0
Loading