fix(blobstore): snapshot removal now properly ordered upon blobstore …

…delete (#32) Snapshots are now always deleted after all existing non-snapshot lvols when removing a blobstore. Signed-off-by: Mikhail Tcymbaliuk <[email protected]>
openebs · Jul 17, 2023 · 7ca587a · 7ca587a
1 parent 4b3b937
commit 7ca587a
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 20 deletions.
diff --git a/lib/blob/blobstore.c b/lib/blob/blobstore.c
@@ -7845,15 +7845,17 @@ spdk_blob_io_writev(struct spdk_blob *blob, struct spdk_io_channel *channel,
 		    struct iovec *iov, int iovcnt, uint64_t offset, uint64_t length,
 		    spdk_blob_op_complete cb_fn, void *cb_arg)
 {
-	blob_request_submit_rw_iov(blob, channel, iov, iovcnt, offset, length, 0, cb_fn, cb_arg, false, NULL);
+	blob_request_submit_rw_iov(blob, channel, iov, iovcnt, offset, length, 0, cb_fn, cb_arg, false,
+				   NULL);
 }
 
 void
 spdk_blob_io_readv(struct spdk_blob *blob, struct spdk_io_channel *channel,
 		   struct iovec *iov, int iovcnt, uint64_t offset, uint64_t length, uint32_t ext_io_flags,
 		   spdk_blob_op_complete cb_fn, void *cb_arg)
 {
-	blob_request_submit_rw_iov(blob, channel, iov, iovcnt, offset, length, ext_io_flags, cb_fn, cb_arg, true, NULL);
+	blob_request_submit_rw_iov(blob, channel, iov, iovcnt, offset, length, ext_io_flags, cb_fn, cb_arg,
+				   true, NULL);
 }
 
 void
@@ -7870,7 +7872,8 @@ spdk_blob_io_readv_ext(struct spdk_blob *blob, struct spdk_io_channel *channel,
 		       struct iovec *iov, int iovcnt, uint64_t offset, uint64_t length, uint32_t ext_io_flags,
 		       spdk_blob_op_complete cb_fn, void *cb_arg, struct spdk_blob_ext_io_opts *io_opts)
 {
-	blob_request_submit_rw_iov(blob, channel, iov, iovcnt, offset, length, ext_io_flags, cb_fn, cb_arg, true,
+	blob_request_submit_rw_iov(blob, channel, iov, iovcnt, offset, length, ext_io_flags, cb_fn, cb_arg,
+				   true,
 				   io_opts);
 }
 

diff --git a/lib/lvol/lvol.c b/lib/lvol/lvol.c
@@ -907,7 +907,7 @@ lvol_delete_blob_cb(void *cb_arg, int lvolerrno)
 	struct spdk_lvol *lvol = req->lvol;
 
 	if (lvolerrno < 0) {
-		SPDK_ERRLOG("Could not remove blob on lvol gracefully - forced removal\n");
+		SPDK_ERRLOG("%s: Could not remove blob on lvol gracefully - forced removal\n", lvol->name);
 	} else {
 		SPDK_INFOLOG(lvol, "Lvol %s deleted\n", lvol->unique_id);
 	}

diff --git a/module/bdev/lvol/vbdev_lvol.c b/module/bdev/lvol/vbdev_lvol.c
@@ -354,34 +354,85 @@ _vbdev_lvs_remove_cb(void *cb_arg, int lvserrno)
 	free(req);
 }
 
+enum vbdev_lvs_remove_step {
+	DELETE_LVOLS,     /* Delete all existing lvols. */
+	DELETE_SNAPSHOTS, /* Delete all existing snapshots after removing lvols. */
+	DELETE_LVS,       /* Delete lvs store once all lvols and snapshots are removed. */
+};
+
+struct vbdev_lvs_remove_ctx {
+	struct lvol_store_bdev *lvs_bdev;
+	enum vbdev_lvs_remove_step step;
+	bool skipped_snapshots;
+};
+
 static void
 _vbdev_lvs_remove_lvol_cb(void *cb_arg, int lvolerrno)
 {
-	struct lvol_store_bdev *lvs_bdev = cb_arg;
+	struct vbdev_lvs_remove_ctx *remove_ctx = cb_arg;
+	struct lvol_store_bdev *lvs_bdev = remove_ctx->lvs_bdev;
 	struct spdk_lvol_store *lvs = lvs_bdev->lvs;
 	struct spdk_lvol *lvol;
 
 	if (lvolerrno != 0) {
-		SPDK_DEBUGLOG(vbdev_lvol, "Lvol removed with errno %d\n", lvolerrno);
+		SPDK_ERRLOG("LVS %s: lvol removed with errno %d\n", lvs->name, lvolerrno);
 	}
 
-	if (TAILQ_EMPTY(&lvs->lvols)) {
-		spdk_lvs_destroy(lvs, _vbdev_lvs_remove_cb, lvs_bdev);
-		return;
-	}
+	/* Blobstore removal consists of the following sequential phases:
+	 * 1. Remove regular lvols.
+	 * 2. Remove snapshots.
+	 * 3. Destroy the blobstore once all lvols and snapshots are removed.
+	 */
+	switch (remove_ctx->step) {
+	case DELETE_LVOLS:
+		lvol = TAILQ_FIRST(&lvs->lvols);
+		while (lvol != NULL) {
+			if (spdk_lvol_deletable(lvol)) {
+				vbdev_lvol_destroy(lvol, _vbdev_lvs_remove_lvol_cb, remove_ctx);
+				return;
+			} else {
+				remove_ctx->skipped_snapshots = true;
+			}
+			lvol = TAILQ_NEXT(lvol, link);
+		}
 
-	lvol = TAILQ_FIRST(&lvs->lvols);
-	while (lvol != NULL) {
-		if (spdk_lvol_deletable(lvol)) {
-			vbdev_lvol_destroy(lvol, _vbdev_lvs_remove_lvol_cb, lvs_bdev);
+		/* Schedule snapshot removal in case snapshots exist, otherwise proceed with blobstore removal. */
+		if (remove_ctx->skipped_snapshots) {
+			SPDK_INFOLOG(vbdev_lvol, "all lvols deleted, starting snapshot removal\n");
+			remove_ctx->step = DELETE_SNAPSHOTS;
+			_vbdev_lvs_remove_lvol_cb(remove_ctx, 0);
+		} else {
+			SPDK_INFOLOG(vbdev_lvol, "no snapshots exist, proceeding directly to lvs removal\n");
+			remove_ctx->step = DELETE_LVS;
+			_vbdev_lvs_remove_lvol_cb(remove_ctx, 0);
+		}
+		break;
+	case DELETE_SNAPSHOTS:
+		lvol = TAILQ_FIRST(&lvs->lvols);
+		while (lvol != NULL) {
+			SPDK_INFOLOG(vbdev_lvol, "deleting snapshot: %s", lvol->name);
+
+			/* Must have only snapshots remaining in blobstore. */
+			assert(!spdk_lvol_deletable(lvol));
+			vbdev_lvol_destroy(lvol, _vbdev_lvs_remove_lvol_cb, remove_ctx);
 			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);
+		/* All snapshots removed, schedule blobstore removal. */
+		remove_ctx->step = DELETE_LVS;
+		_vbdev_lvs_remove_lvol_cb(remove_ctx, 0);
+		break;
+	case DELETE_LVS:
+		SPDK_INFOLOG(vbdev_lvol, "all lvols and snapshots deleted, deleting blobstore\n");
+		if (!TAILQ_EMPTY(&lvs->lvols)) {
+			SPDK_ERRLOG("Lvols left in lvs ('%s'), but unable to delete.\n", lvs->name);
+		}
+
+		/* Free invocation context and destroy blobstore. */
+		free(cb_arg);
+		spdk_lvs_destroy(lvs, _vbdev_lvs_remove_cb, lvs_bdev);
+		break;
+	}
 }
 
 static bool
@@ -451,7 +502,15 @@ _vbdev_lvs_remove(struct spdk_lvol_store *lvs, spdk_lvs_op_complete cb_fn, void
 		}
 	} else {
 		if (destroy) {
-			_vbdev_lvs_remove_lvol_cb(lvs_bdev, 0);
+			struct vbdev_lvs_remove_ctx *remove_ctx;
+
+			remove_ctx = calloc(1, sizeof(*remove_ctx));
+
+			remove_ctx->lvs_bdev = lvs_bdev;
+			remove_ctx->step = DELETE_LVOLS;
+			remove_ctx->skipped_snapshots = false;
+
+			_vbdev_lvs_remove_lvol_cb(remove_ctx, 0);
 		} else {
 			TAILQ_FOREACH_SAFE(lvol, &lvs->lvols, link, tmp) {
 				spdk_bdev_unregister(lvol->bdev, _vbdev_lvs_remove_bdev_unregistered_cb, lvs_bdev);