* [PATCH 1/7] Add memory pool and reference counting for format instances.
2011-03-09 12:22 [PATCH 0/7] Add memory pool for format instance and fix memory leaks Peter Rajnoha
@ 2011-03-09 12:22 ` Peter Rajnoha
2011-03-09 12:22 ` [PATCH 2/7] Move text_context allocation inside create_instance fn Peter Rajnoha
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Peter Rajnoha @ 2011-03-09 12:22 UTC (permalink / raw)
To: lvm-devel
Format instances can be created anytime on demand and it contains
metadata area information mostly (at least for now, but in the future,
we may store more things here to update/edit in a PV/VG). In case we
have lots of metadata areas, memory consumption will rise. Using cmd
context mempool is not quite optimal here because it is destroyed too
late.
So let's use a separate mempool for format instances.
Also, we need to destroy the hash which does not use a mempool in
destroy_instance fn. This call was not used explicitly before, but now
it has to be (and I can't free any cmd mempool allocations for format
instance since there could be other non-format-instance allocations
throughout the code and I'm not able to track those at all, so I can't
simply call dm_pool_free for the various fid allocations).
This change will definitely make it much easier to maintain the memory
allocations within the fid....
Reference counting is used because fids could be shared, e.g. each PV
has either a PV-based fid or VG-based fid. If it's VG-based, each PV has
a shared fid with the VG - a reference to VG's fid.
Signed-off-by: Peter Rajnoha <prajnoha@redhat.com>
---
lib/format1/format1.c | 12 ++++++++++--
lib/format_pool/format_pool.c | 10 ++++++++--
lib/format_text/format-text.c | 24 +++++++++++-------------
lib/metadata/metadata-exported.h | 3 +++
lib/metadata/metadata.c | 14 ++++++++++++--
5 files changed, 44 insertions(+), 19 deletions(-)
diff --git a/lib/format1/format1.c b/lib/format1/format1.c
index 8550a1e..a024d99 100644
--- a/lib/format1/format1.c
+++ b/lib/format1/format1.c
@@ -535,8 +535,10 @@ static struct format_instance *_format1_create_instance(const struct format_type
/* Define a NULL metadata area */
if (!(mda = dm_pool_zalloc(fmt->cmd->mem, sizeof(*mda)))) {
+ log_error("Unable to allocate metadata area structure "
+ "for lvm1 format");
dm_pool_free(fmt->cmd->mem, fid);
- return_NULL;
+ goto bad;
}
mda->ops = &_metadata_format1_ops;
@@ -545,10 +547,16 @@ static struct format_instance *_format1_create_instance(const struct format_type
dm_list_add(&fid->metadata_areas_in_use, &mda->list);
return fid;
+
+bad:
+ dm_pool_destroy(fid->mem);
+ return NULL;
}
-static void _format1_destroy_instance(struct format_instance *fid __attribute__((unused)))
+static void _format1_destroy_instance(struct format_instance *fid)
{
+ if (--fid->ref_count <= 1)
+ dm_pool_destroy(fid->mem);
}
static void _format1_destroy(struct format_type *fmt)
diff --git a/lib/format_pool/format_pool.c b/lib/format_pool/format_pool.c
index 482d79a..24c21c2 100644
--- a/lib/format_pool/format_pool.c
+++ b/lib/format_pool/format_pool.c
@@ -264,7 +264,7 @@ static struct format_instance *_pool_create_instance(const struct format_type *f
log_error("Unable to allocate metadata area structure "
"for pool format");
dm_pool_free(fmt->cmd->mem, fid);
- return NULL;
+ goto bad;
}
mda->ops = &_metadata_format_pool_ops;
@@ -273,10 +273,16 @@ static struct format_instance *_pool_create_instance(const struct format_type *f
dm_list_add(&fid->metadata_areas_in_use, &mda->list);
return fid;
+
+bad:
+ dm_pool_destroy(fid->mem);
+ return NULL;
}
-static void _pool_destroy_instance(struct format_instance *fid __attribute__((unused)))
+static void _pool_destroy_instance(struct format_instance *fid)
{
+ if (--fid->ref_count <= 1)
+ dm_pool_destroy(fid->mem);
}
static void _pool_destroy(struct format_type *fmt)
diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c
index e5156f8..2a2f0ed 100644
--- a/lib/format_text/format-text.c
+++ b/lib/format_text/format-text.c
@@ -1571,8 +1571,13 @@ static int _text_pv_initialise(const struct format_type *fmt,
return 1;
}
-static void _text_destroy_instance(struct format_instance *fid __attribute__((unused)))
+static void _text_destroy_instance(struct format_instance *fid)
{
+ if (--fid->ref_count <= 1) {
+ if (fid->type & FMT_INSTANCE_VG && fid->metadata_areas_index.hash)
+ dm_hash_destroy(fid->metadata_areas_index.hash);
+ dm_pool_destroy(fid->mem);
+ }
}
static void _free_dirs(struct dm_list *dir_list)
@@ -2188,28 +2193,21 @@ static int _text_pv_resize(const struct format_type *fmt,
return 1;
}
-/* NULL vgname means use only the supplied context e.g. an archive file */
static struct format_instance *_text_create_text_instance(const struct format_type *fmt,
const struct format_instance_ctx *fic)
{
struct format_instance *fid;
- int r;
if (!(fid = alloc_fid(fmt, fic)))
return_NULL;
- if (fid->type & FMT_INSTANCE_VG)
- r = _create_vg_text_instance(fid, fic);
- else
- r = _create_pv_text_instance(fid, fic);
-
- if (r)
+ if (fid->type & FMT_INSTANCE_VG ? _create_vg_text_instance(fid, fic) :
+ _create_pv_text_instance(fid, fic))
return fid;
- else {
- dm_pool_free(fmt->cmd->mem, fid);
- return NULL;
- }
+ dm_pool_free(fmt->cmd->mem, fid);
+ dm_pool_destroy(fid->mem);
+ return NULL;
}
void *create_text_context(struct cmd_context *cmd, const char *path,
diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index d1794c2..4bdf40c 100644
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -196,6 +196,9 @@ struct pv_segment {
#define FMT_INSTANCE_PRIVATE_MDAS 0x00000008U
struct format_instance {
+ unsigned ref_count;
+ struct dm_pool *mem;
+
uint32_t type;
const struct format_type *fmt;
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 5d84a35..5fba636 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -3954,20 +3954,30 @@ uint32_t vg_lock_newname(struct cmd_context *cmd, const char *vgname)
struct format_instance *alloc_fid(const struct format_type *fmt,
const struct format_instance_ctx *fic)
{
+ struct dm_pool *mem;
struct format_instance *fid;
+ if (!(mem = dm_pool_create("format_instance", 1024)))
+ return_NULL;
+
if (!(fid = dm_pool_zalloc(fmt->cmd->mem, sizeof(*fid)))) {
log_error("Couldn't allocate format_instance object.");
- return NULL;
+ goto bad;
}
- fid->fmt = fmt;
+ fid->ref_count = 1;
+ fid->mem = mem;
fid->type = fic->type;
+ fid->fmt = fmt;
dm_list_init(&fid->metadata_areas_in_use);
dm_list_init(&fid->metadata_areas_ignored);
return fid;
+
+bad:
+ dm_pool_destroy(mem);
+ return NULL;
}
void vg_set_fid(struct volume_group *vg,
--
1.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/7] Move text_context allocation inside create_instance fn.
2011-03-09 12:22 [PATCH 0/7] Add memory pool for format instance and fix memory leaks Peter Rajnoha
2011-03-09 12:22 ` [PATCH 1/7] Add memory pool and reference counting for format instances Peter Rajnoha
@ 2011-03-09 12:22 ` Peter Rajnoha
2011-03-09 12:22 ` [PATCH 3/7] Use vg_set_fid and new pv_set_fid throughout Peter Rajnoha
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Peter Rajnoha @ 2011-03-09 12:22 UTC (permalink / raw)
To: lvm-devel
Just another cleanup - moving the text_context part allocation inside
the create_instance fn. There are two simple reasons for this:
- we'd like to use the fid mempool for text_context that is stored
in the instance (we used cmd mempool before, so the order of
initialisation was not a matter, but now it is since we need to
create the fid mempool first which happens in create_instance fn)
- there's really no other need to have the allocation code outside
(the "create_text_context" call)
Signed-off-by: Peter Rajnoha <prajnoha@redhat.com>
---
lib/format_text/archive.c | 7 ++-
lib/format_text/archiver.c | 14 ++++--
lib/format_text/format-text.c | 96 +++++++++++++++++++++-------------------
lib/format_text/format-text.h | 7 ++-
4 files changed, 70 insertions(+), 54 deletions(-)
diff --git a/lib/format_text/archive.c b/lib/format_text/archive.c
index 4ba6d07..e23cb2d 100644
--- a/lib/format_text/archive.c
+++ b/lib/format_text/archive.c
@@ -301,6 +301,9 @@ static void _display_archive(struct cmd_context *cmd, struct archive_file *af)
struct volume_group *vg = NULL;
struct format_instance *tf;
struct format_instance_ctx fic;
+ struct text_context tc = {.path_live = af->path,
+ .path_edit = NULL,
+ .desc = NULL};
time_t when;
char *desc;
@@ -308,8 +311,8 @@ static void _display_archive(struct cmd_context *cmd, struct archive_file *af)
log_print("File:\t\t%s", af->path);
fic.type = FMT_INSTANCE_VG | FMT_INSTANCE_PRIVATE_MDAS;
- if (!(fic.context.private = create_text_context(cmd, af->path, NULL)) ||
- !(tf = cmd->fmt_backup->ops->create_instance(cmd->fmt_backup, &fic))) {
+ fic.context.private = &tc;
+ if (!(tf = cmd->fmt_backup->ops->create_instance(cmd->fmt_backup, &fic))) {
log_error("Couldn't create text instance object.");
return;
}
diff --git a/lib/format_text/archiver.c b/lib/format_text/archiver.c
index cffc04a..3d33e24 100644
--- a/lib/format_text/archiver.c
+++ b/lib/format_text/archiver.c
@@ -271,11 +271,14 @@ struct volume_group *backup_read_vg(struct cmd_context *cmd,
struct volume_group *vg = NULL;
struct format_instance *tf;
struct format_instance_ctx fic;
+ struct text_context tc = {.path_live = file,
+ .path_edit = NULL,
+ .desc = cmd->cmd_line};
struct metadata_area *mda;
fic.type = FMT_INSTANCE_VG | FMT_INSTANCE_PRIVATE_MDAS;
- if (!(fic.context.private = create_text_context(cmd, file, cmd->cmd_line)) ||
- !(tf = cmd->fmt_backup->ops->create_instance(cmd->fmt_backup, &fic))) {
+ fic.context.private = &tc;
+ if (!(tf = cmd->fmt_backup->ops->create_instance(cmd->fmt_backup, &fic))) {
log_error("Couldn't create text format object.");
return NULL;
}
@@ -379,6 +382,9 @@ int backup_to_file(const char *file, const char *desc, struct volume_group *vg)
int r = 0;
struct format_instance *tf;
struct format_instance_ctx fic;
+ struct text_context tc = {.path_live = file,
+ .path_edit = NULL,
+ .desc = desc};
struct metadata_area *mda;
struct cmd_context *cmd;
@@ -387,8 +393,8 @@ int backup_to_file(const char *file, const char *desc, struct volume_group *vg)
log_verbose("Creating volume group backup \"%s\" (seqno %u).", file, vg->seqno);
fic.type = FMT_INSTANCE_VG | FMT_INSTANCE_PRIVATE_MDAS;
- if (!(fic.context.private = create_text_context(cmd, file, desc)) ||
- !(tf = cmd->fmt_backup->ops->create_instance(cmd->fmt_backup, &fic))) {
+ fic.context.private = &tc;
+ if (!(tf = cmd->fmt_backup->ops->create_instance(cmd->fmt_backup, &fic))) {
log_error("Couldn't create backup object.");
return 0;
}
diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c
index 2a2f0ed..166c8cc 100644
--- a/lib/format_text/format-text.c
+++ b/lib/format_text/format-text.c
@@ -59,12 +59,6 @@ struct raw_list {
struct device_area dev_area;
};
-struct text_context {
- char *path_live; /* Path to file holding live metadata */
- char *path_edit; /* Path to file holding edited metadata */
- char *desc; /* Description placed inside file */
-};
-
int rlocn_is_ignored(const struct raw_locn *rlocn)
{
return (rlocn->flags & RAW_LOCN_IGNORED ? 1 : 0);
@@ -1758,6 +1752,51 @@ static int _create_pv_text_instance(struct format_instance *fid,
return 1;
}
+static void *_create_text_context(struct dm_pool *mem, struct text_context *tc)
+{
+ struct text_context *new_tc;
+ char *path, *tmp;
+
+ if (!tc)
+ return NULL;
+
+ path = tc->path_live;
+
+ if ((tmp = strstr(path, ".tmp")) && (tmp == path + strlen(path) - 4)) {
+ log_error("%s: Volume group filename may not end in .tmp",
+ path);
+ return NULL;
+ }
+
+ if (!(new_tc = dm_pool_alloc(mem, sizeof(*new_tc))))
+ return_NULL;
+
+ if (!(new_tc->path_live = dm_pool_strdup(mem, path)))
+ goto_bad;
+
+ /* If path_edit not defined, create one from path_live with .tmp suffix. */
+ if (!tc->path_edit) {
+ if (!(new_tc->path_edit = dm_pool_alloc(mem, strlen(path) + 5)))
+ goto_bad;
+
+ sprintf(new_tc->path_edit, "%s.tmp", path);
+ }
+ else
+ new_tc->path_edit = dm_pool_strdup(mem, tc->path_edit);
+
+ if (!(new_tc->desc = tc->desc ? dm_pool_strdup(mem, tc->desc)
+ : dm_pool_strdup(mem, "")))
+ goto_bad;
+
+ return (void *) new_tc;
+
+ bad:
+ dm_pool_free(mem, new_tc);
+
+ log_error("Couldn't allocate text format context object.");
+ return NULL;
+}
+
static int _create_vg_text_instance(struct format_instance *fid,
const struct format_instance_ctx *fic)
{
@@ -1769,6 +1808,7 @@ static int _create_vg_text_instance(struct format_instance *fid,
struct raw_list *rl;
struct dm_list *dir_list, *raw_list;
char path[PATH_MAX];
+ struct text_context tc;
struct lvmcache_vginfo *vginfo;
struct lvmcache_info *info;
const char *vg_name, *vg_id;
@@ -1786,7 +1826,7 @@ static int _create_vg_text_instance(struct format_instance *fid,
if (!(mda = dm_pool_zalloc(fid->fmt->cmd->mem, sizeof(*mda))))
return_0;
mda->ops = &_metadata_text_file_backup_ops;
- mda->metadata_locn = fic->context.private;
+ mda->metadata_locn = _create_text_context(fid->fmt->cmd->mem, fic->context.private);
mda->status = 0;
fid->metadata_areas_index.hash = NULL;
fid_add_mda(fid, mda, NULL, 0, 0);
@@ -1811,7 +1851,9 @@ static int _create_vg_text_instance(struct format_instance *fid,
if (!(mda = dm_pool_zalloc(fid->fmt->cmd->mem, sizeof(*mda))))
return_0;
mda->ops = &_metadata_text_file_ops;
- mda->metadata_locn = create_text_context(fid->fmt->cmd, path, NULL);
+ tc.path_live = path;
+ tc.path_edit = tc.desc = NULL;
+ mda->metadata_locn = _create_text_context(fid->fmt->cmd->mem, &tc);
mda->status = 0;
fid_add_mda(fid, mda, NULL, 0, 0);
}
@@ -2210,44 +2252,6 @@ static struct format_instance *_text_create_text_instance(const struct format_ty
return NULL;
}
-void *create_text_context(struct cmd_context *cmd, const char *path,
- const char *desc)
-{
- struct text_context *tc;
- char *tmp;
-
- if ((tmp = strstr(path, ".tmp")) && (tmp == path + strlen(path) - 4)) {
- log_error("%s: Volume group filename may not end in .tmp",
- path);
- return NULL;
- }
-
- if (!(tc = dm_pool_alloc(cmd->mem, sizeof(*tc))))
- return_NULL;
-
- if (!(tc->path_live = dm_pool_strdup(cmd->mem, path)))
- goto_bad;
-
- if (!(tc->path_edit = dm_pool_alloc(cmd->mem, strlen(path) + 5)))
- goto_bad;
-
- sprintf(tc->path_edit, "%s.tmp", path);
-
- if (!desc)
- desc = "";
-
- if (!(tc->desc = dm_pool_strdup(cmd->mem, desc)))
- goto_bad;
-
- return (void *) tc;
-
- bad:
- dm_pool_free(cmd->mem, tc);
-
- log_error("Couldn't allocate text format context object.");
- return NULL;
-}
-
static struct format_handler _text_handler = {
.scan = _text_scan,
.pv_read = _text_pv_read,
diff --git a/lib/format_text/format-text.h b/lib/format_text/format-text.h
index 87fe1be..0281879 100644
--- a/lib/format_text/format-text.h
+++ b/lib/format_text/format-text.h
@@ -44,9 +44,12 @@ int backup_list(struct cmd_context *cmd, const char *dir, const char *vgname);
/*
* The text format can read and write a volume_group to a file.
*/
+struct text_context {
+ const char *path_live; /* Path to file holding live metadata */
+ const char *path_edit; /* Path to file holding edited metadata */
+ const char *desc; /* Description placed inside file */
+};
struct format_type *create_text_format(struct cmd_context *cmd);
-void *create_text_context(struct cmd_context *cmd, const char *path,
- const char *desc);
struct labeller *text_labeller_create(const struct format_type *fmt);
--
1.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/7] Use vg_set_fid and new pv_set_fid throughout.
2011-03-09 12:22 [PATCH 0/7] Add memory pool for format instance and fix memory leaks Peter Rajnoha
2011-03-09 12:22 ` [PATCH 1/7] Add memory pool and reference counting for format instances Peter Rajnoha
2011-03-09 12:22 ` [PATCH 2/7] Move text_context allocation inside create_instance fn Peter Rajnoha
@ 2011-03-09 12:22 ` Peter Rajnoha
2011-03-09 12:22 ` [PATCH 4/7] Add new free_pv_fid fn and use it throughout Peter Rajnoha
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Peter Rajnoha @ 2011-03-09 12:22 UTC (permalink / raw)
To: lvm-devel
Let's define a strict rule that each fid assignment to a PV/VG will be
done via pv_set_fid/vg_set_fid fn. This is needed for proper reference
counting.
(Even a NULL fid should be assigned using these fns to decrement
ref_count for any currently assigned fid)
Signed-off-by: Peter Rajnoha <prajnoha@redhat.com>
---
lib/format1/format1.c | 2 +-
lib/format_pool/format_pool.c | 2 +-
lib/format_text/format-text.c | 6 +----
lib/format_text/import_vsn1.c | 8 +++---
lib/metadata/metadata.c | 52 +++++++++++++++++++++++++++++++++++-----
lib/metadata/metadata.h | 1 +
6 files changed, 53 insertions(+), 18 deletions(-)
diff --git a/lib/format1/format1.c b/lib/format1/format1.c
index a024d99..3961b67 100644
--- a/lib/format1/format1.c
+++ b/lib/format1/format1.c
@@ -192,12 +192,12 @@ static struct volume_group *_build_vg(struct format_instance *fid,
vg->cmd = fid->fmt->cmd;
vg->vgmem = mem;
- vg->fid = fid;
vg->seqno = 0;
dm_list_init(&vg->pvs);
dm_list_init(&vg->lvs);
dm_list_init(&vg->tags);
dm_list_init(&vg->removed_pvs);
+ vg_set_fid(vg, fid);
if (!_check_vgs(pvs, vg))
goto_bad;
diff --git a/lib/format_pool/format_pool.c b/lib/format_pool/format_pool.c
index 24c21c2..53b4171 100644
--- a/lib/format_pool/format_pool.c
+++ b/lib/format_pool/format_pool.c
@@ -114,7 +114,6 @@ static struct volume_group *_build_vg_from_pds(struct format_instance
vg->cmd = fid->fmt->cmd;
vg->vgmem = mem;
- vg->fid = fid;
vg->name = NULL;
vg->status = 0;
vg->extent_count = 0;
@@ -125,6 +124,7 @@ static struct volume_group *_build_vg_from_pds(struct format_instance
dm_list_init(&vg->lvs);
dm_list_init(&vg->tags);
dm_list_init(&vg->removed_pvs);
+ vg_set_fid(vg, fid);
if (!import_pool_vg(vg, smem, pds))
return_NULL;
diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c
index 166c8cc..789c984 100644
--- a/lib/format_text/format-text.c
+++ b/lib/format_text/format-text.c
@@ -1690,12 +1690,8 @@ static int _text_pv_setup(const struct format_type *fmt,
(pv_mdac = pv_mda->metadata_locn))
size_reduction = pv_mdac->area.size >> SECTOR_SHIFT;
- /* Destroy old PV-based format instance if it exists. */
- if (!(pv->fid->type & FMT_INSTANCE_VG))
- pv->fmt->ops->destroy_instance(pv->fid);
-
/* From now on, VG format instance will be used. */
- pv->fid = vg->fid;
+ pv_set_fid(pv, vg->fid);
/* FIXME Cope with genuine pe_count 0 */
diff --git a/lib/format_text/import_vsn1.c b/lib/format_text/import_vsn1.c
index 0ffe334..d71f1ec 100644
--- a/lib/format_text/import_vsn1.c
+++ b/lib/format_text/import_vsn1.c
@@ -673,10 +673,6 @@ static struct volume_group *_read_vg(struct format_instance *fid,
vg->vgmem = mem;
vg->cmd = fid->fmt->cmd;
- /* 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)))
goto_bad;
@@ -812,6 +808,10 @@ static struct volume_group *_read_vg(struct format_instance *fid,
dm_hash_destroy(pv_hash);
dm_hash_destroy(lv_hash);
+ /* FIXME Determine format type from file contents */
+ /* eg Set to instance of fmt1 here if reading a format1 backup? */
+ vg_set_fid(vg, fid);
+
/*
* Finished.
*/
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 5fba636..cfbc1b7 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -166,14 +166,24 @@ void add_pvl_to_vgs(struct volume_group *vg, struct pv_list *pvl)
dm_list_add(&vg->pvs, &pvl->list);
vg->pv_count++;
pvl->pv->vg = vg;
- pvl->pv->fid = vg->fid;
+ pv_set_fid(pvl->pv, vg->fid);
}
void del_pvl_from_vgs(struct volume_group *vg, struct pv_list *pvl)
{
+ struct format_instance_ctx fic;
+ struct format_instance *fid;
+
vg->pv_count--;
dm_list_del(&pvl->list);
pvl->pv->vg = NULL; /* orphan */
+
+ /* Use a new PV-based format instance since the PV is orphan now. */
+ fic.type = FMT_INSTANCE_PV | FMT_INSTANCE_MDAS | FMT_INSTANCE_AUX_MDAS;
+ fic.context.pv_id = (const char *) &pvl->pv->id;
+ fid = pvl->pv->fid->fmt->ops->create_instance(pvl->pv->fid->fmt, &fic);
+
+ pv_set_fid(pvl->pv, fid);
}
@@ -288,6 +298,10 @@ static int _copy_pv(struct dm_pool *pvmem,
{
memcpy(pv_to, pv_from, sizeof(*pv_to));
+ /* We must use pv_set_fid here to update the reference counter! */
+ pv_to->fid = NULL;
+ pv_set_fid(pv_to, pv_from->fid);
+
if (!(pv_to->vg_name = dm_pool_strdup(pvmem, pv_from->vg_name)))
return_0;
@@ -890,6 +904,7 @@ struct volume_group *vg_create(struct cmd_context *cmd, const char *vg_name)
{
struct volume_group *vg;
struct format_instance_ctx fic;
+ struct format_instance *fid;
int consistent = 0;
struct dm_pool *mem;
uint32_t rc;
@@ -966,10 +981,11 @@ struct volume_group *vg_create(struct cmd_context *cmd, const char *vg_name)
fic.type = FMT_INSTANCE_VG | FMT_INSTANCE_MDAS | FMT_INSTANCE_AUX_MDAS;
fic.context.vg_ref.vg_name = vg_name;
fic.context.vg_ref.vg_id = NULL;
- if (!(vg->fid = cmd->fmt->ops->create_instance(cmd->fmt, &fic))) {
+ if (!(fid = cmd->fmt->ops->create_instance(cmd->fmt, &fic))) {
log_error("Failed to create format instance");
goto bad;
}
+ vg_set_fid(vg, fid);
if (vg->fid->fmt->ops->vg_setup &&
!vg->fid->fmt->ops->vg_setup(vg->fid, vg)) {
@@ -1536,7 +1552,7 @@ static struct physical_volume *_alloc_pv(struct dm_pool *mem, struct device *dev
if (!pv)
return_NULL;
- pv->fid = NULL;
+ pv_set_fid(pv, NULL);
pv->pe_size = 0;
pv->pe_start = 0;
pv->pe_count = 0;
@@ -1591,6 +1607,7 @@ struct physical_volume *pv_create(const struct cmd_context *cmd,
{
const struct format_type *fmt = cmd->fmt;
struct format_instance_ctx fic;
+ struct format_instance *fid;
struct dm_pool *mem = fmt->cmd->mem;
struct physical_volume *pv = _alloc_pv(mem, dev);
unsigned mda_index;
@@ -1634,10 +1651,11 @@ struct physical_volume *pv_create(const struct cmd_context *cmd,
fic.type = FMT_INSTANCE_PV;
fic.context.pv_id = (const char *) &pv->id;
- if (!(pv->fid = fmt->ops->create_instance(fmt, &fic))) {
+ if (!(fid = fmt->ops->create_instance(fmt, &fic))) {
log_error("Couldn't create format instance for PV %s.", pv_dev_name(pv));
goto bad;
}
+ pv_set_fid(pv, fid);
pv->fmt = fmt;
pv->vg_name = fmt->orphan_vg_name;
@@ -2609,6 +2627,7 @@ static struct volume_group *_vg_read_orphans(struct cmd_context *cmd,
const char *orphan_vgname)
{
struct format_instance_ctx fic;
+ struct format_instance *fid;
struct lvmcache_vginfo *vginfo;
struct lvmcache_info *info;
struct pv_list *pvl;
@@ -2643,10 +2662,11 @@ static struct volume_group *_vg_read_orphans(struct cmd_context *cmd,
fic.type = FMT_INSTANCE_VG | FMT_INSTANCE_AUX_MDAS;
fic.context.vg_ref.vg_name = orphan_vgname;
fic.context.vg_ref.vg_id = NULL;
- if (!(vg->fid = vginfo->fmt->ops->create_instance(vginfo->fmt, &fic))) {
+ if (!(fid = vginfo->fmt->ops->create_instance(vginfo->fmt, &fic))) {
log_error("Failed to create format instance");
goto bad;
}
+ vg_set_fid(vg, fid);
dm_list_iterate_items(info, &vginfo->infos) {
if (!(pv = _pv_read(cmd, mem, dev_name(info->dev),
@@ -3425,11 +3445,12 @@ static struct physical_volume *_pv_read(struct cmd_context *cmd,
else {
fic.type = FMT_INSTANCE_PV | FMT_INSTANCE_MDAS | FMT_INSTANCE_AUX_MDAS;
fic.context.pv_id = (const char *) &pv->id;
- if (!(pv->fid = pv->fmt->ops->create_instance(pv->fmt, &fic))) {
+ if (!(fid = pv->fmt->ops->create_instance(pv->fmt, &fic))) {
log_error("_pv_read: Couldn't create format instance "
"for PV %s", pv_name);
goto bad;
}
+ pv_set_fid(pv, fid);
}
return pv;
@@ -3980,14 +4001,31 @@ bad:
return NULL;
}
+void pv_set_fid(struct physical_volume *pv,
+ struct format_instance *fid)
+{
+ if (pv->fid)
+ pv->fid->fmt->ops->destroy_instance(pv->fid);
+
+ pv->fid = fid;
+ if (fid)
+ fid->ref_count++;
+}
+
void vg_set_fid(struct volume_group *vg,
struct format_instance *fid)
{
struct pv_list *pvl;
+ if (vg->fid)
+ vg->fid->fmt->ops->destroy_instance(vg->fid);
+
vg->fid = fid;
+ if (fid)
+ fid->ref_count++;
+
dm_list_iterate_items(pvl, &vg->pvs)
- pvl->pv->fid = fid;
+ pv_set_fid(pvl->pv, fid);
}
static int _convert_key_to_string(const char *key, size_t key_len,
diff --git a/lib/metadata/metadata.h b/lib/metadata/metadata.h
index 8f0a6b1..46311df 100644
--- a/lib/metadata/metadata.h
+++ b/lib/metadata/metadata.h
@@ -206,6 +206,7 @@ struct format_instance_ctx {
struct format_instance *alloc_fid(const struct format_type *fmt,
const struct format_instance_ctx *fic);
+void pv_set_fid(struct physical_volume *pv, struct format_instance *fid);
void vg_set_fid(struct volume_group *vg, struct format_instance *fid);
/* FIXME: Add generic interface for mda counts based on given key. */
int fid_add_mda(struct format_instance *fid, struct metadata_area *mda,
--
1.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 4/7] Add new free_pv_fid fn and use it throughout.
2011-03-09 12:22 [PATCH 0/7] Add memory pool for format instance and fix memory leaks Peter Rajnoha
` (2 preceding siblings ...)
2011-03-09 12:22 ` [PATCH 3/7] Use vg_set_fid and new pv_set_fid throughout Peter Rajnoha
@ 2011-03-09 12:22 ` Peter Rajnoha
2011-03-09 12:22 ` [PATCH 5/7] Call destroy_instance for all PVs in a VG while calling free_vg Peter Rajnoha
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Peter Rajnoha @ 2011-03-09 12:22 UTC (permalink / raw)
To: lvm-devel
Normally, when the PV structure is part of the VG
(vg->pvs/vg->removed->pvs list), a call to free_vg fn will also free the
PV fid part automatically (that will be in following patch). But for any
standalone PV structures, we have to call free_pv_fid directly now.
I meant this to be "free_pv" at first, but since PV does not have its
own "mem" yet, we have only the format instance part to deallocate
(therefore "free_pv_fid"). Whenever we have a mempool or anything else
to destroy/deallocate for the PV, we can just rename the free_pv_fid to
free_pv and add the code there. Just like we have the "free_vg".
Signed-off-by: Peter Rajnoha <prajnoha@redhat.com>
---
lib/metadata/metadata-exported.h | 8 +++
lib/metadata/metadata.c | 116 ++++++++++++++++++++++++++------------
lib/metadata/mirror.c | 6 ++-
tools/lvconvert.c | 1 +
tools/pvcreate.c | 4 +-
tools/pvmove.c | 9 +++-
tools/pvremove.c | 18 ++++--
tools/pvresize.c | 2 +
tools/pvscan.c | 4 +-
tools/toollib.c | 13 ++++
tools/vgconvert.c | 3 +
tools/vgreduce.c | 3 +
12 files changed, 142 insertions(+), 45 deletions(-)
diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index 4bdf40c..170b25c 100644
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -460,6 +460,14 @@ void del_pvl_from_vgs(struct volume_group *vg, struct pv_list *pvl);
int remove_lvs_in_vg(struct cmd_context *cmd,
struct volume_group *vg,
force_t force);
+
+/*
+ * free_pv_fid() must be called on every struct physical_volume allocated
+ * by pv_create, pv_read, find_pv_by_name or pv_by_path to free it when
+ * no longer required.
+ */
+void free_pv_fid(struct physical_volume *pv);
+
/*
* free_vg() must be called on every struct volume_group allocated
* by vg_create() or vg_read_internal() to free it when no longer required.
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index cfbc1b7..5e9d13a 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -644,8 +644,10 @@ static int vg_extend_single_pv(struct volume_group *vg, char *pv_name,
if (!pv)
return 0;
}
- if (!add_pv_to_vg(vg, pv_name, pv))
+ if (!add_pv_to_vg(vg, pv_name, pv)) {
+ free_pv_fid(pv);
return 0;
+ }
return 1;
}
@@ -1348,6 +1350,7 @@ static int pvcreate_check(struct cmd_context *cmd, const char *name,
* system.
*/
if (pv && is_orphan(pv) && !dm_list_size(&pv->fid->metadata_areas_in_use)) {
+ free_pv_fid(pv);
if (!scan_vgs_for_pvs(cmd, 0))
return_0;
pv = pv_read(cmd, name, NULL, 0, 0);
@@ -1358,18 +1361,18 @@ static int pvcreate_check(struct cmd_context *cmd, const char *name,
if (pv && !is_orphan(pv) && pp->force != DONT_PROMPT_OVERRIDE) {
log_error("Can't initialize physical volume \"%s\" of "
"volume group \"%s\" without -ff", name, pv_vg_name(pv));
- return 0;
+ goto bad;
}
/* prompt */
if (pv && !is_orphan(pv) && !pp->yes &&
yes_no_prompt(_really_init, name, pv_vg_name(pv)) == 'n') {
log_error("%s: physical volume not initialized", name);
- return 0;
+ goto bad;
}
if (sigint_caught())
- return 0;
+ goto_bad;
dev = dev_cache_get(name, cmd->filter);
@@ -1384,7 +1387,7 @@ static int pvcreate_check(struct cmd_context *cmd, const char *name,
if (!dev) {
log_error("Device %s not found (or ignored by filtering).", name);
- return 0;
+ goto bad;
}
/*
@@ -1394,20 +1397,20 @@ static int pvcreate_check(struct cmd_context *cmd, const char *name,
/* FIXME Detect whether device-mapper itself is still using it */
log_error("Can't open %s exclusively. Mounted filesystem?",
name);
- return 0;
+ goto bad;
}
if (!_wipe_sb(dev, "software RAID md superblock", name, 4, pp, dev_is_md))
- return 0;
+ goto_bad;
if (!_wipe_sb(dev, "swap signature", name, 10, pp, dev_is_swap))
- return 0;
+ goto_bad;
if (!_wipe_sb(dev, "LUKS signature", name, 8, pp, dev_is_luks))
- return 0;
+ goto_bad;
if (sigint_caught())
- return 0;
+ goto_bad;
if (pv && !is_orphan(pv) && pp->force) {
log_warn("WARNING: Forcing physical volume creation on "
@@ -1417,7 +1420,12 @@ static int pvcreate_check(struct cmd_context *cmd, const char *name,
!is_orphan(pv) ? "\"" : "");
}
+ free_pv_fid(pv);
return 1;
+
+bad:
+ free_pv_fid(pv);
+ return 0;
}
void pvcreate_params_set_defaults(struct pvcreate_params *pp)
@@ -1455,7 +1463,7 @@ struct physical_volume * pvcreate_single(struct cmd_context *cmd,
const char *pv_name,
struct pvcreate_params *pp)
{
- struct physical_volume *pv;
+ struct physical_volume *pv = NULL;
struct device *dev;
struct dm_list mdas;
struct pvcreate_params default_pp;
@@ -1470,23 +1478,23 @@ struct physical_volume * pvcreate_single(struct cmd_context *cmd,
(dev != dev_cache_get(pv_name, cmd->filter))) {
if (!id_write_format((const struct id*)&pp->idp->uuid,
buffer, sizeof(buffer)))
- return_NULL;
+ goto_bad;
log_error("uuid %s already in use on \"%s\"", buffer,
dev_name(dev));
- return NULL;
+ goto bad;;
}
}
if (!pvcreate_check(cmd, pv_name, pp))
- goto error;
+ goto_bad;
if (sigint_caught())
- goto error;
+ goto_bad;
if (!(dev = dev_cache_get(pv_name, cmd->filter))) {
log_error("%s: Couldn't find device. Check your filters?",
pv_name);
- goto error;
+ goto bad;
}
dm_list_init(&mdas);
@@ -1497,7 +1505,7 @@ struct physical_volume * pvcreate_single(struct cmd_context *cmd,
pp->labelsector, pp->pvmetadatacopies,
pp->pvmetadatasize, pp->metadataignore))) {
log_error("Failed to setup physical volume \"%s\"", pv_name);
- goto error;
+ goto bad;
}
log_verbose("Set up physical volume for \"%s\" with %" PRIu64
@@ -1506,20 +1514,20 @@ struct physical_volume * pvcreate_single(struct cmd_context *cmd,
/* Wipe existing label first */
if (!label_remove(pv_dev(pv))) {
log_error("Failed to wipe existing label on %s", pv_name);
- goto error;
+ goto bad;
}
if (pp->zero) {
log_verbose("Zeroing start of device %s", pv_name);
if (!dev_open_quiet(dev)) {
log_error("%s not opened: device not zeroed", pv_name);
- goto error;
+ goto bad;
}
if (!dev_set(dev, UINT64_C(0), (size_t) 2048, 0)) {
log_error("%s not wiped: aborting", pv_name);
dev_close(dev);
- goto error;
+ goto bad;
}
dev_close(dev);
}
@@ -1529,22 +1537,18 @@ struct physical_volume * pvcreate_single(struct cmd_context *cmd,
if (!(pv_write(cmd, pv, 0))) {
log_error("Failed to write physical volume \"%s\"", pv_name);
- goto error;
+ goto bad;
}
log_print("Physical volume \"%s\" successfully created", pv_name);
return pv;
- error:
+bad:
+ free_pv_fid(pv);
return NULL;
}
-static void _free_pv(struct dm_pool *mem, struct physical_volume *pv)
-{
- dm_pool_free(mem, pv);
-}
-
static struct physical_volume *_alloc_pv(struct dm_pool *mem, struct device *dev)
{
struct physical_volume *pv = dm_pool_zalloc(mem, sizeof(*pv));
@@ -1683,7 +1687,8 @@ struct physical_volume *pv_create(const struct cmd_context *cmd,
return pv;
bad:
- _free_pv(mem, pv);
+ free_pv_fid(pv);
+ dm_pool_free(mem, pv);
return NULL;
}
@@ -1835,25 +1840,30 @@ static struct physical_volume *_find_pv_by_name(struct cmd_context *cmd,
if (!(pv = _pv_read(cmd, cmd->mem, pv_name, NULL, NULL, 1, 0))) {
log_error("Physical volume %s not found", pv_name);
- return NULL;
+ goto bad;
}
if (is_orphan_vg(pv->vg_name) && !dm_list_size(&pv->fid->metadata_areas_in_use)) {
/* If a PV has no MDAs - need to search all VGs for it */
if (!scan_vgs_for_pvs(cmd, 1))
- return_NULL;
+ goto_bad;
+ free_pv_fid(pv);
if (!(pv = _pv_read(cmd, cmd->mem, pv_name, NULL, NULL, 1, 0))) {
log_error("Physical volume %s not found", pv_name);
- return NULL;
+ goto bad;
}
}
if (is_orphan_vg(pv->vg_name)) {
log_error("Physical volume %s not in a volume group", pv_name);
- return NULL;
+ goto bad;
}
return pv;
+
+bad:
+ free_pv_fid(pv);
+ return NULL;
}
/* Find segment at a given logical extent in an LV */
@@ -2632,7 +2642,7 @@ static struct volume_group *_vg_read_orphans(struct cmd_context *cmd,
struct lvmcache_info *info;
struct pv_list *pvl;
struct volume_group *vg;
- struct physical_volume *pv;
+ struct physical_volume *pv = NULL;
struct dm_pool *mem;
lvmcache_label_scan(cmd, 0);
@@ -2683,6 +2693,7 @@ static struct volume_group *_vg_read_orphans(struct cmd_context *cmd,
return vg;
bad:
+ free_pv_fid(pv);
dm_pool_destroy(mem);
return NULL;
}
@@ -2713,6 +2724,14 @@ static int _update_pv_list(struct dm_pool *pvmem, struct dm_list *all_pvs, struc
return 1;
}
+static void _free_pv_list(struct dm_list *all_pvs)
+{
+ struct pv_list *pvl;
+
+ dm_list_iterate_items(pvl, all_pvs)
+ pvl->pv->fid->fmt->ops->destroy_instance(pvl->pv->fid);
+}
+
int vg_missing_pv_count(const struct volume_group *vg)
{
int ret = 0;
@@ -3021,6 +3040,7 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
if (!correct_vg) {
correct_vg = vg;
if (!_update_pv_list(cmd->mem, &all_pvs, correct_vg)) {
+ _free_pv_list(&all_pvs);
free_vg(vg);
return_NULL;
}
@@ -3045,6 +3065,7 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
inconsistent_seqno = 1;
}
if (!_update_pv_list(cmd->mem, &all_pvs, vg)) {
+ _free_pv_list(&all_pvs);
free_vg(vg);
free_vg(correct_vg);
return_NULL;
@@ -3060,8 +3081,10 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
}
/* Give up looking */
- if (!correct_vg)
+ if (!correct_vg) {
+ _free_pv_list(&all_pvs);
return_NULL;
+ }
}
/*
@@ -3084,20 +3107,25 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
*/
if (!inconsistent_seqno) {
*consistent = 0;
+ _free_pv_list(&all_pvs);
return correct_vg;
}
+ _free_pv_list(&all_pvs);
free_vg(correct_vg);
return NULL;
}
- if (!*consistent)
+ if (!*consistent) {
+ _free_pv_list(&all_pvs);
return correct_vg;
+ }
/* Don't touch if vgids didn't match */
if (inconsistent_vgid) {
log_error("Inconsistent metadata UUIDs found for "
"volume group %s", vgname);
*consistent = 0;
+ _free_pv_list(&all_pvs);
return correct_vg;
}
@@ -3114,6 +3142,7 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
cmd->handles_missing_pvs = 1;
if (!vg_write(correct_vg)) {
log_error("Automatic metadata correction failed");
+ _free_pv_list(&all_pvs);
free_vg(correct_vg);
cmd->handles_missing_pvs = saved_handles_missing_pvs;
return NULL;
@@ -3123,6 +3152,7 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
if (!vg_commit(correct_vg)) {
log_error("Automatic metadata correction commit "
"failed");
+ _free_pv_list(&all_pvs);
free_vg(correct_vg);
return NULL;
}
@@ -3133,12 +3163,14 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
goto next_pv;
}
if (!id_write_format(&pvl->pv->id, uuid, sizeof(uuid))) {
+ _free_pv_list(&all_pvs);
free_vg(correct_vg);
return_NULL;
}
log_error("Removing PV %s (%s) that no longer belongs to VG %s",
pv_dev_name(pvl->pv), uuid, correct_vg->name);
if (!pv_write_orphan(cmd, pvl->pv)) {
+ _free_pv_list(&all_pvs);
free_vg(correct_vg);
return_NULL;
}
@@ -3150,6 +3182,8 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
}
}
+ _free_pv_list(&all_pvs);
+
if (vg_missing_pv_count(correct_vg)) {
log_verbose("There are %d physical volumes missing.",
vg_missing_pv_count(correct_vg));
@@ -3209,6 +3243,15 @@ struct volume_group *vg_read_internal(struct cmd_context *cmd, const char *vgnam
return vg;
}
+void free_pv_fid(struct physical_volume *pv)
+{
+ if (!pv)
+ return;
+
+ if (pv->fid)
+ pv->fid->fmt->ops->destroy_instance(pv->fid);
+}
+
void free_vg(struct volume_group *vg)
{
if (!vg)
@@ -3455,7 +3498,8 @@ static struct physical_volume *_pv_read(struct cmd_context *cmd,
return pv;
bad:
- _free_pv(pvmem, pv);
+ free_pv_fid(pv);
+ dm_pool_free(pvmem, pv);
return NULL;
}
diff --git a/lib/metadata/mirror.c b/lib/metadata/mirror.c
index 03e73cd..793a68b 100644
--- a/lib/metadata/mirror.c
+++ b/lib/metadata/mirror.c
@@ -1463,11 +1463,15 @@ struct logical_volume *find_pvmove_lv_from_pvname(struct cmd_context *cmd,
uint32_t lv_type)
{
struct physical_volume *pv;
+ struct logical_volume *lv;
if (!(pv = find_pv_by_name(cmd, name)))
return_NULL;
- return find_pvmove_lv(vg, pv->dev, lv_type);
+ lv = find_pvmove_lv(vg, pv->dev, lv_type);
+ free_pv_fid(pv);
+
+ return lv;
}
struct dm_list *lvs_using_lv(struct cmd_context *cmd, struct volume_group *vg,
diff --git a/tools/lvconvert.c b/tools/lvconvert.c
index b35bc81..f8b5587 100644
--- a/tools/lvconvert.c
+++ b/tools/lvconvert.c
@@ -791,6 +791,7 @@ static void _remove_missing_empty_pv(struct volume_group *vg, struct dm_list *re
vg->free_count -= pvl_vg->pv->pe_count;
vg->extent_count -= pvl_vg->pv->pe_count;
del_pvl_from_vgs(vg, pvl_vg);
+ free_pv_fid(pvl_vg->pv);
removed++;
}
diff --git a/tools/pvcreate.c b/tools/pvcreate.c
index a955d37..90fd2b4 100644
--- a/tools/pvcreate.c
+++ b/tools/pvcreate.c
@@ -93,6 +93,7 @@ int pvcreate(struct cmd_context *cmd, int argc, char **argv)
int i;
int ret = ECMD_PROCESSED;
struct pvcreate_params pp;
+ struct physical_volume *pv;
pvcreate_params_set_defaults(&pp);
@@ -111,11 +112,12 @@ int pvcreate(struct cmd_context *cmd, int argc, char **argv)
unescape_colons_and_at_signs(argv[i], NULL, NULL);
- if (!pvcreate_single(cmd, argv[i], &pp)) {
+ if (!(pv = pvcreate_single(cmd, argv[i], &pp))) {
stack;
ret = ECMD_FAILED;
}
+ free_pv_fid(pv);
unlock_vg(cmd, VG_ORPHANS);
if (sigint_caught())
return ret;
diff --git a/tools/pvmove.c b/tools/pvmove.c
index 51f442f..c661e62 100644
--- a/tools/pvmove.c
+++ b/tools/pvmove.c
@@ -431,11 +431,13 @@ static int _set_up_pvmove(struct cmd_context *cmd, const char *pv_name,
if (!(lv_name = _extract_lvname(cmd, pv_vg_name(pv),
arg_value(cmd, name_ARG)))) {
stack;
+ free_pv_fid(pv);
return EINVALID_CMD_LINE;
}
if (!validate_name(lv_name)) {
log_error("Logical volume name %s is invalid", lv_name);
+ free_pv_fid(pv);
return EINVALID_CMD_LINE;
}
}
@@ -510,6 +512,7 @@ static int _set_up_pvmove(struct cmd_context *cmd, const char *pv_name,
/* LVs are all in status LOCKED */
r = ECMD_PROCESSED;
out:
+ free_pv_fid(pv);
unlock_and_free_vg(cmd, vg, pv_vg_name(pv));
return r;
}
@@ -600,6 +603,7 @@ static struct volume_group *_get_move_vg(struct cmd_context *cmd,
const char *uuid __attribute__((unused)))
{
struct physical_volume *pv;
+ struct volume_group *vg;
/* Reread all metadata in case it got changed */
if (!(pv = find_pv_by_name(cmd, name))) {
@@ -608,7 +612,10 @@ static struct volume_group *_get_move_vg(struct cmd_context *cmd,
return NULL;
}
- return _get_vg(cmd, pv_vg_name(pv));
+ vg = _get_vg(cmd, pv_vg_name(pv));
+ free_pv_fid(pv);
+
+ return vg;
}
static struct poll_functions _pvmove_fns = {
diff --git a/tools/pvremove.c b/tools/pvremove.c
index 36da62d..060303e 100644
--- a/tools/pvremove.c
+++ b/tools/pvremove.c
@@ -49,31 +49,34 @@ static int pvremove_check(struct cmd_context *cmd, const char *name)
if (!scan_vgs_for_pvs(cmd, 0)) {
log_error("Rescan for PVs without metadata areas "
"failed.");
- return 0;
+ goto bad;
}
+ free_pv_fid(pv);
if (!(pv = pv_read(cmd, name, NULL, 1, 0))) {
log_error("Failed to read physical volume %s", name);
- return 0;
+ goto bad;
}
}
/* orphan ? */
- if (is_orphan(pv))
+ if (is_orphan(pv)) {
+ free_pv_fid(pv);
return 1;
+ }
/* Allow partial & exported VGs to be destroyed. */
/* we must have -ff to overwrite a non orphan */
if (arg_count(cmd, force_ARG) < 2) {
log_error("PV %s belongs to Volume Group %s so please use vgreduce first.", name, pv_vg_name(pv));
log_error("(If you are certain you need pvremove, then confirm by using --force twice.)");
- return 0;
+ goto bad;
}
/* prompt */
if (!arg_count(cmd, yes_ARG) &&
yes_no_prompt(_really_wipe, name, pv_vg_name(pv)) == 'n') {
log_error("%s: physical volume label not removed", name);
- return 0;
+ goto bad;
}
if (arg_count(cmd, force_ARG)) {
@@ -84,7 +87,12 @@ static int pvremove_check(struct cmd_context *cmd, const char *name)
!is_orphan(pv) ? "\"" : "");
}
+ free_pv_fid(pv);
return 1;
+
+bad:
+ free_pv_fid(pv);
+ return 0;
}
static int pvremove_single(struct cmd_context *cmd, const char *pv_name,
diff --git a/tools/pvresize.c b/tools/pvresize.c
index 31d8929..b12fcbc 100644
--- a/tools/pvresize.c
+++ b/tools/pvresize.c
@@ -126,6 +126,8 @@ out:
log_error("Use pvcreate and vgcfgrestore "
"to repair from archived metadata.");
unlock_vg(cmd, vg_name);
+ if (is_orphan_vg(vg_name))
+ free_pv_fid(pv);
if (!old_vg)
free_vg(vg);
return r;
diff --git a/tools/pvscan.c b/tools/pvscan.c
index b24b7ab..b40be6d 100644
--- a/tools/pvscan.c
+++ b/tools/pvscan.c
@@ -183,8 +183,10 @@ int pvscan(struct cmd_context *cmd, int argc __attribute__((unused)),
pv_max_name_len += 2;
vg_max_name_len += 2;
- dm_list_iterate_items(pvl, pvslist)
+ dm_list_iterate_items(pvl, pvslist) {
_pvscan_display_single(cmd, pvl->pv, NULL);
+ free_pv_fid(pvl->pv);
+ }
if (!pvs_found) {
log_print("No matching physical volumes found");
diff --git a/tools/toollib.c b/tools/toollib.c
index e671987..2e95f3a 100644
--- a/tools/toollib.c
+++ b/tools/toollib.c
@@ -658,6 +658,9 @@ static int _process_all_devs(struct cmd_context *cmd, void *handle,
pv = &pv_dummy;
}
ret = process_single_pv(cmd, NULL, pv, handle);
+
+ free_pv_fid(pv);
+
if (ret > ret_max)
ret_max = ret;
if (sigint_caught())
@@ -757,6 +760,7 @@ int process_each_pv(struct cmd_context *cmd, int argc, char **argv,
continue;
}
scanned = 1;
+ free_pv_fid(pv);
if (!(pv = pv_read(cmd, argv[opt],
NULL, 1,
scan_label_only))) {
@@ -770,6 +774,14 @@ int process_each_pv(struct cmd_context *cmd, int argc, char **argv,
}
ret = process_single_pv(cmd, vg, pv, handle);
+
+ /*
+ * Free PV only if we called pv_read before,
+ * otherwise the PV structure is part of the VG.
+ */
+ if (!vg)
+ free_pv_fid(pv);
+
if (ret > ret_max)
ret_max = ret;
if (sigint_caught())
@@ -823,6 +835,7 @@ int process_each_pv(struct cmd_context *cmd, int argc, char **argv,
dm_list_iterate_items(pvl, pvslist) {
ret = process_single_pv(cmd, NULL, pvl->pv,
handle);
+ free_pv_fid(pvl->pv);
if (ret > ret_max)
ret_max = ret;
if (sigint_caught())
diff --git a/tools/vgconvert.c b/tools/vgconvert.c
index 5c42f3b..76da0d8 100644
--- a/tools/vgconvert.c
+++ b/tools/vgconvert.c
@@ -146,6 +146,7 @@ static int vgconvert_single(struct cmd_context *cmd, const char *vg_name,
pv_dev_name(pv));
log_error("Use pvcreate and vgcfgrestore to repair "
"from archived metadata.");
+ free_pv_fid(pv);
return ECMD_FAILED;
}
@@ -156,11 +157,13 @@ static int vgconvert_single(struct cmd_context *cmd, const char *vg_name,
pv_dev_name(pv));
log_error("Use pvcreate and vgcfgrestore to repair "
"from archived metadata.");
+ free_pv_fid(pv);
return ECMD_FAILED;
}
log_verbose("Physical volume \"%s\" successfully created",
pv_dev_name(pv));
+ free_pv_fid(pv);
}
log_verbose("Deleting existing metadata for VG %s", vg_name);
diff --git a/tools/vgreduce.c b/tools/vgreduce.c
index d497c76..1cabc8d 100644
--- a/tools/vgreduce.c
+++ b/tools/vgreduce.c
@@ -40,6 +40,7 @@ static int _remove_pv(struct volume_group *vg, struct pv_list *pvl, int silent)
vg->free_count -= pvl->pv->pe_count;
vg->extent_count -= pvl->pv->pe_count;
del_pvl_from_vgs(vg, pvl);
+ free_pv_fid(pvl->pv);
return 1;
}
@@ -450,6 +451,8 @@ static int _vgreduce_single(struct cmd_context *cmd, struct volume_group *vg,
log_print("Removed \"%s\" from volume group \"%s\"", name, vg->name);
r = ECMD_PROCESSED;
bad:
+ if (pvl)
+ free_pv_fid(pvl->pv);
unlock_and_free_vg(cmd, orphan_vg, VG_ORPHANS);
return r;
}
--
1.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 5/7] Call destroy_instance for all PVs in a VG while calling free_vg.
2011-03-09 12:22 [PATCH 0/7] Add memory pool for format instance and fix memory leaks Peter Rajnoha
` (3 preceding siblings ...)
2011-03-09 12:22 ` [PATCH 4/7] Add new free_pv_fid fn and use it throughout Peter Rajnoha
@ 2011-03-09 12:22 ` Peter Rajnoha
2011-03-09 12:22 ` [PATCH 6/7] Various cleanups for fid mem and ref_count changes Peter Rajnoha
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Peter Rajnoha @ 2011-03-09 12:22 UTC (permalink / raw)
To: lvm-devel
This destroys/decrements ref_count for all PVs found in the vg->pvs or
vg->removed_pvs list.
Signed-off-by: Peter Rajnoha <prajnoha@redhat.com>
---
lib/metadata/metadata.c | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 5e9d13a..3a72c96 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -3254,9 +3254,24 @@ void free_pv_fid(struct physical_volume *pv)
void free_vg(struct volume_group *vg)
{
+ struct pv_list *pvl;
+
if (!vg)
return;
+ if (vg->pvs.n)
+ dm_list_iterate_items(pvl, &vg->pvs)
+ if (pvl)
+ pvl->pv->fid->fmt->ops->destroy_instance(pvl->pv->fid);
+
+ if (vg->removed_pvs.n)
+ dm_list_iterate_items(pvl, &vg->removed_pvs)
+ if (pvl)
+ pvl->pv->fid->fmt->ops->destroy_instance(pvl->pv->fid);
+
+ if (vg->fid)
+ vg->fid->fmt->ops->destroy_instance(vg->fid);
+
if (vg->cmd && vg->vgmem == vg->cmd->mem) {
log_error(INTERNAL_ERROR "global memory pool used for VG %s",
vg->name);
--
1.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 6/7] Various cleanups for fid mem and ref_count changes.
2011-03-09 12:22 [PATCH 0/7] Add memory pool for format instance and fix memory leaks Peter Rajnoha
` (4 preceding siblings ...)
2011-03-09 12:22 ` [PATCH 5/7] Call destroy_instance for all PVs in a VG while calling free_vg Peter Rajnoha
@ 2011-03-09 12:22 ` Peter Rajnoha
2011-03-09 12:22 ` [PATCH 7/7] Switch over to format instance mempool use where possible Peter Rajnoha
2011-03-09 12:40 ` [PATCH 0/7] Add memory pool for format instance and fix memory leaks Peter Rajnoha
7 siblings, 0 replies; 9+ messages in thread
From: Peter Rajnoha @ 2011-03-09 12:22 UTC (permalink / raw)
To: lvm-devel
Well, I've encountered these by chance:
Missing free_vg on error_paths in lvmcache_get_vg and _vg_read_orphans
fn. Call destroy_instance only if the fid is not part of the vg in
backup_read_vg fn (otherwise it's part of the VG we're returning and
we definitely don't want to destroy it!).
Signed-off-by: Peter Rajnoha <prajnoha@redhat.com>
---
lib/cache/lvmcache.c | 19 ++++++++++---------
lib/format_text/archive.c | 1 -
lib/format_text/archiver.c | 5 ++++-
lib/metadata/metadata.c | 3 ++-
4 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c
index 0a8693c..b75039d 100644
--- a/lib/cache/lvmcache.c
+++ b/lib/cache/lvmcache.c
@@ -627,7 +627,7 @@ int lvmcache_label_scan(struct cmd_context *cmd, int full_scan)
struct volume_group *lvmcache_get_vg(const char *vgid, unsigned precommitted)
{
struct lvmcache_vginfo *vginfo;
- struct volume_group *vg;
+ struct volume_group *vg = NULL;
struct format_instance *fid;
struct format_instance_ctx fic;
@@ -663,20 +663,21 @@ struct volume_group *lvmcache_get_vg(const char *vgid, unsigned precommitted)
if (!vginfo->cft &&
!(vginfo->cft =
create_config_tree_from_string(fid->fmt->cmd,
- vginfo->vgmetadata))) {
- _free_cached_vgmetadata(vginfo);
- return_NULL;
- }
+ vginfo->vgmetadata)))
+ goto_bad;
- 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)))
+ goto_bad;
log_debug("Using cached %smetadata for VG %s.",
vginfo->precommitted ? "pre-committed" : "", vginfo->vgname);
return vg;
+
+bad:
+ free_vg(vg);
+ _free_cached_vgmetadata(vginfo);
+ return NULL;
}
struct dm_list *lvmcache_get_vgids(struct cmd_context *cmd,
diff --git a/lib/format_text/archive.c b/lib/format_text/archive.c
index e23cb2d..4e7c8f1 100644
--- a/lib/format_text/archive.c
+++ b/lib/format_text/archive.c
@@ -333,7 +333,6 @@ static void _display_archive(struct cmd_context *cmd, struct archive_file *af)
log_print("Backup Time:\t%s", ctime(&when));
free_vg(vg);
- tf->fmt->ops->destroy_instance(tf);
}
int archive_list(struct cmd_context *cmd, const char *dir, const char *vgname)
diff --git a/lib/format_text/archiver.c b/lib/format_text/archiver.c
index 3d33e24..e3e9fe0 100644
--- a/lib/format_text/archiver.c
+++ b/lib/format_text/archiver.c
@@ -289,7 +289,9 @@ struct volume_group *backup_read_vg(struct cmd_context *cmd,
break;
}
- tf->fmt->ops->destroy_instance(tf);
+ if (!vg)
+ tf->fmt->ops->destroy_instance(tf);
+
return vg;
}
@@ -401,6 +403,7 @@ int backup_to_file(const char *file, const char *desc, struct volume_group *vg)
if (!dm_list_size(&tf->metadata_areas_in_use)) {
log_error(INTERNAL_ERROR "No in use metadata areas to write.");
+ tf->fmt->ops->destroy_instance(tf);
return 0;
}
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 3a72c96..c6a0b27 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -2641,7 +2641,7 @@ static struct volume_group *_vg_read_orphans(struct cmd_context *cmd,
struct lvmcache_vginfo *vginfo;
struct lvmcache_info *info;
struct pv_list *pvl;
- struct volume_group *vg;
+ struct volume_group *vg = NULL;
struct physical_volume *pv = NULL;
struct dm_pool *mem;
@@ -2694,6 +2694,7 @@ static struct volume_group *_vg_read_orphans(struct cmd_context *cmd,
return vg;
bad:
free_pv_fid(pv);
+ free_vg(vg);
dm_pool_destroy(mem);
return NULL;
}
--
1.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 7/7] Switch over to format instance mempool use where possible.
2011-03-09 12:22 [PATCH 0/7] Add memory pool for format instance and fix memory leaks Peter Rajnoha
` (5 preceding siblings ...)
2011-03-09 12:22 ` [PATCH 6/7] Various cleanups for fid mem and ref_count changes Peter Rajnoha
@ 2011-03-09 12:22 ` Peter Rajnoha
2011-03-09 12:40 ` [PATCH 0/7] Add memory pool for format instance and fix memory leaks Peter Rajnoha
7 siblings, 0 replies; 9+ messages in thread
From: Peter Rajnoha @ 2011-03-09 12:22 UTC (permalink / raw)
To: lvm-devel
We used cmd mempool before in format instances, now we have our own
mempool...
Signed-off-by: Peter Rajnoha <prajnoha@redhat.com>
---
lib/format1/format1.c | 3 +--
lib/format_pool/format_pool.c | 3 +--
lib/format_text/format-text.c | 25 ++++++++++++-------------
lib/metadata/metadata.c | 4 ++--
4 files changed, 16 insertions(+), 19 deletions(-)
diff --git a/lib/format1/format1.c b/lib/format1/format1.c
index 3961b67..c2df5cc 100644
--- a/lib/format1/format1.c
+++ b/lib/format1/format1.c
@@ -534,10 +534,9 @@ static struct format_instance *_format1_create_instance(const struct format_type
return_NULL;
/* Define a NULL metadata area */
- if (!(mda = dm_pool_zalloc(fmt->cmd->mem, sizeof(*mda)))) {
+ if (!(mda = dm_pool_zalloc(fid->mem, sizeof(*mda)))) {
log_error("Unable to allocate metadata area structure "
"for lvm1 format");
- dm_pool_free(fmt->cmd->mem, fid);
goto bad;
}
diff --git a/lib/format_pool/format_pool.c b/lib/format_pool/format_pool.c
index 53b4171..ff811fc 100644
--- a/lib/format_pool/format_pool.c
+++ b/lib/format_pool/format_pool.c
@@ -260,10 +260,9 @@ static struct format_instance *_pool_create_instance(const struct format_type *f
return_NULL;
/* Define a NULL metadata area */
- if (!(mda = dm_pool_zalloc(fmt->cmd->mem, sizeof(*mda)))) {
+ if (!(mda = dm_pool_zalloc(fid->mem, sizeof(*mda)))) {
log_error("Unable to allocate metadata area structure "
"for pool format");
- dm_pool_free(fmt->cmd->mem, fid);
goto bad;
}
diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c
index 789c984..f4ff9d3 100644
--- a/lib/format_text/format-text.c
+++ b/lib/format_text/format-text.c
@@ -1659,7 +1659,7 @@ static int _text_pv_setup(const struct format_type *fmt,
/* Be sure it's not already in VG's format instance! */
if (!fid_get_mda_indexed(vg->fid, pvid, ID_LEN, mda_index)) {
- pv_mda_copy = mda_copy(vg->fid->fmt->cmd->mem, pv_mda);
+ pv_mda_copy = mda_copy(vg->fid->mem, pv_mda);
fid_add_mda(vg->fid, pv_mda_copy, pvid, ID_LEN, mda_index);
}
}
@@ -1727,14 +1727,14 @@ static int _create_pv_text_instance(struct format_instance *fid,
struct lvmcache_info *info;
if (!(fid_pv_tc = (struct text_fid_pv_context *)
- dm_pool_zalloc(fid->fmt->cmd->mem, sizeof(*fid_pv_tc)))) {
+ dm_pool_zalloc(fid->mem, sizeof(*fid_pv_tc)))) {
log_error("Couldn't allocate text_fid_pv_context.");
return 0;
}
fid_pv_tc->label_sector = -1;
fid->private = (void *) fid_pv_tc;
- if (!(fid->metadata_areas_index.array = dm_pool_zalloc(fid->fmt->cmd->mem,
+ if (!(fid->metadata_areas_index.array = dm_pool_zalloc(fid->mem,
FMT_TEXT_MAX_MDAS_PER_PV *
sizeof(struct metadata_area *)))) {
log_error("Couldn't allocate format instance metadata index.");
@@ -1810,7 +1810,7 @@ static int _create_vg_text_instance(struct format_instance *fid,
const char *vg_name, *vg_id;
if (!(fidtc = (struct text_fid_context *)
- dm_pool_zalloc(fid->fmt->cmd->mem,sizeof(*fidtc)))) {
+ dm_pool_zalloc(fid->mem, sizeof(*fidtc)))) {
log_error("Couldn't allocate text_fid_context.");
return 0;
}
@@ -1819,10 +1819,10 @@ static int _create_vg_text_instance(struct format_instance *fid,
fid->private = (void *) fidtc;
if (type & FMT_INSTANCE_PRIVATE_MDAS) {
- if (!(mda = dm_pool_zalloc(fid->fmt->cmd->mem, sizeof(*mda))))
+ if (!(mda = dm_pool_zalloc(fid->mem, sizeof(*mda))))
return_0;
mda->ops = &_metadata_text_file_backup_ops;
- mda->metadata_locn = _create_text_context(fid->fmt->cmd->mem, fic->context.private);
+ mda->metadata_locn = _create_text_context(fid->mem, fic->context.private);
mda->status = 0;
fid->metadata_areas_index.hash = NULL;
fid_add_mda(fid, mda, NULL, 0, 0);
@@ -1844,12 +1844,12 @@ static int _create_vg_text_instance(struct format_instance *fid,
return 0;
}
- if (!(mda = dm_pool_zalloc(fid->fmt->cmd->mem, sizeof(*mda))))
+ if (!(mda = dm_pool_zalloc(fid->mem, sizeof(*mda))))
return_0;
mda->ops = &_metadata_text_file_ops;
tc.path_live = path;
tc.path_edit = tc.desc = NULL;
- mda->metadata_locn = _create_text_context(fid->fmt->cmd->mem, &tc);
+ mda->metadata_locn = _create_text_context(fid->mem, &tc);
mda->status = 0;
fid_add_mda(fid, mda, NULL, 0, 0);
}
@@ -1860,10 +1860,10 @@ static int _create_vg_text_instance(struct format_instance *fid,
if (!_raw_holds_vgname(fid, &rl->dev_area, vg_name))
continue;
- if (!(mda = dm_pool_zalloc(fid->fmt->cmd->mem, sizeof(*mda))))
+ if (!(mda = dm_pool_zalloc(fid->mem, sizeof(*mda))))
return_0;
- if (!(mdac = dm_pool_zalloc(fid->fmt->cmd->mem, sizeof(*mdac))))
+ if (!(mdac = dm_pool_zalloc(fid->mem, sizeof(*mdac))))
return_0;
mda->metadata_locn = mdac;
/* FIXME Allow multiple dev_areas inside area */
@@ -1911,12 +1911,12 @@ static int _add_metadata_area_to_pv(struct physical_volume *pv,
pv->fmt->name);
}
- if (!(mda = dm_malloc(sizeof(struct metadata_area)))) {
+ if (!(mda = dm_pool_zalloc(pv->fid->mem, sizeof(struct metadata_area)))) {
log_error("struct metadata_area allocation failed");
return 0;
}
- if (!(mdac = dm_malloc(sizeof(struct mda_context)))) {
+ if (!(mdac = dm_pool_zalloc(pv->fid->mem, sizeof(struct mda_context)))) {
log_error("struct mda_context allocation failed");
dm_free(mda);
return 0;
@@ -2243,7 +2243,6 @@ static struct format_instance *_text_create_text_instance(const struct format_ty
_create_pv_text_instance(fid, fic))
return fid;
- dm_pool_free(fmt->cmd->mem, fid);
dm_pool_destroy(fid->mem);
return NULL;
}
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index c6a0b27..c3b7785 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -4041,7 +4041,7 @@ struct format_instance *alloc_fid(const struct format_type *fmt,
if (!(mem = dm_pool_create("format_instance", 1024)))
return_NULL;
- if (!(fid = dm_pool_zalloc(fmt->cmd->mem, sizeof(*fid)))) {
+ if (!(fid = dm_pool_zalloc(mem, sizeof(*fid)))) {
log_error("Couldn't allocate format_instance object.");
goto bad;
}
@@ -4133,7 +4133,7 @@ int fid_add_mdas(struct format_instance *fid, struct dm_list *mdas,
unsigned mda_index = 0;
dm_list_iterate_items(mda, mdas) {
- mda_new = mda_copy(fid->fmt->cmd->mem, mda);
+ mda_new = mda_copy(fid->mem, mda);
if (!mda_new)
return_0;
fid_remove_mda(fid, NULL, key, key_len, mda_index);
--
1.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 0/7] Add memory pool for format instance and fix memory leaks
2011-03-09 12:22 [PATCH 0/7] Add memory pool for format instance and fix memory leaks Peter Rajnoha
` (6 preceding siblings ...)
2011-03-09 12:22 ` [PATCH 7/7] Switch over to format instance mempool use where possible Peter Rajnoha
@ 2011-03-09 12:40 ` Peter Rajnoha
7 siblings, 0 replies; 9+ messages in thread
From: Peter Rajnoha @ 2011-03-09 12:40 UTC (permalink / raw)
To: lvm-devel
On 03/09/2011 01:22 PM +0100, Peter Rajnoha wrote:
> Add memory pool and reference counting for format instances.
> Move text_context allocation inside create_instance fn.
> Use vg_set_fid and new pv_set_fid throughout.
> Add new free_pv_fid fn and use it throughout.
> Call destroy_instance for all PVs in a VG while calling free_vg.
> Various cleanups for fid mem and ref_count changes.
> Switch over to format instance mempool use where possible.
+ one more that has to come as the very first one!
Use new alloc_fid fn for common format instance initialisation.
Just a little cleanup for each format's create_instance fn. There's
common code that doesn't need to be repeated. Factor it out into
a separate "alloc_fid" fn. This makes the code easier to maintain.
---
lib/format1/format1.c | 8 +-------
lib/format_pool/format_pool.c | 13 ++-----------
lib/format_text/format-text.c | 14 +++-----------
lib/metadata/metadata.c | 19 +++++++++++++++++++
lib/metadata/metadata.h | 27 +++++++++++++++------------
5 files changed, 40 insertions(+), 41 deletions(-)
diff --git a/lib/format1/format1.c b/lib/format1/format1.c
index 9011982..8550a1e 100644
--- a/lib/format1/format1.c
+++ b/lib/format1/format1.c
@@ -530,15 +530,9 @@ static struct format_instance *_format1_create_instance(const struct format_type
struct format_instance *fid;
struct metadata_area *mda;
- if (!(fid = dm_pool_alloc(fmt->cmd->mem, sizeof(*fid))))
+ if (!(fid = alloc_fid(fmt, fic)))
return_NULL;
- fid->fmt = fmt;
- fid->type = fic->type;
-
- dm_list_init(&fid->metadata_areas_in_use);
- dm_list_init(&fid->metadata_areas_ignored);
-
/* Define a NULL metadata area */
if (!(mda = dm_pool_zalloc(fmt->cmd->mem, sizeof(*mda)))) {
dm_pool_free(fmt->cmd->mem, fid);
diff --git a/lib/format_pool/format_pool.c b/lib/format_pool/format_pool.c
index 4252dad..482d79a 100644
--- a/lib/format_pool/format_pool.c
+++ b/lib/format_pool/format_pool.c
@@ -256,17 +256,8 @@ static struct format_instance *_pool_create_instance(const struct format_type *f
struct format_instance *fid;
struct metadata_area *mda;
- if (!(fid = dm_pool_zalloc(fmt->cmd->mem, sizeof(*fid)))) {
- log_error("Unable to allocate format instance structure for "
- "pool format");
- return NULL;
- }
-
- fid->fmt = fmt;
- fid->type = fic->type;
-
- dm_list_init(&fid->metadata_areas_in_use);
- dm_list_init(&fid->metadata_areas_ignored);
+ if (!(fid = alloc_fid(fmt, fic)))
+ return_NULL;
/* Define a NULL metadata area */
if (!(mda = dm_pool_zalloc(fmt->cmd->mem, sizeof(*mda)))) {
diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c
index c5db278..e5156f8 100644
--- a/lib/format_text/format-text.c
+++ b/lib/format_text/format-text.c
@@ -2190,21 +2190,13 @@ static int _text_pv_resize(const struct format_type *fmt,
/* NULL vgname means use only the supplied context e.g. an archive file */
static struct format_instance *_text_create_text_instance(const struct format_type *fmt,
- const struct format_instance_ctx *fic)
+ const struct format_instance_ctx *fic)
{
struct format_instance *fid;
int r;
- if (!(fid = dm_pool_alloc(fmt->cmd->mem, sizeof(*fid)))) {
- log_error("Couldn't allocate format instance object.");
- return NULL;
- }
-
- fid->fmt = fmt;
- fid->type = fic->type;
-
- dm_list_init(&fid->metadata_areas_in_use);
- dm_list_init(&fid->metadata_areas_ignored);
+ if (!(fid = alloc_fid(fmt, fic)))
+ return_NULL;
if (fid->type & FMT_INSTANCE_VG)
r = _create_vg_text_instance(fid, fic);
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index ff61cbb..3cfbce6 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -3951,6 +3951,25 @@ uint32_t vg_lock_newname(struct cmd_context *cmd, const char *vgname)
return FAILED_EXIST;
}
+struct format_instance *alloc_fid(const struct format_type *fmt,
+ const struct format_instance_ctx *fic)
+{
+ struct format_instance *fid;
+
+ if (!(fid = dm_pool_zalloc(fmt->cmd->mem, sizeof(*fid)))) {
+ log_error("Couldn't allocate format_instance object.");
+ return NULL;
+ }
+
+ fid->fmt = fmt;
+ fid->type = fic->type;
+
+ dm_list_init(&fid->metadata_areas_in_use);
+ dm_list_init(&fid->metadata_areas_ignored);
+
+ return fid;
+}
+
void vg_set_fid(struct volume_group *vg,
struct format_instance *fid)
{
diff --git a/lib/metadata/metadata.h b/lib/metadata/metadata.h
index 1982711..8f0a6b1 100644
--- a/lib/metadata/metadata.h
+++ b/lib/metadata/metadata.h
@@ -191,6 +191,21 @@ struct metadata_area *mda_copy(struct dm_pool *mem,
unsigned mda_is_ignored(struct metadata_area *mda);
void mda_set_ignored(struct metadata_area *mda, unsigned ignored);
unsigned mda_locns_match(struct metadata_area *mda1, struct metadata_area *mda2);
+
+struct format_instance_ctx {
+ uint32_t type;
+ union {
+ const char *pv_id;
+ struct {
+ const char *vg_name;
+ const char *vg_id;
+ } vg_ref;
+ void *private;
+ } context;
+};
+
+struct format_instance *alloc_fid(const struct format_type *fmt,
+ const struct format_instance_ctx *fic);
void vg_set_fid(struct volume_group *vg, struct format_instance *fid);
/* FIXME: Add generic interface for mda counts based on given key. */
int fid_add_mda(struct format_instance *fid, struct metadata_area *mda,
@@ -229,18 +244,6 @@ struct seg_list {
struct lv_segment *seg;
};
-struct format_instance_ctx {
- uint32_t type;
- union {
- const char *pv_id;
- struct {
- const char *vg_name;
- const char *vg_id;
- } vg_ref;
- void *private;
- } context;
-};
-
/*
* Ownership of objects passes to caller.
*/
--
1.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread