ivampiresp/Spdk
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

lvol: do not open esnaps during bs_load

Browse Source 
As an lvstore is opening it calls spdk_bs_load(), which briefly opens
each blob and has no use for external snapshots. Since there is no point
in opening them at this time, don't open them. Once the blobstore has
been loaded, update lvs->load_esnaps so that external snapshots are
opened as the lvols open their blobs.

Change-Id: Ib16c8474300ff4b106aad0baa5b8b38332c23b01
Signed-off-by: Mike Gerdts <mgerdts@nvidia.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/16424
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
Reviewed-by: Jim Harris <james.r.harris@intel.com>
This commit is contained in:
Mike Gerdts 2022-10-10 15:19:49 -05:00 committed by Jim Harris
parent 609205739e
commit ae0b53b1b6
5 changed files with 160 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
include/spdk/blob.h
View File

@ -132,6 +132,9 @@ typedef void (*spdk_blob_op_with_bs_dev)(void *cb_arg, struct spdk_bs_dev *bs_de
* callback registered with the blobstore to create the external snapshot device. The blobstore
* consumer must set this while loading the blobstore if it intends to support external snapshots.
*
* If the blobstore consumer does not wish to load an external snapshot, it should set *bs_dev to
* NULL and return 0.
*
* \param bs_ctx Context provided by the blobstore consumer via esnap_ctx member of struct
* spdk_bs_opts.
* \param blob_ctx Context provided to spdk_bs_open_ext() via esnap_ctx member of struct

3
include/spdk_internal/lvolstore.h
View File

@ -1,6 +1,7 @@
/* SPDX-License-Identifier: BSD-3-Clause
* Copyright (C) 2017 Intel Corporation.
* All rights reserved.
* Copyright (c) 2022-2023 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
*/
#ifndef SPDK_INTERNAL_LVOLSTORE_H
@ -74,10 +75,12 @@ struct spdk_lvol_store {
TAILQ_HEAD(, spdk_lvol) lvols;
TAILQ_HEAD(, spdk_lvol) pending_lvols;
TAILQ_HEAD(, spdk_lvol) retry_open_lvols;
bool load_esnaps;
bool on_list;
TAILQ_ENTRY(spdk_lvol_store) link;
char name[SPDK_LVS_NAME_MAX];
char new_name[SPDK_LVS_NAME_MAX];
spdk_bs_esnap_dev_create esnap_bs_dev_create;
};
struct spdk_lvol {

38
lib/blob/blobstore.c
View File

@ -372,6 +372,20 @@ blob_back_bs_destroy_esnap_done(void *ctx, struct spdk_blob *blob, int bserrno)
assert(false);
}
if (bs_dev == NULL) {
/*
* This check exists to make scanbuild happy.
*
* blob->back_bs_dev for an esnap is NULL during the first iteration of blobs while
* the blobstore is being loaded. It could also be NULL if there was an error
* opening the esnap device. In each of these cases, no channels could have been
* created because back_bs_dev->create_channel() would have led to a NULL pointer
* deref.
*/
assert(false);
return;
}
SPDK_DEBUGLOG(blob_esnap, "blob 0x%" PRIx64 ": calling destroy on back_bs_dev\n", blob->id);
bs_dev->destroy(bs_dev);
}
@ -1420,14 +1434,19 @@ blob_load_esnap(struct spdk_blob *blob, void *blob_ctx)
return rc;
}
assert(bs_dev != NULL);
SPDK_DEBUGLOG(blob_esnap, "blob 0x%" PRIx64 ": loaded back_bs_dev\n", blob->id);
if ((bs->io_unit_size % bs_dev->blocklen) != 0) {
SPDK_NOTICELOG("blob 0x%" PRIx64 " external snapshot device block size %u is not "
"compatible with blobstore block size %u\n",
blob->id, bs_dev->blocklen, bs->io_unit_size);
bs_dev->destroy(bs_dev);
return -EINVAL;
/*
* Note: bs_dev might be NULL if the consumer chose to not open the external snapshot.
* This especially might happen during spdk_bs_load() iteration.
*/
if (bs_dev != NULL) {
SPDK_DEBUGLOG(blob_esnap, "blob 0x%" PRIx64 ": loaded back_bs_dev\n", blob->id);
if ((bs->io_unit_size % bs_dev->blocklen) != 0) {
SPDK_NOTICELOG("blob 0x%" PRIx64 " external snapshot device block size %u "
"is not compatible with blobstore block size %u\n",
blob->id, bs_dev->blocklen, bs->io_unit_size);
bs_dev->destroy(bs_dev);
return -EINVAL;
}
}
blob->back_bs_dev = bs_dev;
@ -1447,7 +1466,6 @@ blob_load_backing_dev(spdk_bs_sequence_t *seq, void *cb_arg)
if (blob_is_esnap_clone(blob)) {
rc = blob_load_esnap(blob, seq->cpl.u.blob_handle.esnap_ctx);
assert((rc == 0) ^ (blob->back_bs_dev == NULL));
blob_load_final(ctx, rc);
return;
}
@ -8937,7 +8955,7 @@ blob_esnap_destroy_bs_dev_channels(struct spdk_blob *blob, bool abort_io,
{
struct blob_esnap_destroy_ctx *ctx;
if (!blob_is_esnap_clone(blob)) {
if (!blob_is_esnap_clone(blob) || blob->back_bs_dev == NULL) {
if (cb_fn != NULL) {
cb_fn(cb_arg, blob, 0);
}

42
lib/lvol/lvol.c
View File

@ -22,6 +22,9 @@ static TAILQ_HEAD(, spdk_lvol_store) g_lvol_stores = TAILQ_HEAD_INITIALIZER(g_lv
static pthread_mutex_t g_lvol_stores_mutex = PTHREAD_MUTEX_INITIALIZER;
static inline int lvs_opts_copy(const struct spdk_lvs_opts *src, struct spdk_lvs_opts *dst);
static int lvs_esnap_bs_dev_create(void *bs_ctx, void *blob_ctx, struct spdk_blob *blob,
const void *esnap_id, uint32_t id_len,
struct spdk_bs_dev **_bs_dev);
static int
add_lvs_to_list(struct spdk_lvol_store *lvs)
@ -59,6 +62,8 @@ lvs_alloc(void)
TAILQ_INIT(&lvs->pending_lvols);
TAILQ_INIT(&lvs->retry_open_lvols);
lvs->load_esnaps = false;
return lvs;
}
@ -188,6 +193,7 @@ load_next_lvol(void *cb_arg, struct spdk_blob *blob, int lvolerrno)
if (lvolerrno == -ENOENT) {
/* Finished iterating */
if (req->lvserrno == 0) {
lvs->load_esnaps = true;
req->cb_fn(req->cb_arg, lvs, req->lvserrno);
free(req);
} else {
@ -441,7 +447,8 @@ lvs_load(struct spdk_bs_dev *bs_dev, const struct spdk_lvs_opts *_lvs_opts,
snprintf(bs_opts.bstype.bstype, sizeof(bs_opts.bstype.bstype), "LVOLSTORE");
if (lvs_opts.esnap_bs_dev_create != NULL) {
bs_opts.esnap_bs_dev_create = lvs_opts.esnap_bs_dev_create;
req->lvol_store->esnap_bs_dev_create = lvs_opts.esnap_bs_dev_create;
bs_opts.esnap_bs_dev_create = lvs_esnap_bs_dev_create;
bs_opts.esnap_ctx = req->lvol_store;
}
@ -1687,3 +1694,36 @@ spdk_lvs_grow(struct spdk_bs_dev *bs_dev, spdk_lvs_op_with_handle_complete cb_fn
spdk_bs_grow(bs_dev, &opts, lvs_load_cb, req);
}
static int
lvs_esnap_bs_dev_create(void *bs_ctx, void *blob_ctx, struct spdk_blob *blob,
const void *esnap_id, uint32_t id_len,
struct spdk_bs_dev **bs_dev)
{
struct spdk_lvol_store *lvs = bs_ctx;
struct spdk_lvol *lvol = blob_ctx;
spdk_blob_id blob_id = spdk_blob_get_id(blob);
if (lvs == NULL) {
if (lvol == NULL) {
SPDK_ERRLOG("Blob 0x%" PRIx64 ": no lvs context nor lvol context\n",
blob_id);
return -EINVAL;
}
lvs = lvol->lvol_store;
}
/*
* When spdk_lvs_load() is called, it iterates through all blobs in its blobstore building
* up a list of lvols (lvs->lvols). During this initial iteration, each blob is opened,
* passed to load_next_lvol(), then closed. There is no need to open the external snapshot
* during this phase. Once the blobstore is loaded, lvs->load_esnaps is set to true so that
* future lvol opens cause the external snapshot to be loaded.
*/
if (!lvs->load_esnaps) {
*bs_dev = NULL;
return 0;
}
return lvs->esnap_bs_dev_create(lvs, lvol, blob, esnap_id, id_len, bs_dev);
}

89
test/unit/lib/lvol/lvol.c/lvol_ut.c
View File

@ -28,8 +28,6 @@
#define SPDK_BLOB_THIN_PROV (1ULL << 0)
DEFINE_STUB(spdk_blob_get_esnap_id, int,
(struct spdk_blob *blob, const void **id, size_t *len), -ENOTSUP);
DEFINE_STUB(spdk_bdev_get_name, const char *, (const struct spdk_bdev *bdev), NULL);
DEFINE_STUB(spdk_bdev_get_by_name, struct spdk_bdev *, (const char *name), NULL);
DEFINE_STUB(spdk_bdev_create_bs_dev_ro, int,
@ -496,6 +494,21 @@ spdk_bs_create_clone(struct spdk_blob_store *bs, spdk_blob_id blobid,
spdk_bs_create_blob_ext(bs, NULL, cb_fn, cb_arg);
}
static int g_spdk_blob_get_esnap_id_errno;
static bool g_spdk_blob_get_esnap_id_called;
static void *g_spdk_blob_get_esnap_id;
static size_t g_spdk_blob_get_esnap_id_len;
int
spdk_blob_get_esnap_id(struct spdk_blob *blob, const void **id, size_t *len)
{
g_spdk_blob_get_esnap_id_called = true;
if (g_spdk_blob_get_esnap_id_errno == 0) {
*id = g_spdk_blob_get_esnap_id;
*len = g_spdk_blob_get_esnap_id_len;
}
return g_spdk_blob_get_esnap_id_errno;
}
static void
lvol_store_op_with_handle_complete(void *cb_arg, struct spdk_lvol_store *lvol_store, int lvserrno)
{
@ -2152,13 +2165,16 @@ lvol_get_xattr(void)
free_dev(&dev);
}
struct spdk_bs_dev *g_esnap_bs_dev;
int g_esnap_bs_dev_errno = -ENOTSUP;
static int
ut_esnap_bs_dev_create(void *bs_ctx, void *blob_ctx, struct spdk_blob *blob,
const void *esnap_id, uint32_t id_len,
struct spdk_bs_dev **_bs_dev)
{
CU_ASSERT(false);
return -ENOTSUP;
*_bs_dev = g_esnap_bs_dev;
return g_esnap_bs_dev_errno;
}
static void
@ -2169,6 +2185,9 @@ lvol_esnap_reload(void)
struct spdk_lvs_opts opts;
int rc;
g_esnap_bs_dev = NULL;
g_esnap_bs_dev_errno = -ENOTSUP;
req = calloc(1, sizeof(*req));
SPDK_CU_ASSERT_FATAL(req != NULL);
@ -2345,6 +2364,67 @@ lvol_esnap_create_delete(void)
g_lvol_store = NULL;
}
static void
lvol_esnap_load_esnaps(void)
{
struct spdk_blob blob = { .id = 42 };
struct spdk_lvol_store *lvs;
struct spdk_lvol *lvol;
struct spdk_bs_dev *bs_dev = NULL;
struct spdk_bs_dev esnap_bs_dev = { 0 };
int rc;
uint64_t esnap_id = 42;
lvs = lvs_alloc();
SPDK_CU_ASSERT_FATAL(lvs != NULL);
lvs->esnap_bs_dev_create = ut_esnap_bs_dev_create;
lvol = lvol_alloc(lvs, __func__, true, LVOL_CLEAR_WITH_DEFAULT);
SPDK_CU_ASSERT_FATAL(lvol != NULL);
/* Handle missing bs_ctx and blob_ctx gracefully */
rc = lvs_esnap_bs_dev_create(NULL, NULL, &blob, &esnap_id, sizeof(esnap_id), &bs_dev);
CU_ASSERT(rc == -EINVAL);
/* Do not try to load external snapshot when load_esnaps is false */
g_spdk_blob_get_esnap_id_called = false;
bs_dev = NULL;
rc = lvs_esnap_bs_dev_create(lvs, lvol, &blob, &esnap_id, sizeof(esnap_id), &bs_dev);
CU_ASSERT(rc == 0);
CU_ASSERT(bs_dev == NULL);
CU_ASSERT(!g_spdk_blob_get_esnap_id_called);
/* Same, with only lvs */
bs_dev = NULL;
rc = lvs_esnap_bs_dev_create(lvs, NULL, &blob, &esnap_id, sizeof(esnap_id), &bs_dev);
CU_ASSERT(rc == 0);
CU_ASSERT(bs_dev == NULL);
CU_ASSERT(!g_spdk_blob_get_esnap_id_called);
/* Same, with only lvol */
bs_dev = NULL;
rc = lvs_esnap_bs_dev_create(NULL, lvol, &blob, &esnap_id, sizeof(esnap_id), &bs_dev);
CU_ASSERT(rc == 0);
CU_ASSERT(bs_dev == NULL);
CU_ASSERT(!g_spdk_blob_get_esnap_id_called);
/* Happy path */
g_esnap_bs_dev = &esnap_bs_dev;
g_esnap_bs_dev_errno = 0;
lvs->load_esnaps = true;
ut_spdk_bdev_create_bs_dev_ro = 0;
g_spdk_blob_get_esnap_id_errno = 0;
bs_dev = NULL;
rc = lvs_esnap_bs_dev_create(lvs, lvol, &blob, &esnap_id, sizeof(esnap_id), &bs_dev);
CU_ASSERT(rc == 0);
/* Clean up */
lvol_free(lvol);
lvs_free(lvs);
g_esnap_bs_dev = NULL;
g_esnap_bs_dev_errno = -ENOTSUP;
}
int
main(int argc, char **argv)
{
@ -2386,6 +2466,7 @@ main(int argc, char **argv)
CU_ADD_TEST(suite, lvol_esnap_reload);
CU_ADD_TEST(suite, lvol_esnap_create_bad_args);
CU_ADD_TEST(suite, lvol_esnap_create_delete);
CU_ADD_TEST(suite, lvol_esnap_load_esnaps);
allocate_threads(1);
set_thread(0);