From 2abccadb9ea9c7025ba6f138f13c7e960dd6b1fa Mon Sep 17 00:00:00 2001 From: Michal Mielewczyk Date: Mon, 23 Sep 2019 07:59:29 -0400 Subject: [PATCH 1/6] New error code for invaild loaded cache name. When loading cache metadata, it should be started with exactly the same name as it was running previously. Otherwise load should fail and return newly added error code. Signed-off-by: Michal Mielewczyk --- inc/ocf_err.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/inc/ocf_err.h b/inc/ocf_err.h index 52345a3..2d0c966 100644 --- a/inc/ocf_err.h +++ b/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__ */ From d332c9d97f5cb770edb54c513f1bd09030342c3a Mon Sep 17 00:00:00 2001 From: Michal Mielewczyk Date: Wed, 25 Sep 2019 03:52:47 -0400 Subject: [PATCH 2/6] New ocf error code in test framework. Signed-off-by: Michal Mielewczyk --- tests/functional/pyocf/types/shared.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/functional/pyocf/types/shared.py b/tests/functional/pyocf/types/shared.py index 155bf09..74f97a0 100644 --- a/tests/functional/pyocf/types/shared.py +++ b/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: From c04ea4898f195121b95ee59b4ef727c489137828 Mon Sep 17 00:00:00 2001 From: Michal Mielewczyk Date: Thu, 19 Sep 2019 19:13:27 -0400 Subject: [PATCH 3/6] Check if loaded cache name is valid. When loading cache, it's name should be the same as the loaded one. Signed-off-by: Michal Mielewczyk --- src/metadata/metadata.c | 1 + src/metadata/metadata.h | 1 + src/mngt/ocf_mngt_cache.c | 9 ++++++++- 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/metadata/metadata.c b/src/metadata/metadata.c index ddcd247..3050d0c 100644 --- a/src/metadata/metadata.c +++ b/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); } diff --git a/src/metadata/metadata.h b/src/metadata/metadata.h index 5c75b89..f776c1d 100644 --- a/src/metadata/metadata.h +++ b/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, diff --git a/src/mngt/ocf_mngt_cache.c b/src/mngt/ocf_mngt_cache.c index 150ef6d..65f1656 100644 --- a/src/mngt/ocf_mngt_cache.c +++ b/src/mngt/ocf_mngt_cache.c @@ -892,7 +892,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 +918,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; From 39c5819a518ce52792c0c6b3aceead82376c269d Mon Sep 17 00:00:00 2001 From: Michal Mielewczyk Date: Mon, 23 Sep 2019 10:19:12 -0400 Subject: [PATCH 4/6] Set cache name before adding it to context list. This change allows to check if specified cache name name is unique. To prevent adding cache instance with the same name, context lock is acquired until name isn't set. Signed-off-by: Michal Mielewczyk --- src/mngt/ocf_mngt_cache.c | 60 ++++++++++++++++++++++++++------------- 1 file changed, 40 insertions(+), 20 deletions(-) diff --git a/src/mngt/ocf_mngt_cache.c b/src/mngt/ocf_mngt_cache.c index 65f1656..d80ab9e 100644 --- a/src/mngt/ocf_mngt_cache.c +++ b/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); - return -OCF_ERR_NO_MEM; + result = -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); - return -OCF_ERR_NO_MEM; + result = -OCF_ERR_NO_MEM; + goto lock_err; } if (!ocf_refcnt_inc(&cache->refcnt.cache)){ - env_mutex_destroy(&cache->flush_mutex); - env_vfree(cache); - return -OCF_ERR_START_CACHE_FAIL; + result = -OCF_ERR_START_CACHE_FAIL; + goto flush_mutex_err; } - INIT_LIST_HEAD(&cache->list); - list_add_tail(&cache->list, ¶ms->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(¶m->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(¶m->ctx->lock); return ret; } @@ -1125,6 +1128,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); @@ -1201,11 +1207,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 = _ocf_mngt_init_prepare_cache(¶ms, cfg); + result = env_rmutex_lock_interruptible(&ctx->lock); if (result) goto _cache_mngt_init_instance_ERROR; + /* Prepare cache */ + result = _ocf_mngt_init_prepare_cache(¶ms, cfg); + if (result) { + env_rmutex_unlock(&ctx->lock); + goto _cache_mngt_init_instance_ERROR; + } + tmp_cache = params.cache; /* @@ -1213,15 +1225,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) From f461f3c62edbed7a7ab2c15a267c6b6dfca4cf76 Mon Sep 17 00:00:00 2001 From: Michal Mielewczyk Date: Mon, 23 Sep 2019 10:28:25 -0400 Subject: [PATCH 5/6] Extend probe informations with cache name. Since ocf requires loading cache with the same name as it was stopped, it should also allow to read name from metadata. Signed-off-by: Michal Mielewczyk --- inc/ocf_metadata.h | 3 +++ src/metadata/metadata.c | 2 ++ 2 files changed, 5 insertions(+) diff --git a/inc/ocf_metadata.h b/inc/ocf_metadata.h index f2951cf..5ecfbca 100644 --- a/inc/ocf_metadata.h +++ b/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]; }; /** diff --git a/src/metadata/metadata.c b/src/metadata/metadata.c index 3050d0c..9964d46 100644 --- a/src/metadata/metadata.c +++ b/src/metadata/metadata.c @@ -330,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); } From 6c076a7c07b5534aa39cf88885d056e2120842a7 Mon Sep 17 00:00:00 2001 From: Michal Mielewczyk Date: Wed, 25 Sep 2019 05:08:51 -0400 Subject: [PATCH 6/6] Remove set_cache_name() from public API. Signed-off-by: Michal Mielewczyk --- inc/ocf_cache.h | 12 ------------ src/ocf_cache_priv.h | 2 ++ 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/inc/ocf_cache.h b/inc/ocf_cache.h index 7d92aaf..13d08ba 100644 --- a/inc/ocf_cache.h +++ b/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 * diff --git a/src/ocf_cache_priv.h b/src/ocf_cache_priv.h index 7f54881..89eb2e7 100644 --- a/src/ocf_cache_priv.h +++ b/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__ */