* [PATCH 1/3] Refactor vg allocation code
2011-02-23 13:52 [PATCH 0/3] Share vg metadata for read only Zdenek Kabelac
@ 2011-02-23 13:52 ` Zdenek Kabelac
2011-02-23 16:48 ` Alasdair G Kergon
2011-02-23 13:52 ` [PATCH 2/3] Add VG refcount and cache created VG structure Zdenek Kabelac
2011-02-23 13:52 ` [PATCH 3/3] Destroy 'fic' memory in error path Zdenek Kabelac
2 siblings, 1 reply; 7+ messages in thread
From: Zdenek Kabelac @ 2011-02-23 13:52 UTC (permalink / raw)
To: lvm-devel
Create new function to allocate VG structure.
Usure about naming strategy - we use i.e. alloc_lv,
but most the new function have now operation prefix.
Also the location after Dave's refactoring patches is unclear.
I'd suppose we should move all 'vg' related code to vg.c,
but for now lot's of stuff is still spread across metadata.c and
other files (i.e. free_vg).
Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
lib/format_text/import_vsn1.c | 33 ++++++++++-----------------------
lib/metadata/metadata.c | 35 +++++------------------------------
lib/metadata/vg.c | 25 +++++++++++++++++++++++++
lib/metadata/vg.h | 2 ++
4 files changed, 42 insertions(+), 53 deletions(-)
diff --git a/lib/format_text/import_vsn1.c b/lib/format_text/import_vsn1.c
index 0ffe334..bc63f36 100644
--- a/lib/format_text/import_vsn1.c
+++ b/lib/format_text/import_vsn1.c
@@ -652,35 +652,28 @@ static struct volume_group *_read_vg(struct format_instance *fid,
const struct config_node *vgn, *cn;
struct volume_group *vg;
struct dm_hash_table *pv_hash = NULL, *lv_hash = NULL;
- struct dm_pool *mem = dm_pool_create("lvm2 vg_read", VG_MEMPOOL_CHUNK);
unsigned scan_done_once = use_cached_pvs;
- if (!mem)
- return_NULL;
-
/* skip any top-level values */
for (vgn = cft->root; (vgn && vgn->v); vgn = vgn->sib)
;
if (!vgn) {
log_error("Couldn't find volume group in file.");
- goto bad;
+ return NULL;
}
- if (!(vg = dm_pool_zalloc(mem, sizeof(*vg))))
- goto_bad;
-
- vg->vgmem = mem;
- vg->cmd = fid->fmt->cmd;
+ if (!(vg = alloc_vg(fid->fmt->cmd)))
+ return_NULL;
/* FIXME Determine format type from file contents */
/* eg Set to instance of fmt1 here if reading a format1 backup? */
vg->fid = fid;
- if (!(vg->name = dm_pool_strdup(mem, vgn->key)))
+ if (!(vg->name = dm_pool_strdup(vg->vgmem, vgn->key)))
goto_bad;
- if (!(vg->system_id = dm_pool_zalloc(mem, NAME_LEN)))
+ if (!(vg->system_id = dm_pool_zalloc(vg->vgmem, NAME_LEN)))
goto_bad;
vgn = vgn->child;
@@ -733,7 +726,6 @@ static struct volume_group *_read_vg(struct format_instance *fid,
goto bad;
}
- vg->alloc = ALLOC_NORMAL;
if ((cn = find_config_node(vgn, "allocation_policy"))) {
const struct config_value *cv = cn->v;
if (!cv || !cv->v.str) {
@@ -761,21 +753,16 @@ static struct volume_group *_read_vg(struct format_instance *fid,
goto bad;
}
- dm_list_init(&vg->pvs);
- if (!_read_sections(fid, "physical_volumes", _read_pv, mem, vg,
+ if (!_read_sections(fid, "physical_volumes", _read_pv, vg->vgmem, vg,
vgn, pv_hash, lv_hash, 0, &scan_done_once)) {
log_error("Couldn't find all physical volumes for volume "
"group %s.", vg->name);
goto bad;
}
- dm_list_init(&vg->lvs);
- dm_list_init(&vg->tags);
- dm_list_init(&vg->removed_pvs);
-
/* Optional tags */
if ((cn = find_config_node(vgn, "tags")) &&
- !(read_tags(mem, &vg->tags, cn->v))) {
+ !(read_tags(vg->vgmem, &vg->tags, cn->v))) {
log_error("Couldn't read tags for volume group %s.", vg->name);
goto bad;
}
@@ -789,14 +776,14 @@ static struct volume_group *_read_vg(struct format_instance *fid,
goto bad;
}
- if (!_read_sections(fid, "logical_volumes", _read_lvnames, mem, vg,
+ if (!_read_sections(fid, "logical_volumes", _read_lvnames, vg->vgmem, vg,
vgn, pv_hash, lv_hash, 1, NULL)) {
log_error("Couldn't read all logical volume names for volume "
"group %s.", vg->name);
goto bad;
}
- if (!_read_sections(fid, "logical_volumes", _read_lvsegs, mem, vg,
+ if (!_read_sections(fid, "logical_volumes", _read_lvsegs, vg->vgmem, vg,
vgn, pv_hash, lv_hash, 1, NULL)) {
log_error("Couldn't read all logical volumes for "
"volume group %s.", vg->name);
@@ -824,7 +811,7 @@ static struct volume_group *_read_vg(struct format_instance *fid,
if (lv_hash)
dm_hash_destroy(lv_hash);
- dm_pool_destroy(mem);
+ free_vg(vg);
return NULL;
}
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 6a29d24..d75a1cf 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -942,18 +942,8 @@ static struct volume_group *_vg_make_handle(struct cmd_context *cmd,
struct volume_group *vg,
uint32_t failure)
{
- struct dm_pool *vgmem;
-
- if (!vg) {
- if (!(vgmem = dm_pool_create("lvm2 vg_handle", VG_MEMPOOL_CHUNK)) ||
- !(vg = dm_pool_zalloc(vgmem, sizeof(*vg)))) {
- log_error("Error allocating vg handle.");
- if (vgmem)
- dm_pool_destroy(vgmem);
- return_NULL;
- }
- vg->vgmem = vgmem;
- }
+ if (!vg && !(vg = alloc_vg(cmd)))
+ return_NULL;
vg->read_status = failure;
@@ -994,7 +984,6 @@ struct volume_group *vg_create(struct cmd_context *cmd, const char *vg_name)
struct volume_group *vg;
struct format_instance_ctx fic;
int consistent = 0;
- struct dm_pool *mem;
uint32_t rc;
if (!validate_name(vg_name)) {
@@ -1017,10 +1006,7 @@ struct volume_group *vg_create(struct cmd_context *cmd, const char *vg_name)
return _vg_make_handle(cmd, NULL, FAILED_EXIST);
}
- if (!(mem = dm_pool_create("lvm2 vg_create", VG_MEMPOOL_CHUNK)))
- goto_bad;
-
- if (!(vg = dm_pool_zalloc(mem, sizeof(*vg))))
+ if (!(vg = alloc_vg(cmd)))
goto_bad;
if (!id_create(&vg->id)) {
@@ -1032,16 +1018,13 @@ struct volume_group *vg_create(struct cmd_context *cmd, const char *vg_name)
/* Strip dev_dir if present */
vg_name = strip_dir(vg_name, cmd->dev_dir);
- vg->vgmem = mem;
- vg->cmd = cmd;
-
- if (!(vg->name = dm_pool_strdup(mem, vg_name)))
+ if (!(vg->name = dm_pool_strdup(vg->vgmem, vg_name)))
goto_bad;
vg->seqno = 0;
vg->status = (RESIZEABLE_VG | LVM_READ | LVM_WRITE);
- if (!(vg->system_id = dm_pool_alloc(mem, NAME_LEN)))
+ if (!(vg->system_id = dm_pool_alloc(vg->vgmem, NAME_LEN)))
goto_bad;
*vg->system_id = '\0';
@@ -1057,14 +1040,6 @@ struct volume_group *vg_create(struct cmd_context *cmd, const char *vg_name)
vg->mda_copies = DEFAULT_VGMETADATACOPIES;
vg->pv_count = 0;
- dm_list_init(&vg->pvs);
-
- dm_list_init(&vg->lvs);
-
- dm_list_init(&vg->tags);
-
- /* initialize removed_pvs list */
- dm_list_init(&vg->removed_pvs);
fic.type = FMT_INSTANCE_VG | FMT_INSTANCE_MDAS | FMT_INSTANCE_AUX_MDAS;
fic.context.vg_ref.vg_name = vg_name;
diff --git a/lib/metadata/vg.c b/lib/metadata/vg.c
index 9d0d5dd..392d23c 100644
--- a/lib/metadata/vg.c
+++ b/lib/metadata/vg.c
@@ -18,6 +18,31 @@
#include "display.h"
#include "activate.h"
+struct volume_group *alloc_vg(struct cmd_context *cmd)
+{
+ struct dm_pool *vgmem;
+ struct volume_group *vg;
+
+ if (!(vgmem = dm_pool_create("lvm2 vg_handle", VG_MEMPOOL_CHUNK)) ||
+ !(vg = dm_pool_zalloc(vgmem, sizeof(*vg)))) {
+ log_error("Error allocating VG structure.");
+ if (vgmem)
+ dm_pool_destroy(vgmem);
+ return NULL;
+ }
+
+ vg->cmd = cmd;
+ vg->vgmem = vgmem;
+ vg->alloc = ALLOC_NORMAL;
+
+ dm_list_init(&vg->pvs);
+ dm_list_init(&vg->lvs);
+ dm_list_init(&vg->tags);
+ dm_list_init(&vg->removed_pvs);
+
+ return vg;
+}
+
char *vg_fmt_dup(const struct volume_group *vg)
{
if (!vg->fid || !vg->fid->fmt)
diff --git a/lib/metadata/vg.h b/lib/metadata/vg.h
index 448dcfe..c00fb74 100644
--- a/lib/metadata/vg.h
+++ b/lib/metadata/vg.h
@@ -95,6 +95,8 @@ struct volume_group {
uint32_t mda_copies; /* target number of mdas for this VG */
};
+struct volume_group *alloc_vg(struct cmd_context *cmd);
+
char *vg_fmt_dup(const struct volume_group *vg);
char *vg_name_dup(const struct volume_group *vg);
char *vg_system_id_dup(const struct volume_group *vg);
--
1.7.4.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/3] Add VG refcount and cache created VG structure
2011-02-23 13:52 [PATCH 0/3] Share vg metadata for read only Zdenek Kabelac
2011-02-23 13:52 ` [PATCH 1/3] Refactor vg allocation code Zdenek Kabelac
@ 2011-02-23 13:52 ` Zdenek Kabelac
2011-02-23 13:52 ` [PATCH 3/3] Destroy 'fic' memory in error path Zdenek Kabelac
2 siblings, 0 replies; 7+ messages in thread
From: Zdenek Kabelac @ 2011-02-23 13:52 UTC (permalink / raw)
To: lvm-devel
Imported VG from config tree is now cached in lvmcache_vginfo for
read-only code path..
Extended lvmcache_get_vg() with new parameter 'consistent'.
It passing information about how the metadata are going to be used.
For 'read-only' case it is == '0'.
VG structure has reference counter to support shared usage of VG.
(One copy is kept by cache - and 2nd is processed by the code and
released with free_vg()).
Sharing of 'write' MDA is not being made - after writting,
metadata cache is flushed and new data is read back and parsed again.
Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
lib/cache/lvmcache.c | 53 +++++++++++++++++++++++++++++++----------------
lib/cache/lvmcache.h | 4 ++-
lib/metadata/metadata.c | 5 +++-
lib/metadata/vg.c | 1 +
lib/metadata/vg.h | 1 +
5 files changed, 44 insertions(+), 20 deletions(-)
diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c
index 0a8693c..7c53b14 100644
--- a/lib/cache/lvmcache.c
+++ b/lib/cache/lvmcache.c
@@ -89,6 +89,11 @@ static void _free_cached_vgmetadata(struct lvmcache_vginfo *vginfo)
vginfo->cft = NULL;
}
+ if (vginfo->vg) {
+ free_vg(vginfo->vg);
+ vginfo->vg = NULL;
+ }
+
log_debug("Metadata cache: VG %s wiped.", vginfo->vgname);
}
@@ -624,7 +629,7 @@ int lvmcache_label_scan(struct cmd_context *cmd, int full_scan)
return r;
}
-struct volume_group *lvmcache_get_vg(const char *vgid, unsigned precommitted)
+struct volume_group *lvmcache_get_vg(const char *vgid, int consistent, unsigned precommitted)
{
struct lvmcache_vginfo *vginfo;
struct volume_group *vg;
@@ -653,25 +658,37 @@ struct volume_group *lvmcache_get_vg(const char *vgid, unsigned precommitted)
(!precommitted && vginfo->precommitted && !critical_section()))
return NULL;
- fic.type = FMT_INSTANCE_VG | FMT_INSTANCE_MDAS | FMT_INSTANCE_AUX_MDAS;
- fic.context.vg_ref.vg_name = vginfo->vgname;
- fic.context.vg_ref.vg_id = vgid;
- if (!(fid = vginfo->fmt->ops->create_instance(vginfo->fmt, &fic)))
- return_NULL;
+ if (vginfo->vg && (consistent == 0)) {
+ vg = vginfo->vg;
+ ++vg->refcount; /* Share with caller */
+ log_debug("Sharing VG %s (%d)", vginfo->vgname, vg->refcount);
+ } else {
+ fic.type = FMT_INSTANCE_VG | FMT_INSTANCE_MDAS | FMT_INSTANCE_AUX_MDAS;
+ fic.context.vg_ref.vg_name = vginfo->vgname;
+ fic.context.vg_ref.vg_id = vgid;
+ if (!(fid = vginfo->fmt->ops->create_instance(vginfo->fmt, &fic)))
+ return_NULL;
- /* Build config tree from vgmetadata, if not yet cached */
- if (!vginfo->cft &&
- !(vginfo->cft =
- create_config_tree_from_string(fid->fmt->cmd,
- vginfo->vgmetadata))) {
- _free_cached_vgmetadata(vginfo);
- return_NULL;
- }
+ /* Build config tree from vgmetadata, if not yet cached */
+ if (!vginfo->cft &&
+ !(vginfo->cft =
+ create_config_tree_from_string(fid->fmt->cmd,
+ vginfo->vgmetadata))) {
+ _free_cached_vgmetadata(vginfo);
+ return_NULL;
+ }
- if (!(vg = import_vg_from_config_tree(vginfo->cft, fid))) {
- _free_cached_vgmetadata(vginfo);
- return_NULL;
- }
+ if (!(vg = import_vg_from_config_tree(vginfo->cft, fid))) {
+ _free_cached_vgmetadata(vginfo);
+ return_NULL;
+ }
+
+ /* Cache only 'read-only' imported VG metadata */
+ if (consistent == 0) {
+ vginfo->vg = vg;
+ ++vg->refcount; /* Keep copy for cache */
+ }
+ }
log_debug("Using cached %smetadata for VG %s.",
vginfo->precommitted ? "pre-committed" : "", vginfo->vgname);
diff --git a/lib/cache/lvmcache.h b/lib/cache/lvmcache.h
index 080f3b5..19ef423 100644
--- a/lib/cache/lvmcache.h
+++ b/lib/cache/lvmcache.h
@@ -49,6 +49,8 @@ struct lvmcache_vginfo {
char *vgmetadata; /* Copy of VG metadata as format_text string */
struct config_tree *cft; /* Config tree created from vgmetadata */
/* Lifetime is directly tied to vgmetadata */
+ struct volume_group *vg; /* Cached parsed VG structure */
+ /* Lifetime is directly tied to vgmetadata */
unsigned precommitted; /* Is vgmetadata live or precommitted? */
};
@@ -120,7 +122,7 @@ struct dm_list *lvmcache_get_pvids(struct cmd_context *cmd, const char *vgname,
const char *vgid);
/* Returns cached volume group metadata. */
-struct volume_group *lvmcache_get_vg(const char *vgid, unsigned precommitted);
+struct volume_group *lvmcache_get_vg(const char *vgid, int consistent, unsigned precommitted);
void lvmcache_drop_metadata(const char *vgname, int drop_precommitted);
void lvmcache_commit_metadata(const char *vgname);
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index d75a1cf..563f0cb 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -2860,7 +2860,7 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
* Also return if use_precommitted is set due to the FIXME in
* the missing PV logic below.
*/
- if ((correct_vg = lvmcache_get_vg(vgid, precommitted)) &&
+ if ((correct_vg = lvmcache_get_vg(vgid, *consistent, precommitted)) &&
(use_precommitted || !*consistent || !(correct_vg->status & INCONSISTENT_VG))) {
if (!(correct_vg->status & INCONSISTENT_VG))
*consistent = 1;
@@ -3272,6 +3272,9 @@ void free_vg(struct volume_group *vg)
if (!vg)
return;
+ if (--vg->refcount)
+ return;
+
if (vg->cmd && vg->vgmem == vg->cmd->mem) {
log_error(INTERNAL_ERROR "global memory pool used for VG %s",
vg->name);
diff --git a/lib/metadata/vg.c b/lib/metadata/vg.c
index 392d23c..e21645c 100644
--- a/lib/metadata/vg.c
+++ b/lib/metadata/vg.c
@@ -31,6 +31,7 @@ struct volume_group *alloc_vg(struct cmd_context *cmd)
return NULL;
}
+ vg->refcount = 1;
vg->cmd = cmd;
vg->vgmem = vgmem;
vg->alloc = ALLOC_NORMAL;
diff --git a/lib/metadata/vg.h b/lib/metadata/vg.h
index c00fb74..ad6e071 100644
--- a/lib/metadata/vg.h
+++ b/lib/metadata/vg.h
@@ -37,6 +37,7 @@ struct volume_group {
struct format_instance *fid;
struct dm_list *cmd_vgs;/* List of wanted/locked and opened VGs */
uint32_t cmd_missing_vgs;/* Flag marks missing VG */
+ uint32_t refcount; /* Reference counter */
uint32_t seqno; /* Metadata sequence number */
alloc_policy_t alloc;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 7+ messages in thread