diff --git a/include/spdk/reduce.h b/include/spdk/reduce.h index 21f19f568..b4b846d49 100644 --- a/include/spdk/reduce.h +++ b/include/spdk/reduce.h @@ -70,21 +70,17 @@ struct spdk_reduce_vol_params { uint32_t chunk_size; /** - * Total size in bytes of the compressed volume. Must be - * an even multiple of chunk_size. Must be greater than 0. + * Total size in bytes of the compressed volume. During + * initialization, the size is calculated from the size of + * backing device size, so this must be set to 0 in the + * structure passed to spdk_reduce_vol_init(). After + * initialization, or a successful load, this field will + * contain the total size which will be an even multiple + * of the chunk size. */ uint64_t vol_size; }; -/** - * Get the required size for the pm file for a compressed volume. - * - * \param params Parameters for the compressed volume - * \return Size of the required pm file (in bytes) needed to create the - * compressed volume. Returns -EINVAL if params is invalid. - */ -int64_t spdk_reduce_get_pm_file_size(struct spdk_reduce_vol_params *params); - /** * Get the required size for the backing device for a compressed volume. * diff --git a/lib/reduce/reduce.c b/lib/reduce/reduce.c index 43e714ffe..5204c7d30 100644 --- a/lib/reduce/reduce.c +++ b/lib/reduce/reduce.c @@ -188,8 +188,16 @@ _reduce_vol_get_chunk_map(struct spdk_reduce_vol *vol, uint64_t chunk_map_index) static int _validate_vol_params(struct spdk_reduce_vol_params *params) { - if (params->vol_size == 0 || params->chunk_size == 0 || - params->backing_io_unit_size == 0 || params->logical_block_size == 0) { + if (params->vol_size > 0) { + /** + * User does not pass in the vol size - it gets calculated by libreduce from + * values in this structure plus the size of the backing device. + */ + return -EINVAL; + } + + if (params->chunk_size == 0 || params->backing_io_unit_size == 0 || + params->logical_block_size == 0) { return -EINVAL; } @@ -203,24 +211,27 @@ _validate_vol_params(struct spdk_reduce_vol_params *params) return -1; } - /* Volume size must be an even multiple of the chunk size. */ - if ((params->vol_size % params->chunk_size) != 0) { - return -EINVAL; - } - return 0; } -int64_t -spdk_reduce_get_pm_file_size(struct spdk_reduce_vol_params *params) +static uint64_t +_get_vol_size(uint64_t chunk_size, uint64_t backing_dev_size) +{ + uint64_t num_chunks; + + num_chunks = backing_dev_size / chunk_size; + if (num_chunks <= REDUCE_NUM_EXTRA_CHUNKS) { + return 0; + } + + num_chunks -= REDUCE_NUM_EXTRA_CHUNKS; + return num_chunks * chunk_size; +} + +static uint64_t +_get_pm_file_size(struct spdk_reduce_vol_params *params) { uint64_t total_pm_size; - int rc; - - rc = _validate_vol_params(params); - if (rc != 0) { - return rc; - } total_pm_size = sizeof(struct spdk_reduce_vol_superblock); total_pm_size += _get_pm_logical_map_size(params->vol_size, params->chunk_size); @@ -229,24 +240,6 @@ spdk_reduce_get_pm_file_size(struct spdk_reduce_vol_params *params) return total_pm_size; } -int64_t -spdk_reduce_get_backing_device_size(struct spdk_reduce_vol_params *params) -{ - uint64_t total_backing_size, num_chunks; - int rc; - - rc = _validate_vol_params(params); - if (rc != 0) { - return rc; - } - - num_chunks = _get_total_chunks(params->vol_size, params->chunk_size); - total_backing_size = num_chunks * params->chunk_size; - total_backing_size += sizeof(struct spdk_reduce_vol_superblock); - - return total_backing_size; -} - const struct spdk_uuid * spdk_reduce_vol_get_uuid(struct spdk_reduce_vol *vol) { @@ -396,7 +389,7 @@ spdk_reduce_vol_init(struct spdk_reduce_vol_params *params, { struct spdk_reduce_vol *vol; struct reduce_init_load_ctx *init_ctx; - int64_t size, size_needed; + uint64_t backing_dev_size; size_t mapped_len; int dir_len, max_dir_len, rc; @@ -424,11 +417,10 @@ spdk_reduce_vol_init(struct spdk_reduce_vol_params *params, return; } - size_needed = spdk_reduce_get_backing_device_size(params); - size = backing_dev->blockcnt * backing_dev->blocklen; - if (size_needed > size) { - SPDK_ERRLOG("backing device size %" PRIi64 " but %" PRIi64 " needed\n", - size, size_needed); + backing_dev_size = backing_dev->blockcnt * backing_dev->blocklen; + params->vol_size = _get_vol_size(params->chunk_size, backing_dev_size); + if (params->vol_size == 0) { + SPDK_ERRLOG("backing device is too small\n"); cb_fn(cb_arg, NULL, -EINVAL); return; } @@ -475,7 +467,7 @@ spdk_reduce_vol_init(struct spdk_reduce_vol_params *params, 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.size = _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); @@ -547,7 +539,7 @@ _load_read_super_and_path_cpl(void *cb_arg, int reduce_errno) { struct reduce_init_load_ctx *load_ctx = cb_arg; struct spdk_reduce_vol *vol = load_ctx->vol; - int64_t size, size_needed; + uint64_t backing_dev_size; size_t mapped_len; int rc; @@ -569,17 +561,16 @@ _load_read_super_and_path_cpl(void *cb_arg, int reduce_errno) goto error; } - size_needed = spdk_reduce_get_backing_device_size(&vol->params); - size = vol->backing_dev->blockcnt * vol->backing_dev->blocklen; - if (size_needed > size) { - SPDK_ERRLOG("backing device size %" PRIi64 " but %" PRIi64 " expected\n", - size, size_needed); + backing_dev_size = vol->backing_dev->blockcnt * vol->backing_dev->blocklen; + if (_get_vol_size(vol->params.chunk_size, backing_dev_size) < vol->params.vol_size) { + SPDK_ERRLOG("backing device size %" PRIi64 " smaller than expected\n", + backing_dev_size); rc = -EILSEQ; goto error; } memcpy(vol->pm_file.path, load_ctx->path, sizeof(vol->pm_file.path)); - vol->pm_file.size = spdk_reduce_get_pm_file_size(&vol->params); + vol->pm_file.size = _get_pm_file_size(&vol->params); vol->pm_file.pm_buf = pmem_map_file(vol->pm_file.path, 0, 0, 0, &mapped_len, &vol->pm_file.pm_is_pmem); if (vol->pm_file.pm_buf == NULL) { diff --git a/test/unit/lib/reduce/reduce.c/reduce_ut.c b/test/unit/lib/reduce/reduce.c/reduce_ut.c index c886efdb4..4e51bee22 100644 --- a/test/unit/lib/reduce/reduce.c/reduce_ut.c +++ b/test/unit/lib/reduce/reduce.c/reduce_ut.c @@ -76,30 +76,13 @@ static void get_pm_file_size(void) { struct spdk_reduce_vol_params params; - int64_t pm_size, expected_pm_size; + uint64_t pm_size, expected_pm_size; - params.vol_size = 0; - params.chunk_size = 0; - params.backing_io_unit_size = 0; - CU_ASSERT(spdk_reduce_get_pm_file_size(¶ms) == -EINVAL); - - /* - * Select a valid backing_io_unit_size. This should still fail since - * vol_size and chunk_size are still 0. - */ params.backing_io_unit_size = 4096; - CU_ASSERT(spdk_reduce_get_pm_file_size(¶ms) == -EINVAL); - - /* - * Select a valid chunk_size. This should still fail since val_size - * is still 0. - */ params.chunk_size = 4096 * 4; - CU_ASSERT(spdk_reduce_get_pm_file_size(¶ms) == -EINVAL); - - /* Select a valid vol_size. This should return a proper pm_size. */ params.vol_size = 4096 * 4 * 100; - pm_size = spdk_reduce_get_pm_file_size(¶ms); + + pm_size = _get_pm_file_size(¶ms); expected_pm_size = sizeof(struct spdk_reduce_vol_superblock); /* 100 chunks in logical map * 8 bytes per chunk */ expected_pm_size += 100 * sizeof(uint64_t); @@ -116,43 +99,13 @@ get_pm_file_size(void) } static void -get_backing_device_size(void) +get_vol_size(void) { - struct spdk_reduce_vol_params params; - int64_t backing_size, expected_backing_size; + uint64_t chunk_size, backing_dev_size; - params.vol_size = 0; - params.chunk_size = 0; - params.backing_io_unit_size = 0; - params.logical_block_size = 512; - CU_ASSERT(spdk_reduce_get_backing_device_size(¶ms) == -EINVAL); - - /* - * Select a valid backing_io_unit_size. This should still fail since - * vol_size and chunk_size are still 0. - */ - params.backing_io_unit_size = 4096; - CU_ASSERT(spdk_reduce_get_backing_device_size(¶ms) == -EINVAL); - - /* - * Select a valid chunk_size. This should still fail since val_size - * is still 0. - */ - params.chunk_size = 4096 * 4; - CU_ASSERT(spdk_reduce_get_backing_device_size(¶ms) == -EINVAL); - - /* Select a valid vol_size. This should return a proper backing device size. */ - params.vol_size = 4096 * 4 * 100; - backing_size = spdk_reduce_get_backing_device_size(¶ms); - expected_backing_size = params.vol_size; - /* reduce allocates some extra chunks too for in-flight writes when logical map - * is full. REDUCE_EXTRA_CHUNKS is a private #ifdef in reduce.c. Backing device - * must have space allocated for these extra chunks. - */ - expected_backing_size += REDUCE_NUM_EXTRA_CHUNKS * params.chunk_size; - /* Account for superblock as well. */ - expected_backing_size += sizeof(struct spdk_reduce_vol_superblock); - CU_ASSERT(backing_size == expected_backing_size); + chunk_size = 16 * 1024; + backing_dev_size = 16 * 1024 * 1000; + CU_ASSERT(_get_vol_size(chunk_size, backing_dev_size) < backing_dev_size); } void * @@ -225,13 +178,17 @@ init_failure(void) struct spdk_reduce_backing_dev backing_dev = {}; backing_dev.blocklen = 512; + /* This blockcnt is too small for a reduce vol - there needs to be + * enough space for at least REDUCE_NUM_EXTRA_CHUNKS + 1 chunks. + */ + backing_dev.blockcnt = 20; - params.vol_size = 1024 * 1024; /* 1MB */ + params.vol_size = 0; params.chunk_size = 16 * 1024; params.backing_io_unit_size = backing_dev.blocklen; params.logical_block_size = 512; - /* backing_dev and pm_file have an invalid size. This should fail. */ + /* backing_dev has an invalid size. This should fail. */ g_vol = NULL; g_reduce_errno = 0; spdk_reduce_vol_init(¶ms, &backing_dev, TEST_MD_PATH, init_cb, NULL); @@ -241,7 +198,7 @@ init_failure(void) /* 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; + backing_dev.blockcnt = 20000; g_vol = NULL; g_reduce_errno = 0; @@ -313,7 +270,7 @@ backing_dev_init(struct spdk_reduce_backing_dev *backing_dev, struct spdk_reduce { int64_t size; - size = spdk_reduce_get_backing_device_size(params); + size = 4 * 1024 * 1024; backing_dev->blocklen = backing_blocklen; backing_dev->blockcnt = size / backing_dev->blocklen; backing_dev->readv = backing_dev_readv; @@ -334,7 +291,6 @@ init_md(void) struct spdk_uuid uuid; uint64_t *entry; - params.vol_size = 1024 * 1024; /* 1MB */ params.chunk_size = 16 * 1024; params.backing_io_unit_size = 512; params.logical_block_size = 512; @@ -386,7 +342,6 @@ _init_backing_dev(uint32_t backing_blocklen) struct spdk_reduce_vol_params *persistent_params; struct spdk_reduce_backing_dev backing_dev = {}; - params.vol_size = 1024 * 1024; /* 1MB */ params.chunk_size = 16 * 1024; params.backing_io_unit_size = 512; params.logical_block_size = 512; @@ -437,7 +392,6 @@ _load(uint32_t backing_blocklen) struct spdk_reduce_backing_dev backing_dev = {}; char pmem_file_path[REDUCE_PATH_MAX]; - params.vol_size = 1024 * 1024; /* 1MB */ params.chunk_size = 16 * 1024; params.backing_io_unit_size = 512; params.logical_block_size = 512; @@ -521,7 +475,6 @@ _write_maps(uint32_t backing_blocklen) uint64_t old_chunk0_map_index, new_chunk0_map_index; uint64_t *old_chunk0_map, *new_chunk0_map; - params.vol_size = 1024 * 1024; /* 1MB */ params.chunk_size = 16 * 1024; params.backing_io_unit_size = 4096; params.logical_block_size = 512; @@ -613,7 +566,6 @@ _read_write(uint32_t backing_blocklen) char compare_buf[16 * 1024]; uint32_t i; - params.vol_size = 1024 * 1024; /* 1MB */ params.chunk_size = 16 * 1024; params.backing_io_unit_size = 4096; params.logical_block_size = 512; @@ -701,7 +653,7 @@ main(int argc, char **argv) if ( CU_add_test(suite, "get_pm_file_size", get_pm_file_size) == NULL || - CU_add_test(suite, "get_backing_device_size", get_backing_device_size) == NULL || + CU_add_test(suite, "get_vol_size", get_vol_size) == NULL || CU_add_test(suite, "init_failure", init_failure) == NULL || CU_add_test(suite, "init_md", init_md) == NULL || CU_add_test(suite, "init_backing_dev", init_backing_dev) == NULL ||