Merge pull request #284 from mmichal10/prevent-cache-name-duplicate

Prevent cache name duplicate

inc/ocf_cache.h

@@ -107,18 +107,6 @@ struct ocf_cache_info {
  */
 ocf_volume_t ocf_cache_get_volume(ocf_cache_t cache);
 
-/**
- * @brief Set name of given cache object
- *
- * @param[in] cache Cache object
- * @param[in] src Source of Cache name
- * @param[in] src_size Size of src
- *
- * @retval 0 Success
- * @retval Non-zero Fail
- */
-int ocf_cache_set_name(ocf_cache_t cache, const char *src, size_t src_size);
-
 /**
  * @brief Get name of given cache object
  *

inc/ocf_err.h

@@ -116,6 +116,9 @@ typedef enum {
 
 	/** Invalid cache line size */
 	OCF_ERR_INVALID_CACHE_LINE_SIZE,
+
+	/** Invalid cache name loaded*/
+	OCF_ERR_CACHE_NAME_MISMATCH,
 } ocf_error_t;
 
 #endif /* __OCF_ERR_H__ */

inc/ocf_metadata.h

@@ -60,6 +60,9 @@ struct ocf_metadata_probe_status {
 
 	/** Cache contains dirty data */
 	bool cache_dirty;
+
+	/** Loaded name of cache instance */
+	char cache_name[OCF_CACHE_NAME_SIZE];
 };
 
 /**

src/metadata/metadata.c

@@ -290,6 +290,7 @@ static void ocf_metadata_load_properties_cmpl(
 	properties.cache_mode = superblock->cache_mode;
 	properties.shutdown_status = superblock->clean_shutdown;
 	properties.dirty_flushed = superblock->dirty_flushed;
+	properties.cache_name = superblock->name;
 
 	OCF_CMPL_RET(priv, 0, &properties);
 }
@@ -329,6 +330,8 @@ static void ocf_metadata_probe_cmpl(struct ocf_metadata_read_sb_ctx *context)
 	status.clean_shutdown = (superblock->clean_shutdown !=
 			ocf_metadata_dirty_shutdown);
 	status.cache_dirty = (superblock->dirty_flushed == DIRTY_NOT_FLUSHED);
+	env_strncpy(status.cache_name, OCF_CACHE_NAME_SIZE, superblock->name,
+			OCF_CACHE_NAME_SIZE);
 
 	OCF_CMPL_RET(priv, 0, &status);
 }

src/metadata/metadata.h

@@ -206,6 +206,7 @@ struct ocf_metadata_load_properties {
 	ocf_metadata_layout_t layout;
 	ocf_cache_line_size_t line_size;
 	ocf_cache_mode_t cache_mode;
+	char *cache_name;
 };
 
 typedef void (*ocf_metadata_load_properties_end_t)(void *priv, int error,

src/mngt/ocf_mngt_cache.c

@@ -60,6 +60,9 @@ struct ocf_cache_mngt_init_params {
 		bool metadata_inited : 1;
 			/*!< Metadata is inited to valid state */
 
+		bool added_to_list : 1;
+			/*!< Cache is added to context list */
+
 		bool cache_locked : 1;
 			/*!< Cache has been locked */
 	} flags;
@@ -547,33 +550,29 @@ static void _ocf_mngt_init_instance_load(
 static int _ocf_mngt_init_new_cache(struct ocf_cache_mngt_init_params *params)
 {
 	ocf_cache_t cache = env_vzalloc(sizeof(*cache));
+	int result;
 
 	if (!cache)
 		return -OCF_ERR_NO_MEM;
 
 	if (ocf_mngt_cache_lock_init(cache)) {
-		env_vfree(cache);
+		result = -OCF_ERR_NO_MEM;
-		return -OCF_ERR_NO_MEM;
+		goto alloc_err;
 	}
 
 	/* Lock cache during setup - this trylock should always succeed */
 	ENV_BUG_ON(ocf_mngt_cache_trylock(cache));
 
 	if (env_mutex_init(&cache->flush_mutex)) {
-		env_vfree(cache);
+		result = -OCF_ERR_NO_MEM;
-		return -OCF_ERR_NO_MEM;
+		goto lock_err;
 	}
 
 	if (!ocf_refcnt_inc(&cache->refcnt.cache)){
-		env_mutex_destroy(&cache->flush_mutex);
+		result = -OCF_ERR_START_CACHE_FAIL;
-		env_vfree(cache);
+		goto flush_mutex_err;
-		return -OCF_ERR_START_CACHE_FAIL;
 	}
 
-	INIT_LIST_HEAD(&cache->list);
-	list_add_tail(&cache->list, &params->ctx->caches);
-	cache->owner = params->ctx;
-
 	/* start with freezed metadata ref counter to indicate detached device*/
 	ocf_refcnt_freeze(&cache->refcnt.metadata);
 
@@ -586,6 +585,15 @@ static int _ocf_mngt_init_new_cache(struct ocf_cache_mngt_init_params *params)
 	params->flags.cache_alloc = true;
 
 	return 0;
+
+flush_mutex_err:
+	env_mutex_destroy(&cache->flush_mutex);
+lock_err:
+	ocf_mngt_cache_lock_deinit(cache);
+alloc_err:
+	env_vfree(cache);
+
+	return result;
 }
 
 static void _ocf_mngt_attach_cache_device(ocf_pipeline_t pipeline,
@@ -653,10 +661,6 @@ static int _ocf_mngt_init_prepare_cache(struct ocf_cache_mngt_init_params *param
 	ocf_cache_t cache;
 	int ret = 0;
 
-	ret = env_rmutex_lock_interruptible(&param->ctx->lock);
-	if (ret)
-		return ret;
-
 	/* Check if cache with specified name exists */
 	ret = ocf_mngt_cache_get_by_name(param->ctx, cfg->name, &cache);
 	if (!ret) {
@@ -686,7 +690,6 @@ static int _ocf_mngt_init_prepare_cache(struct ocf_cache_mngt_init_params *param
 	cache->metadata.is_volatile = cfg->metadata_volatile;
 
 out:
-	env_rmutex_unlock(&param->ctx->lock);
 	return ret;
 }
 
@@ -892,7 +895,6 @@ err_buffer:
 /**
  * Prepare metadata accordingly to mode (for load/recovery read from disk)
  */
-
 static void _ocf_mngt_attach_load_properties_end(void *priv, int error,
 		struct ocf_metadata_load_properties *properties)
 {
@@ -919,6 +921,14 @@ static void _ocf_mngt_attach_load_properties_end(void *priv, int error,
 		OCF_PL_FINISH_RET(context->pipeline, -OCF_ERR_METADATA_FOUND);
 	}
 
+	/*
+	 * Check if name loaded from disk is the same as present one.
+	 */
+	if (env_strncmp(cache->conf_meta->name, properties->cache_name,
+				OCF_CACHE_NAME_SIZE)) {
+		OCF_PL_FINISH_RET(context->pipeline, -OCF_ERR_CACHE_NAME_MISMATCH);
+	}
+
 	context->metadata.shutdown_status = properties->shutdown_status;
 	context->metadata.dirty_flushed = properties->dirty_flushed;
 
@@ -1110,6 +1120,9 @@ static void _ocf_mngt_init_handle_error(ocf_ctx_t ctx,
 	if (params->flags.metadata_inited)
 		ocf_metadata_deinit(cache);
 
+	if (!params->flags.added_to_list)
+		return;
+
 	env_rmutex_lock(&ctx->lock);
 
 	list_del(&cache->list);
@@ -1186,11 +1199,17 @@ static int _ocf_mngt_cache_start(ocf_ctx_t ctx, ocf_cache_t *cache,
 	params.metadata.promotion_policy = cfg->promotion_policy;
 	params.locked = cfg->locked;
 
-	/* Prepare cache */
+	result = env_rmutex_lock_interruptible(&ctx->lock);
-	result = _ocf_mngt_init_prepare_cache(&params, cfg);
 	if (result)
 		goto _cache_mngt_init_instance_ERROR;
 
+	/* Prepare cache */
+	result = _ocf_mngt_init_prepare_cache(&params, cfg);
+	if (result) {
+		env_rmutex_unlock(&ctx->lock);
+		goto _cache_mngt_init_instance_ERROR;
+	}
+
 	tmp_cache = params.cache;
 
 	/*
@@ -1198,15 +1217,23 @@ static int _ocf_mngt_cache_start(ocf_ctx_t ctx, ocf_cache_t *cache,
 	 */
 	result = ocf_metadata_init(tmp_cache, params.metadata.line_size);
 	if (result) {
+		env_rmutex_unlock(&ctx->lock);
 		result =  -OCF_ERR_START_CACHE_FAIL;
 		goto _cache_mngt_init_instance_ERROR;
 	}
-
 	params.flags.metadata_inited = true;
 
+	tmp_cache->owner = ctx;
+
 	result = ocf_cache_set_name(tmp_cache, cfg->name, OCF_CACHE_NAME_SIZE);
-	if (result)
+	if (result) {
+		env_rmutex_unlock(&ctx->lock);
 		goto _cache_mngt_init_instance_ERROR;
+	}
+
+	list_add_tail(&tmp_cache->list, &ctx->caches);
+	params.flags.added_to_list = true;
+	env_rmutex_unlock(&ctx->lock);
 
 	result = ocf_metadata_io_init(tmp_cache);
 	if (result)

src/ocf_cache_priv.h

@@ -212,4 +212,6 @@ static inline uint64_t ocf_get_cache_occupancy(ocf_cache_t cache)
 	return result;
 }
 
+int ocf_cache_set_name(ocf_cache_t cache, const char *src, size_t src_size);
+
 #endif /* __OCF_CACHE_PRIV_H__ */

tests/functional/pyocf/types/shared.py

@@ -46,6 +46,7 @@ class OcfErrorCode(IntEnum):
     OCF_ERR_CORE_IN_INACTIVE_STATE = auto()
     OCF_ERR_INVALID_CACHE_MODE = auto()
     OCF_ERR_INVALID_CACHE_LINE_SIZE = auto()
+    OCF_ERR_CACHE_NAME_MISMATCH = auto()
 
 
 class OcfCompletion: