Commit d31f2be5 authored by Jim Harris's avatar Jim Harris
Browse files

reduce: have library create pmem file 



Currently the API requires the caller to open the
pmem file and pass the mapped buffer pointer, length
and pmem flag to libreduce using the spdk_reduce_pm_file
structure.

Let's have the library just do the pmem_map_file() instead.
Users then just pass the path to create the pmem file.  The
library will still use the spdk_reduce_pm_file structure
internally - so move its definition out of the public header
and into reduce.c.

Signed-off-by: default avatarJim Harris <james.r.harris@intel.com>
Change-Id: I81fcbfdfbb3211dca016d6aa422cf2e1ab16d84d

Reviewed-on: https://review.gerrithub.io/432593


Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
parent e1ec5c60

include/spdk/reduce.h

+4 −19
Original line number Diff line number Diff line
@@ -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);

lib/reduce/reduce.c

+56 −9
Original line number Diff line number Diff line
@@ -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(&params->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,

			    &params->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, &params->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);

test/unit/lib/reduce/reduce.c/reduce_ut.c

+51 −54
Original line number Diff line number Diff line
@@ -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(&params, &backing_dev, &pm_file, init_cb, NULL);

	spdk_reduce_vol_init(&params, &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(&params) / backing_dev.blocklen;



	g_vol = NULL;

	g_ziperrno = 0;

	spdk_reduce_vol_init(&params, &backing_dev, &pm_file, init_cb, NULL);

	spdk_reduce_vol_init(&params, &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, &params);



	g_vol = NULL;

	g_ziperrno = 0;

	spdk_reduce_vol_init(&params, &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, &params);

	pm_file_init(&pm_file, &params);



	g_vol = NULL;

	g_ziperrno = -1;

	spdk_reduce_vol_init(&params, &backing_dev, &pm_file, init_cb, NULL);

	spdk_reduce_vol_init(&params, &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, &params, sizeof(params)) == 0);



	/* Check that the pm file path was constructed correctly.  It should be in

	 * the form:

	 * TEST_MD_PATH + "/" + <uuid string>

	 */

	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(&params.uuid);



	backing_dev_init(&backing_dev, &params);

	pm_file_init(&pm_file, &params);



	g_vol = NULL;

	g_path = NULL;

	g_ziperrno = -1;

	spdk_reduce_vol_init(&params, &backing_dev, &pm_file, init_cb, NULL);

	spdk_reduce_vol_init(&params, &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);

}