diff --git a/include/spdk/reduce.h b/include/spdk/reduce.h index 5a1a154a0..cd6ae706c 100644 --- a/include/spdk/reduce.h +++ b/include/spdk/reduce.h @@ -88,23 +88,6 @@ int64_t spdk_reduce_get_backing_device_size(struct spdk_reduce_vol_params *param struct spdk_reduce_vol; -#define REDUCE_PATH_MAX 4096 - -/** - * Describes a persistent memory file used to hold metadata associated with a - * compressed volume. - */ -struct spdk_reduce_pm_file { - char path[REDUCE_PATH_MAX]; - void *pm_buf; - bool pm_is_pmem; - - /** Size of the persistent memory file in bytes. */ - uint64_t size; - - void (*close)(struct spdk_reduce_pm_file *); -}; - typedef void (*spdk_reduce_vol_op_complete)(void *ctx, int ziperrno); typedef void (*spdk_reduce_vol_op_with_handle_complete)(void *ctx, struct spdk_reduce_vol *vol, @@ -146,13 +129,15 @@ const struct spdk_uuid *spdk_reduce_vol_get_uuid(struct spdk_reduce_vol *vol); * * \param params Parameters for the new volume. * \param backing_dev Structure describing the backing device to use for the new volume. - * \param pm_file Structure describing the persistent memory file to use for the new volume. + * \param pm_file_dir Directory to use for creation of the persistent memory file to + * use for the new volume. This function will append the UUID as + * the filename to create in this directory. * \param cb_fn Callback function to signal completion of the initialization process. * \param cb_arg Argument to pass to the callback function. */ void spdk_reduce_vol_init(struct spdk_reduce_vol_params *params, struct spdk_reduce_backing_dev *backing_dev, - struct spdk_reduce_pm_file *pm_file, + const char *pm_file_dir, spdk_reduce_vol_op_with_handle_complete cb_fn, void *cb_arg); diff --git a/lib/reduce/reduce.c b/lib/reduce/reduce.c index afc47d3e2..e5d4e8adc 100644 --- a/lib/reduce/reduce.c +++ b/lib/reduce/reduce.c @@ -56,6 +56,19 @@ struct spdk_reduce_vol_superblock { }; SPDK_STATIC_ASSERT(sizeof(struct spdk_reduce_vol_superblock) == 4096, "size incorrect"); +#define REDUCE_PATH_MAX 4096 + +/** + * Describes a persistent memory file used to hold metadata associated with a + * compressed volume. + */ +struct spdk_reduce_pm_file { + char path[REDUCE_PATH_MAX]; + void *pm_buf; + int pm_is_pmem; + uint64_t size; +}; + struct spdk_reduce_vol { struct spdk_uuid uuid; struct spdk_reduce_pm_file pm_file; @@ -211,14 +224,26 @@ _init_write_path_cpl(void *cb_arg, int ziperrno) void spdk_reduce_vol_init(struct spdk_reduce_vol_params *params, struct spdk_reduce_backing_dev *backing_dev, - struct spdk_reduce_pm_file *pm_file, + const char *pm_file_dir, spdk_reduce_vol_op_with_handle_complete cb_fn, void *cb_arg) { struct spdk_reduce_vol *vol; struct spdk_reduce_vol_superblock *pm_super; struct reduce_init_load_ctx *init_ctx; int64_t size, size_needed; - int rc; + size_t mapped_len; + int dir_len, max_dir_len, rc; + + /* We need to append a path separator and the UUID to the supplied + * path. + */ + max_dir_len = REDUCE_PATH_MAX - SPDK_UUID_STRING_LEN - 1; + dir_len = strnlen(pm_file_dir, max_dir_len); + if (dir_len == max_dir_len) { + SPDK_ERRLOG("pm_file_dir (%s) too long\n", pm_file_dir); + cb_fn(cb_arg, NULL, -EINVAL); + return; + } rc = _validate_vol_params(params); if (rc != 0) { @@ -236,8 +261,6 @@ spdk_reduce_vol_init(struct spdk_reduce_vol_params *params, return; } - size_needed = spdk_reduce_get_pm_file_size(params); - size = pm_file->size; if (size_needed > size) { SPDK_ERRLOG("pm file size %" PRIi64 " but %" PRIi64 " needed\n", size, size_needed); @@ -258,8 +281,6 @@ spdk_reduce_vol_init(struct spdk_reduce_vol_params *params, return; } - memcpy(&vol->pm_file, pm_file, sizeof(*pm_file)); - vol->backing_super = spdk_dma_zmalloc(sizeof(*vol->backing_super), 0, NULL); if (vol->backing_super == NULL) { free(vol); @@ -288,6 +309,34 @@ spdk_reduce_vol_init(struct spdk_reduce_vol_params *params, spdk_uuid_generate(¶ms->uuid); } + memcpy(vol->pm_file.path, pm_file_dir, dir_len); + vol->pm_file.path[dir_len] = '/'; + spdk_uuid_fmt_lower(&vol->pm_file.path[dir_len + 1], SPDK_UUID_STRING_LEN, + ¶ms->uuid); + vol->pm_file.size = spdk_reduce_get_pm_file_size(params); + vol->pm_file.pm_buf = pmem_map_file(vol->pm_file.path, vol->pm_file.size, + PMEM_FILE_CREATE | PMEM_FILE_EXCL, 0600, + &mapped_len, &vol->pm_file.pm_is_pmem); + if (vol->pm_file.pm_buf == NULL) { + SPDK_ERRLOG("could not pmem_map_file(%s): %s\n", + vol->pm_file.path, strerror(errno)); + free(init_ctx); + spdk_dma_free(vol->backing_super); + free(vol); + cb_fn(cb_arg, NULL, -errno); + return; + } + + if (vol->pm_file.size != mapped_len) { + SPDK_ERRLOG("could not map entire pmem file (size=%" PRIu64 " mapped=%" PRIu64 ")\n", + vol->pm_file.size, mapped_len); + free(init_ctx); + spdk_dma_free(vol->backing_super); + free(vol); + cb_fn(cb_arg, NULL, -ENOMEM); + return; + } + memcpy(&vol->uuid, ¶ms->uuid, sizeof(params->uuid)); vol->backing_dev = backing_dev; @@ -335,9 +384,7 @@ spdk_reduce_vol_unload(struct spdk_reduce_vol *vol, return; } - if (vol->pm_file.close != NULL) { - vol->pm_file.close(&vol->pm_file); - } + pmem_unmap(vol->pm_file.pm_buf, vol->pm_file.size); vol->backing_dev->close(vol->backing_dev); diff --git a/test/unit/lib/reduce/reduce.c/reduce_ut.c b/test/unit/lib/reduce/reduce.c/reduce_ut.c index 31345f625..985fe6ac2 100644 --- a/test/unit/lib/reduce/reduce.c/reduce_ut.c +++ b/test/unit/lib/reduce/reduce.c/reduce_ut.c @@ -42,11 +42,13 @@ static struct spdk_reduce_vol *g_vol; static int g_ziperrno; static char *g_volatile_pm_buf; +static size_t g_volatile_pm_buf_len; static char *g_persistent_pm_buf; static bool g_backing_dev_closed; static char *g_backing_dev_buf; +static const char *g_path; -#define TEST_MD_PATH "/tmp/meta.data" +#define TEST_MD_PATH "/tmp" static void sync_pm_buf(const void *addr, size_t length) @@ -151,43 +153,45 @@ get_backing_device_size(void) CU_ASSERT(backing_size == expected_backing_size); } -static void -pm_file_close(struct spdk_reduce_pm_file *pm_file) +void * +pmem_map_file(const char *path, size_t len, int flags, mode_t mode, + size_t *mapped_lenp, int *is_pmemp) { + CU_ASSERT(g_volatile_pm_buf == NULL); + g_volatile_pm_buf = calloc(1, len); + g_volatile_pm_buf_len = len; + g_path = path; + SPDK_CU_ASSERT_FATAL(g_volatile_pm_buf != NULL); + *mapped_lenp = len; + *is_pmemp = 1; + + if (g_persistent_pm_buf == NULL) { + g_persistent_pm_buf = calloc(1, len); + SPDK_CU_ASSERT_FATAL(g_persistent_pm_buf != NULL); + } + + return g_volatile_pm_buf; +} + +int +pmem_unmap(void *addr, size_t len) +{ + CU_ASSERT(addr == g_volatile_pm_buf); + CU_ASSERT(len == g_volatile_pm_buf_len); free(g_volatile_pm_buf); g_volatile_pm_buf = NULL; + + return 0; } static void -pm_file_destroy(void) +persistent_pm_buf_destroy(void) { CU_ASSERT(g_persistent_pm_buf != NULL); free(g_persistent_pm_buf); g_persistent_pm_buf = NULL; } -static int -pm_file_init(struct spdk_reduce_pm_file *pm_file, struct spdk_reduce_vol_params *params) -{ - pm_file->size = spdk_reduce_get_pm_file_size(params); - - CU_ASSERT(g_persistent_pm_buf == NULL); - g_persistent_pm_buf = calloc(1, pm_file->size); - SPDK_CU_ASSERT_FATAL(g_persistent_pm_buf != NULL); - - CU_ASSERT(g_volatile_pm_buf == NULL); - g_volatile_pm_buf = calloc(1, pm_file->size); - SPDK_CU_ASSERT_FATAL(g_volatile_pm_buf != NULL); - - memcpy(pm_file->path, TEST_MD_PATH, strlen(TEST_MD_PATH)); - pm_file->path[strlen(TEST_MD_PATH)] = '\0'; - - pm_file->pm_buf = g_volatile_pm_buf; - pm_file->close = pm_file_close; - - return 0; -} - static void init_cb(void *cb_arg, struct spdk_reduce_vol *vol, int ziperrno) { @@ -206,7 +210,6 @@ init_failure(void) { struct spdk_reduce_vol_params params = {}; struct spdk_reduce_backing_dev backing_dev = {}; - struct spdk_reduce_pm_file pm_file = {}; backing_dev.blocklen = 512; @@ -217,34 +220,20 @@ init_failure(void) /* backing_dev and pm_file have an invalid size. This should fail. */ g_vol = NULL; g_ziperrno = 0; - spdk_reduce_vol_init(¶ms, &backing_dev, &pm_file, init_cb, NULL); + spdk_reduce_vol_init(¶ms, &backing_dev, TEST_MD_PATH, init_cb, NULL); CU_ASSERT(g_ziperrno == -EINVAL); SPDK_CU_ASSERT_FATAL(g_vol == NULL); - /* backing_dev now has valid size, but pm_file is still invalid. - * This should fail. + /* backing_dev now has valid size, but backing_dev still has null + * function pointers. This should fail. */ backing_dev.blockcnt = spdk_reduce_get_backing_device_size(¶ms) / backing_dev.blocklen; g_vol = NULL; g_ziperrno = 0; - spdk_reduce_vol_init(¶ms, &backing_dev, &pm_file, init_cb, NULL); + spdk_reduce_vol_init(¶ms, &backing_dev, TEST_MD_PATH, init_cb, NULL); CU_ASSERT(g_ziperrno == -EINVAL); SPDK_CU_ASSERT_FATAL(g_vol == NULL); - - /* pm_file now has valid size, but backing_dev still has null function - * pointers. This should fail. - */ - pm_file_init(&pm_file, ¶ms); - - g_vol = NULL; - g_ziperrno = 0; - spdk_reduce_vol_init(¶ms, &backing_dev, &pm_file, init_cb, NULL); - CU_ASSERT(g_ziperrno == -EINVAL); - SPDK_CU_ASSERT_FATAL(g_vol == NULL); - - pm_file_close(&pm_file); - pm_file_destroy(); } static void @@ -327,18 +316,17 @@ init_md(void) struct spdk_reduce_vol_params params = {}; struct spdk_reduce_vol_params *persistent_params; struct spdk_reduce_backing_dev backing_dev = {}; - struct spdk_reduce_pm_file pm_file = {}; + struct spdk_uuid uuid; params.vol_size = 1024 * 1024; /* 1MB */ params.chunk_size = 16 * 1024; params.backing_io_unit_size = 512; backing_dev_init(&backing_dev, ¶ms); - pm_file_init(&pm_file, ¶ms); g_vol = NULL; g_ziperrno = -1; - spdk_reduce_vol_init(¶ms, &backing_dev, &pm_file, init_cb, NULL); + spdk_reduce_vol_init(¶ms, &backing_dev, TEST_MD_PATH, init_cb, NULL); CU_ASSERT(g_ziperrno == 0); SPDK_CU_ASSERT_FATAL(g_vol != NULL); /* Confirm that reduce persisted the params to metadata. */ @@ -346,6 +334,15 @@ init_md(void) persistent_params = (struct spdk_reduce_vol_params *)(g_persistent_pm_buf + 8); CU_ASSERT(memcmp(persistent_params, ¶ms, sizeof(params)) == 0); + /* Check that the pm file path was constructed correctly. It should be in + * the form: + * TEST_MD_PATH + "/" + + */ + CU_ASSERT(strncmp(&g_path[0], TEST_MD_PATH, strlen(TEST_MD_PATH)) == 0); + CU_ASSERT(g_path[strlen(TEST_MD_PATH)] == '/'); + CU_ASSERT(spdk_uuid_parse(&uuid, &g_path[strlen(TEST_MD_PATH) + 1]) == 0); + CU_ASSERT(spdk_uuid_compare(&uuid, spdk_reduce_vol_get_uuid(g_vol)) == 0); + g_ziperrno = -1; g_backing_dev_closed = false; spdk_reduce_vol_unload(g_vol, unload_cb, NULL); @@ -353,7 +350,7 @@ init_md(void) CU_ASSERT(g_backing_dev_closed == true); CU_ASSERT(g_volatile_pm_buf == NULL); - pm_file_destroy(); + persistent_pm_buf_destroy(); backing_dev_destroy(&backing_dev); } @@ -363,7 +360,6 @@ init_backing_dev(void) struct spdk_reduce_vol_params params = {}; struct spdk_reduce_vol_params *persistent_params; struct spdk_reduce_backing_dev backing_dev = {}; - struct spdk_reduce_pm_file pm_file = {}; params.vol_size = 1024 * 1024; /* 1MB */ params.chunk_size = 16 * 1024; @@ -371,13 +367,14 @@ init_backing_dev(void) spdk_uuid_generate(¶ms.uuid); backing_dev_init(&backing_dev, ¶ms); - pm_file_init(&pm_file, ¶ms); g_vol = NULL; + g_path = NULL; g_ziperrno = -1; - spdk_reduce_vol_init(¶ms, &backing_dev, &pm_file, init_cb, NULL); + spdk_reduce_vol_init(¶ms, &backing_dev, TEST_MD_PATH, init_cb, NULL); CU_ASSERT(g_ziperrno == 0); SPDK_CU_ASSERT_FATAL(g_vol != NULL); + SPDK_CU_ASSERT_FATAL(g_path != NULL); /* Confirm that libreduce persisted the params to the backing device. */ CU_ASSERT(memcmp(g_backing_dev_buf, SPDK_REDUCE_SIGNATURE, 8) == 0); persistent_params = (struct spdk_reduce_vol_params *)(g_backing_dev_buf + 8); @@ -386,7 +383,7 @@ init_backing_dev(void) /* Confirm that the path to the persistent memory metadata file was persisted to * the backing device. */ - CU_ASSERT(strncmp(TEST_MD_PATH, + CU_ASSERT(strncmp(g_path, g_backing_dev_buf + REDUCE_BACKING_DEV_PATH_OFFSET, REDUCE_PATH_MAX) == 0); @@ -396,7 +393,7 @@ init_backing_dev(void) CU_ASSERT(g_ziperrno == 0); CU_ASSERT(g_backing_dev_closed == true); - pm_file_destroy(); + persistent_pm_buf_destroy(); backing_dev_destroy(&backing_dev); }