Skip to contentSkip to conversation

Add memory pool for format instance and fix memory leaks

PATCH New Peter Rajnoha 1 participant 9 messages Mar 9, 2011

lore
Share & tools
lore

Peter Rajnoha

author
#0 lore raw

cover Add memory pool for format instance and fix memory leaks

This patchset fixes memory leaks introduced with changes in format instance handling.

Now, the format instances have their own memory pool with reference counting (since the instances could be shared). This makes the memory handling much controllable and it uses memory in a more optimal way (long living cmd context mempool was used before and we couldn't free the memory from unused format instances).

Peter Rajnoha (7):
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.

 lib/cache/lvmcache.c             |   19 ++--
 lib/format1/format1.c            |   17 +++-
 lib/format_pool/format_pool.c    |   15 ++-
 lib/format_text/archive.c        |    8 +-
 lib/format_text/archiver.c       |   19 +++-
 lib/format_text/format-text.c    |  143 +++++++++++++--------------
 lib/format_text/format-text.h    |    7 +-
 lib/format_text/import_vsn1.c    |    8 +-
 lib/metadata/metadata-exported.h |   11 ++
 lib/metadata/metadata.c          |  204 +++++++++++++++++++++++++++++---------
 lib/metadata/metadata.h          |    1 +
 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 +
 21 files changed, 352 insertions(+), 163 deletions(-)
Signature · 1 line
1.7.4
author
#1 lore raw

PATCH 1/7 Add memory pool and reference counting for format instances.

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 <[email protected]>
---
 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(-)
lib/format1/format1.c +10 -2
index 8550a1e..a024d99 100644
@@ -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)
lib/format_pool/format_pool.c +8 -2
index 482d79a..24c21c2 100644
@@ -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)
lib/format_text/format-text.c +11 -13
index e5156f8..2a2f0ed 100644
@@ -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,
lib/metadata/metadata-exported.h +3 -0
index d1794c2..4bdf40c 100644
@@ -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;
lib/metadata/metadata.c +12 -2
index 5d84a35..5fba636 100644
@@ -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,
Signature · 1 line
1.7.4
author
#2 lore raw

PATCH 2/7 Move text_context allocation inside create_instance fn.

In reply to Peter Rajnoha

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 <[email protected]>
---
 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(-)
lib/format_text/archive.c +5 -2
index 4ba6d07..e23cb2d 100644
@@ -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;
 	}
lib/format_text/archiver.c +10 -4
index cffc04a..3d33e24 100644
@@ -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;
 	}
lib/format_text/format-text.c +50 -46

132 lines not on this page · raw message

lib/format_text/format-text.h +5 -2
index 87fe1be..0281879 100644
@@ -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);
Signature · 1 line
1.7.4
author
#3 lore raw

PATCH 3/7 Use vg_set_fid and new pv_set_fid throughout.

In reply to Peter Rajnoha

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 <[email protected]>
---
 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(-)
lib/format1/format1.c +1 -1
index a024d99..3961b67 100644
@@ -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;
lib/format_pool/format_pool.c +1 -1
index 24c21c2..53b4171 100644
@@ -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;
lib/format_text/format-text.c +1 -5
index 166c8cc..789c984 100644
@@ -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 */
lib/format_text/import_vsn1.c +4 -4
index 0ffe334..d71f1ec 100644
@@ -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.
 	 */
lib/metadata/metadata.c +45 -7

145 lines not on this page · raw message

lib/metadata/metadata.h +1 -0
index 8f0a6b1..46311df 100644
@@ -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,
Signature · 1 line
1.7.4
author
#4 lore raw

PATCH 4/7 Add new free_pv_fid fn and use it throughout.

In reply to Peter Rajnoha

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 <[email protected]>
---
 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(-)
lib/metadata/metadata-exported.h +8 -0
index 4bdf40c..170b25c 100644
@@ -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.
lib/metadata/metadata.c +80 -36

359 lines not on this page · raw message

lib/metadata/mirror.c +5 -1
index 03e73cd..793a68b 100644
@@ -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,
tools/lvconvert.c +1 -0
index b35bc81..f8b5587 100644
@@ -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++;
 		}
tools/pvcreate.c +3 -1
index a955d37..90fd2b4 100644
@@ -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;
tools/pvmove.c +8 -1
index 51f442f..c661e62 100644
@@ -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 = {
tools/pvremove.c +13 -5
index 36da62d..060303e 100644
@@ -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,
tools/pvresize.c +2 -0
index 31d8929..b12fcbc 100644
@@ -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;
tools/pvscan.c +3 -1
index b24b7ab..b40be6d 100644
@@ -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");
tools/toollib.c +13 -0
index e671987..2e95f3a 100644
@@ -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())
tools/vgconvert.c +3 -0
index 5c42f3b..76da0d8 100644
@@ -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);
tools/vgreduce.c +3 -0
index d497c76..1cabc8d 100644
@@ -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;
 }
Signature · 1 line
1.7.4
author
#5 lore raw

PATCH 5/7 Call destroy_instance for all PVs in a VG while calling free_vg.

In reply to Peter Rajnoha

This destroys/decrements ref_count for all PVs found in the vg->pvs or vg->removed_pvs list.

Signed-off-by Peter Rajnoha <[email protected]>
---
 lib/metadata/metadata.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)
lib/metadata/metadata.c +15 -0
index 5e9d13a..3a72c96 100644
@@ -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);
Signature · 1 line
1.7.4
author
#6 lore raw

PATCH 6/7 Various cleanups for fid mem and ref_count changes.

In reply to Peter Rajnoha

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 <[email protected]>
---
 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(-)
lib/cache/lvmcache.c +10 -9
index 0a8693c..b75039d 100644
@@ -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,
lib/format_text/archive.c +0 -1
index e23cb2d..4e7c8f1 100644
@@ -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)
lib/format_text/archiver.c +4 -1
index 3d33e24..e3e9fe0 100644
@@ -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;
 	}
lib/metadata/metadata.c +2 -1
index 3a72c96..c6a0b27 100644
@@ -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;
 }
Signature · 1 line
1.7.4
author
#7 lore raw

PATCH 7/7 Switch over to format instance mempool use where possible.

In reply to Peter Rajnoha

We used cmd mempool before in format instances, now we have our own mempool...

Signed-off-by Peter Rajnoha <[email protected]>
---
 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(-)
lib/format1/format1.c +1 -2
index 3961b67..c2df5cc 100644
@@ -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;
 	}
lib/format_pool/format_pool.c +1 -2
index 53b4171..ff811fc 100644
@@ -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;
 	}
lib/format_text/format-text.c +12 -13
index 789c984..f4ff9d3 100644
@@ -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;
 }
lib/metadata/metadata.c +2 -2
index c6a0b27..c3b7785 100644
@@ -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);
Signature · 1 line
1.7.4
author
#8 lore raw

cover Add memory pool for format instance and fix memory leaks

In reply to Peter Rajnoha
Quoted 7 lines · Peter Rajnoha

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(-)
lib/format1/format1.c +1 -7
index 9011982..8550a1e 100644
@@ -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);
lib/format_pool/format_pool.c +2 -11
index 4252dad..482d79a 100644
@@ -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)))) {
lib/format_text/format-text.c +3 -11
index c5db278..e5156f8 100644
@@ -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);
lib/metadata/metadata.c +19 -0
index ff61cbb..3cfbce6 100644
@@ -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)
 {
lib/metadata/metadata.h +15 -12
index 1982711..8f0a6b1 100644
@@ -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.
  */
Signature · 1 line
1.7.4