bdev/lvol: remove lvs with lvols during application shutdown
Once bdev finish starts, bdev unregister is called on all unclaimed bdevs. This means that for lvs with at least one lvol present, there will be a corresponding bdev unregister. Yet the vbdev_lvol module does not attempt to unload the lvs, once last lvol from that lvs is unregistered. Leaving the base bdev for lvs claimed. This patch fixes that by using fini_start callback from bdev_module to mark when shutdown begins. After that last lvol unregistered on lvs will unload it. Expanded struct lvol_bdev to contain lvol_store_bdev. Closing the lvol will free spdk_lvol, so lvol->lvol_store cannot be accessed. Changed ut_lvol_destroy UT to ut_bdev_finish. Previous UT didn't really test vbdev_lvol_destroy, but 'hotremove' of the lvol bdev. In effect there is no hotremove of the lvol bdevs (only lvs bdev). spdk_bdev_unregister() can only be called from within vbdev_lvol, or during bdev module finish. This UT will now check the bdev module finish. Note that at this point lvs with no lvols will not trigger lvs unload. Next patches in series will introduce async fini_start, to allow for the unload. Signed-off-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com> Change-Id: I8f51e8c1fcfdc55a5d090a3bc84ccefda813aef8 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/9093 Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com> Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins <sys_sgci@intel.com> Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com> Reviewed-by: Jim Harris <james.r.harris@intel.com> Reviewed-by: Konrad Sztyber <konrad.sztyber@intel.com>
This commit is contained in:
parent
2aeee54686
commit
d2dd47433b
@ -44,12 +44,15 @@ static TAILQ_HEAD(, lvol_store_bdev) g_spdk_lvol_pairs = TAILQ_HEAD_INITIALIZER(
|
||||
g_spdk_lvol_pairs);
|
||||
|
||||
static int vbdev_lvs_init(void);
|
||||
static void vbdev_lvs_fini_start(void);
|
||||
static int vbdev_lvs_get_ctx_size(void);
|
||||
static void vbdev_lvs_examine(struct spdk_bdev *bdev);
|
||||
static bool g_shutdown_started = false;
|
||||
|
||||
static struct spdk_bdev_module g_lvol_if = {
|
||||
.name = "lvol",
|
||||
.module_init = vbdev_lvs_init,
|
||||
.fini_start = vbdev_lvs_fini_start,
|
||||
.examine_disk = vbdev_lvs_examine,
|
||||
.get_ctx_size = vbdev_lvs_get_ctx_size,
|
||||
|
||||
@ -548,10 +551,33 @@ struct vbdev_lvol_destroy_ctx {
|
||||
void *cb_arg;
|
||||
};
|
||||
|
||||
static void
|
||||
_vbdev_lvol_unregister_unload_lvs(void *cb_arg, int lvserrno)
|
||||
{
|
||||
struct lvol_bdev *lvol_bdev = cb_arg;
|
||||
struct lvol_store_bdev *lvs_bdev = lvol_bdev->lvs_bdev;
|
||||
|
||||
if (lvserrno != 0) {
|
||||
SPDK_INFOLOG(vbdev_lvol, "Lvol store removed with error: %d.\n", lvserrno);
|
||||
}
|
||||
|
||||
TAILQ_REMOVE(&g_spdk_lvol_pairs, lvs_bdev, lvol_stores);
|
||||
free(lvs_bdev);
|
||||
|
||||
spdk_bdev_destruct_done(&lvol_bdev->bdev, lvserrno);
|
||||
free(lvol_bdev);
|
||||
}
|
||||
|
||||
static void
|
||||
_vbdev_lvol_unregister_cb(void *ctx, int lvolerrno)
|
||||
{
|
||||
struct lvol_bdev *lvol_bdev = ctx;
|
||||
struct lvol_store_bdev *lvs_bdev = lvol_bdev->lvs_bdev;
|
||||
|
||||
if (g_shutdown_started && _vbdev_lvs_are_lvols_closed(lvs_bdev->lvs)) {
|
||||
spdk_lvs_unload(lvs_bdev->lvs, _vbdev_lvol_unregister_unload_lvs, lvol_bdev);
|
||||
return;
|
||||
}
|
||||
|
||||
spdk_bdev_destruct_done(&lvol_bdev->bdev, lvolerrno);
|
||||
free(lvol_bdev);
|
||||
@ -944,6 +970,7 @@ _create_lvol_disk(struct spdk_lvol *lvol, bool destroy)
|
||||
}
|
||||
|
||||
lvol_bdev->lvol = lvol;
|
||||
lvol_bdev->lvs_bdev = lvs_bdev;
|
||||
|
||||
bdev = &lvol_bdev->bdev;
|
||||
bdev->name = lvol->unique_id;
|
||||
@ -1203,6 +1230,12 @@ vbdev_lvs_init(void)
|
||||
return 0;
|
||||
}
|
||||
|
||||
static void
|
||||
vbdev_lvs_fini_start(void)
|
||||
{
|
||||
g_shutdown_started = true;
|
||||
}
|
||||
|
||||
static int
|
||||
vbdev_lvs_get_ctx_size(void)
|
||||
{
|
||||
|
@ -50,6 +50,7 @@ struct lvol_store_bdev {
|
||||
struct lvol_bdev {
|
||||
struct spdk_bdev bdev;
|
||||
struct spdk_lvol *lvol;
|
||||
struct lvol_store_bdev *lvs_bdev;
|
||||
};
|
||||
|
||||
int vbdev_lvs_create(const char *base_bdev_name, const char *name, uint32_t cluster_sz,
|
||||
|
@ -1040,7 +1040,7 @@ ut_lvol_rename(void)
|
||||
}
|
||||
|
||||
static void
|
||||
ut_lvol_destroy(void)
|
||||
ut_bdev_finish(void)
|
||||
{
|
||||
struct spdk_lvol_store *lvs;
|
||||
struct spdk_lvol *lvol;
|
||||
@ -1048,44 +1048,47 @@ ut_lvol_destroy(void)
|
||||
int sz = 10;
|
||||
int rc;
|
||||
|
||||
/* Lvol store is successfully created */
|
||||
rc = vbdev_lvs_create("bdev", "lvs", 0, LVS_CLEAR_WITH_UNMAP, lvol_store_op_with_handle_complete,
|
||||
NULL);
|
||||
/* Test creating lvs with two lvols. Delete first lvol explicitly,
|
||||
* then start bdev finish. This should unload the remaining lvol and
|
||||
* lvol store. */
|
||||
|
||||
rc = vbdev_lvs_create("bdev", "lvs", 0, LVS_CLEAR_WITH_UNMAP,
|
||||
lvol_store_op_with_handle_complete, NULL);
|
||||
CU_ASSERT(rc == 0);
|
||||
CU_ASSERT(g_lvserrno == 0);
|
||||
SPDK_CU_ASSERT_FATAL(g_lvol_store != NULL);
|
||||
CU_ASSERT(g_lvol_store->bs_dev != NULL);
|
||||
lvs = g_lvol_store;
|
||||
|
||||
/* Successful lvols create */
|
||||
g_lvolerrno = -1;
|
||||
rc = vbdev_lvol_create(lvs, "lvol", sz, false, LVOL_CLEAR_WITH_DEFAULT, vbdev_lvol_create_complete,
|
||||
NULL);
|
||||
rc = vbdev_lvol_create(lvs, "lvol", sz, false, LVOL_CLEAR_WITH_DEFAULT,
|
||||
vbdev_lvol_create_complete, NULL);
|
||||
SPDK_CU_ASSERT_FATAL(rc == 0);
|
||||
CU_ASSERT(g_lvol != NULL);
|
||||
CU_ASSERT(g_lvolerrno == 0);
|
||||
lvol = g_lvol;
|
||||
|
||||
g_lvolerrno = -1;
|
||||
rc = vbdev_lvol_create(lvs, "lvol2", sz, false, LVOL_CLEAR_WITH_DEFAULT, vbdev_lvol_create_complete,
|
||||
NULL);
|
||||
rc = vbdev_lvol_create(lvs, "lvol2", sz, false, LVOL_CLEAR_WITH_DEFAULT,
|
||||
vbdev_lvol_create_complete, NULL);
|
||||
SPDK_CU_ASSERT_FATAL(rc == 0);
|
||||
CU_ASSERT(g_lvol != NULL);
|
||||
CU_ASSERT(g_lvolerrno == 0);
|
||||
lvol2 = g_lvol;
|
||||
|
||||
/* Successful lvols destroy */
|
||||
/* Destroy explicitly first lvol */
|
||||
vbdev_lvol_destroy(lvol, lvol_store_op_complete, NULL);
|
||||
CU_ASSERT(g_lvol == NULL);
|
||||
CU_ASSERT(g_lvolerrno == 0);
|
||||
|
||||
/* Hot remove lvol bdev */
|
||||
/* Start bdev finish and unregister remaining lvol */
|
||||
vbdev_lvs_fini_start();
|
||||
CU_ASSERT(g_shutdown_started == true);
|
||||
spdk_bdev_unregister(lvol2->bdev, _spdk_bdev_unregister_cb, NULL);
|
||||
|
||||
/* Unload lvol store */
|
||||
vbdev_lvs_unload(lvs, lvol_store_op_complete, NULL);
|
||||
CU_ASSERT(g_lvserrno == 0);
|
||||
/* During shutdown, removal of last lvol should unload lvs */
|
||||
CU_ASSERT(g_lvol_store == NULL);
|
||||
CU_ASSERT(TAILQ_EMPTY(&g_spdk_lvol_pairs));
|
||||
|
||||
/* Revert module state back to normal */
|
||||
g_shutdown_started = false;
|
||||
}
|
||||
|
||||
static void
|
||||
@ -1456,7 +1459,7 @@ int main(int argc, char **argv)
|
||||
CU_ADD_TEST(suite, ut_vbdev_lvol_submit_request);
|
||||
CU_ADD_TEST(suite, ut_lvol_examine);
|
||||
CU_ADD_TEST(suite, ut_lvol_rename);
|
||||
CU_ADD_TEST(suite, ut_lvol_destroy);
|
||||
CU_ADD_TEST(suite, ut_bdev_finish);
|
||||
CU_ADD_TEST(suite, ut_lvs_rename);
|
||||
|
||||
CU_basic_set_mode(CU_BRM_VERBOSE);
|
||||
|
Loading…
Reference in New Issue
Block a user