Alloc VG

PATCH New Zdenek Kabelac 2 participants 9 messages Mar 2, 2011 – Mar 9, 2011

lore

Share & tools

lore

Zdenek Kabelac

author

2011-03-02 18:20 UTC #0 lore raw

cover Alloc VG

Refactor allocation of volume group structure.

Patch is currently splitted into multiple files to make a review easier.

Things to check - why is pool_vg_read used seqno == 1? (To make it same for all - I'd leave to 0 ?)

Memory for import functions is always used from vg mempool.

Future updates -
It would be probably worth to switch vg name to char[128] (max vg name len) to simplify surrounding allocations...

Zdenek Kabelac (4):
Refactor vg allocation code
Use alloc_vg for _pool_vg_read
Use allog_vg for _free_vg in _pvsegs_sub_single
Use alloc_vg for forma1_vg_read

 lib/format1/format1.c         |   72 ++++++++++---------------------
 lib/format_pool/format_pool.c |   96 ++++++++++++++---------------------------
 lib/format_text/import_vsn1.c |   38 +++++-----------
 lib/metadata/metadata.c       |   75 ++++++-------------------------
 lib/metadata/vg.c             |   32 ++++++++++++++
 lib/metadata/vg.h             |    3 +
 tools/reporter.c              |   25 +++++------
 7 files changed, 128 insertions(+), 213 deletions(-)

Signature · 1 line

1.7.4.1

Zdenek Kabelac

author

2011-03-02 18:20 UTC #1 lore raw

PATCH 1/4 Refactor vg allocation code

Create new function to allocate VG structure.

Currently it takes pool_name (for easier debugging). And also take vg_name to futher simplify code.

Signed-off-by Zdenek Kabelac <[email protected]>

---
 lib/format_text/import_vsn1.c |   38 ++++++---------------
 lib/metadata/metadata.c       |   75 ++++++++--------------------------------
 lib/metadata/vg.c             |   32 +++++++++++++++++
 lib/metadata/vg.h             |    3 ++
 4 files changed, 61 insertions(+), 87 deletions(-)

lib/format_text/import_vsn1.c +11 ~~-27~~

index 0ffe334..d75e7af 100644
@@ -652,35 +652,25 @@ static struct volume_group *_read_vg(struct format_instance *fid,
 	const struct config_node *vgn, *cn;
 	struct volume_group *vg;
 	struct dm_hash_table *pv_hash = NULL, *lv_hash = NULL;
-	struct dm_pool *mem = dm_pool_create("lvm2 vg_read", VG_MEMPOOL_CHUNK);
 	unsigned scan_done_once = use_cached_pvs;
 
-	if (!mem)
-		return_NULL;
-
 	/* skip any top-level values */
 	for (vgn = cft->root; (vgn && vgn->v); vgn = vgn->sib)
 		;
 
 	if (!vgn) {
 		log_error("Couldn't find volume group in file.");
-		goto bad;
+		return NULL;
 	}
 
-	if (!(vg = dm_pool_zalloc(mem, sizeof(*vg))))
-		goto_bad;
-
-	vg->vgmem = mem;
-	vg->cmd = fid->fmt->cmd;
+	if (!(vg = alloc_vg("read_vg", fid->fmt->cmd, vgn->key)))
+		return_NULL;
 
 	/* FIXME Determine format type from file contents */
 	/* eg Set to instance of fmt1 here if reading a format1 backup? */
 	vg->fid = fid;
 
-	if (!(vg->name = dm_pool_strdup(mem, vgn->key)))
-		goto_bad;
-
-	if (!(vg->system_id = dm_pool_zalloc(mem, NAME_LEN)))
+	if (!(vg->system_id = dm_pool_zalloc(vg->vgmem, NAME_LEN)))
 		goto_bad;
 
 	vgn = vgn->child;

@@ -733,7 +723,6 @@ static struct volume_group *_read_vg(struct format_instance *fid,
 		goto bad;
 	}
 
-	vg->alloc = ALLOC_NORMAL;
 	if ((cn = find_config_node(vgn, "allocation_policy"))) {
 		const struct config_value *cv = cn->v;
 		if (!cv || !cv->v.str) {

@@ -761,21 +750,16 @@ static struct volume_group *_read_vg(struct format_instance *fid,
 		goto bad;
 	}
 
-	dm_list_init(&vg->pvs);
-	if (!_read_sections(fid, "physical_volumes", _read_pv, mem, vg,
+	if (!_read_sections(fid, "physical_volumes", _read_pv, vg->vgmem, vg,
 			    vgn, pv_hash, lv_hash, 0, &scan_done_once)) {
 		log_error("Couldn't find all physical volumes for volume "
 			  "group %s.", vg->name);
 		goto bad;
 	}
 
-	dm_list_init(&vg->lvs);
-	dm_list_init(&vg->tags);
-	dm_list_init(&vg->removed_pvs);
-
 	/* Optional tags */
 	if ((cn = find_config_node(vgn, "tags")) &&
-	    !(read_tags(mem, &vg->tags, cn->v))) {
+	    !(read_tags(vg->vgmem, &vg->tags, cn->v))) {
 		log_error("Couldn't read tags for volume group %s.", vg->name);
 		goto bad;
 	}

@@ -789,15 +773,15 @@ static struct volume_group *_read_vg(struct format_instance *fid,
 		goto bad;
 	}
 
-	if (!_read_sections(fid, "logical_volumes", _read_lvnames, mem, vg,
-			    vgn, pv_hash, lv_hash, 1, NULL)) {
+	if (!_read_sections(fid, "logical_volumes", _read_lvnames, vg->vgmem,
+			    vg, vgn, pv_hash, lv_hash, 1, NULL)) {
 		log_error("Couldn't read all logical volume names for volume "
 			  "group %s.", vg->name);
 		goto bad;
 	}
 
-	if (!_read_sections(fid, "logical_volumes", _read_lvsegs, mem, vg,
-			    vgn, pv_hash, lv_hash, 1, NULL)) {
+	if (!_read_sections(fid, "logical_volumes", _read_lvsegs, vg->vgmem,
+			    vg, vgn, pv_hash, lv_hash, 1, NULL)) {
 		log_error("Couldn't read all logical volumes for "
 			  "volume group %s.", vg->name);
 		goto bad;

@@ -824,7 +808,7 @@ static struct volume_group *_read_vg(struct format_instance *fid,
 	if (lv_hash)
 		dm_hash_destroy(lv_hash);
 
-	dm_pool_destroy(mem);
+	free_vg(vg);
 	return NULL;
 }

lib/metadata/metadata.c +15 ~~-60~~

139 lines not on this page · raw message

lib/metadata/vg.c +32 -0

index 9d0d5dd..5bd00ae 100644
@@ -18,6 +18,38 @@
 #include "display.h"
 #include "activate.h"
 
+struct volume_group *alloc_vg(const char *pool_name, struct cmd_context *cmd,
+			      const char *vg_name)
+{
+	struct dm_pool *vgmem;
+	struct volume_group *vg;
+
+	if (!(vgmem = dm_pool_create(pool_name, VG_MEMPOOL_CHUNK)) ||
+	    !(vg = dm_pool_zalloc(vgmem, sizeof(*vg)))) {
+		log_error("Failed to allocate volume group structure");
+		if (vgmem)
+			dm_pool_destroy(vgmem);
+		return NULL;
+	}
+
+	if (vg_name && !(vg->name = dm_pool_strdup(vgmem, vg_name))) {
+		log_error("Failed to allocate VG name.");
+		dm_pool_destroy(vgmem);
+		return NULL;
+	}
+
+	vg->cmd = cmd;
+	vg->vgmem = vgmem;
+	vg->alloc = ALLOC_NORMAL;
+
+	dm_list_init(&vg->pvs);
+	dm_list_init(&vg->lvs);
+	dm_list_init(&vg->tags);
+	dm_list_init(&vg->removed_pvs);
+
+	return vg;
+}
+
 char *vg_fmt_dup(const struct volume_group *vg)
 {
 	if (!vg->fid || !vg->fid->fmt)

lib/metadata/vg.h +3 -0

index 448dcfe..2cc6047 100644
@@ -95,6 +95,9 @@ struct volume_group {
 	uint32_t mda_copies; /* target number of mdas for this VG */
 };
 
+struct volume_group *alloc_vg(const char *pool_name, struct cmd_context *cmd,
+			      const char *vg_name);
+
 char *vg_fmt_dup(const struct volume_group *vg);
 char *vg_name_dup(const struct volume_group *vg);
 char *vg_system_id_dup(const struct volume_group *vg);

Signature · 1 line

1.7.4.1

Zdenek Kabelac

author

2011-03-02 18:20 UTC #2 lore raw

PATCH 2/4 Use alloc_vg for _pool_vg_read

In reply to Zdenek Kabelac

For simplier review.

Move remainder of _build_vg_from_pds to _pool_vg_read. Use vg memory pool for import functions. (it's been using smem -> fid mempool -> cmd mempool)

CHECKME: Why is here seqno == 1 ?
It's seems like every other alloc_vg is happy with seqno == 0. Or all the others are wrond and count should start with 1 ? Seems to be there from import day.

Signed-off-by Zdenek Kabelac <[email protected]>

---
 lib/format_pool/format_pool.c |   96 ++++++++++++++---------------------------
 1 files changed, 33 insertions(+), 63 deletions(-)

lib/format_pool/format_pool.c +33 ~~-63~~

index 4252dad..f37e1e3 100644
@@ -98,93 +98,63 @@ static int _check_usp(const char *vgname, struct user_subpool *usp, int sp_count
 	return 1;
 }
 
-static struct volume_group *_build_vg_from_pds(struct format_instance
-					       *fid, struct dm_pool *mem,
-					       struct dm_list *pds)
+static struct volume_group *_pool_vg_read(struct format_instance *fid,
+					  const char *vg_name,
+					  struct metadata_area *mda __attribute__((unused)))
 {
-	struct dm_pool *smem = fid->fmt->cmd->mem;
-	struct volume_group *vg = NULL;
-	struct user_subpool *usp = NULL;
+	struct volume_group *vg;
+	struct user_subpool *usp;
 	int sp_count;
+	DM_LIST_INIT(pds);
 
-	if (!(vg = dm_pool_zalloc(smem, sizeof(*vg)))) {
-		log_error("Unable to allocate volume group structure");
-		return NULL;
-	}
+	/* We can safely ignore the mda passed in */
 
-	vg->cmd = fid->fmt->cmd;
-	vg->vgmem = mem;
-	vg->fid = fid;
-	vg->name = NULL;
-	vg->status = 0;
-	vg->extent_count = 0;
-	vg->pv_count = 0;
-	vg->seqno = 1;
-	vg->system_id = NULL;
-	dm_list_init(&vg->pvs);
-	dm_list_init(&vg->lvs);
-	dm_list_init(&vg->tags);
-	dm_list_init(&vg->removed_pvs);
-
-	if (!import_pool_vg(vg, smem, pds))
-		return_NULL;
+	/* Strip dev_dir if present */
+	vg_name = strip_dir(vg_name, fid->fmt->cmd->dev_dir);
 
-	if (!import_pool_pvs(fid->fmt, vg, smem, pds))
+	/* Set vg_name through read_pool_pds() (SIMPLIFY in alloc_vg) */
+	if (!(vg = alloc_vg("pool_vg_read", fid->fmt->cmd, NULL)))
 		return_NULL;
 
-	if (!import_pool_lvs(vg, smem, pds))
-		return_NULL;
+	/* Read all the pvs in the vg */
+	if (!read_pool_pds(fid->fmt, vg_name, vg->vgmem, &pds))
+		goto_bad;
+
+	vg->fid = fid;
+	vg->seqno = 1; /* FIXME: Why is used 1 here? */
+
+	if (!import_pool_vg(vg, vg->vgmem, &pds))
+		goto_bad;
+
+	if (!import_pool_pvs(fid->fmt, vg, vg->vgmem, &pds))
+		goto_bad;
+
+	if (!import_pool_lvs(vg, vg->vgmem, &pds))
+		goto_bad;
 
 	/*
 	 * I need an intermediate subpool structure that contains all the
 	 * relevant info for this.  Then i can iterate through the subpool
 	 * structures for checking, and create the segments
 	 */
-	if (!(usp = _build_usp(pds, mem, &sp_count)))
-		return_NULL;
+	if (!(usp = _build_usp(&pds, vg->vgmem, &sp_count)))
+		goto_bad;
 
 	/*
 	 * check the subpool structures - we can't handle partial VGs in
 	 * the pool format, so this will error out if we're missing PVs
 	 */
 	if (!_check_usp(vg->name, usp, sp_count))
-		return_NULL;
+		goto_bad;
 
-	if (!import_pool_segments(&vg->lvs, smem, usp, sp_count))
-		return_NULL;
+	if (!import_pool_segments(&vg->lvs, vg->vgmem, usp, sp_count))
+		goto_bad;
 
 	return vg;
-}
 
-static struct volume_group *_pool_vg_read(struct format_instance *fid,
-				     const char *vg_name,
-				     struct metadata_area *mda __attribute__((unused)))
-{
-	struct dm_pool *mem = dm_pool_create("pool vg_read", VG_MEMPOOL_CHUNK);
-	struct dm_list pds;
-	struct volume_group *vg = NULL;
-
-	dm_list_init(&pds);
-
-	/* We can safely ignore the mda passed in */
-
-	if (!mem)
-		return_NULL;
-
-	/* Strip dev_dir if present */
-	vg_name = strip_dir(vg_name, fid->fmt->cmd->dev_dir);
-
-	/* Read all the pvs in the vg */
-	if (!read_pool_pds(fid->fmt, vg_name, mem, &pds))
-		goto_out;
+bad:
+	free_vg(vg);
 
-	/* Do the rest of the vg stuff */
-	if (!(vg = _build_vg_from_pds(fid, mem, &pds)))
-		goto_out;
-
-	return vg;
-out:
-	dm_pool_destroy(mem);
 	return NULL;
 }

Signature · 1 line

1.7.4.1

Zdenek Kabelac

author

2011-03-02 18:20 UTC #3 lore raw

PATCH 3/4 Use allog_vg for _free_vg in _pvsegs_sub_single

In reply to Zdenek Kabelac

For simplier review.

Switch to use alloc_vg function for allocation of VG struct.

Signed-off-by Zdenek Kabelac <[email protected]>

---
 tools/reporter.c |   25 +++++++++++--------------
 1 files changed, 11 insertions(+), 14 deletions(-)

tools/reporter.c +11 ~~-14~~

index ebe85e7..b8f8e37 100644
@@ -58,17 +58,9 @@ static int _pvsegs_sub_single(struct cmd_context *cmd,
 {
 	int ret = ECMD_PROCESSED;
 	struct lv_segment *seg = pvseg->lvseg;
-
-	struct volume_group _free_vg = {
-		.cmd = cmd,
-		.name = "",
-	};
-
-        if (!(_free_vg.vgmem = dm_pool_create("_free_vg", 10240)))
-		return ECMD_FAILED;
+	struct volume_group *_free_vg;
 
 	struct logical_volume _free_logical_volume = {
-		.vg = vg ?: &_free_vg,
 		.name = "",
 		.snapshot = NULL,
 		.status = VISIBLE_LV,

@@ -92,11 +84,15 @@ static int _pvsegs_sub_single(struct cmd_context *cmd,
 		.areas = NULL,
 	};
 
+	if (!(_free_vg = alloc_vg("free_vg", cmd, ""))) {
+		stack;
+		return ECMD_FAILED;
+	}
+
+	_free_logical_volume.vg = vg ?: _free_vg;
 	_free_lv_segment.segtype = get_segtype_from_string(cmd, "free");
 	_free_lv_segment.len = pvseg->len;
-	dm_list_init(&_free_vg.pvs);
-	dm_list_init(&_free_vg.lvs);
-	dm_list_init(&_free_vg.tags);
+
 	dm_list_init(&_free_lv_segment.tags);
 	dm_list_init(&_free_lv_segment.origin_list);
 	dm_list_init(&_free_logical_volume.tags);

@@ -109,8 +105,9 @@ static int _pvsegs_sub_single(struct cmd_context *cmd,
 		ret = ECMD_FAILED;
                 goto_out;
 	}
- out:
-	free_vg(&_free_vg);
+out:
+	free_vg(_free_vg);
+
 	return ret;
 }

Signature · 1 line

1.7.4.1

Alasdair G Kergon

2011-03-08 17:09 UTC #4 lore raw

PATCH 3/4 Use allog_vg for _free_vg in _pvsegs_sub_single

Quoted 1 line · Zdenek Kabelac

Switch to use alloc_vg function for allocation of VG struct.

This code looks wrong. Maybe it suffered from some previous global change.

tools/reporter.c static int _pvsegs_sub_single(struct cmd_context *cmd, · line 65

Quoted 4 lines

tools/reporter.c +0 -4

-	struct volume_group _free_vg = {
-		.cmd = cmd,
-		.name = "",
-	};

I think that should stay as it is.

tools/reporter.c static int _pvsegs_sub_single(struct cmd_context *cmd, · line 68

Quoted 2 lines

tools/reporter.c +0 -2

-        if (!(_free_vg.vgmem = dm_pool_create("_free_vg", 10240)))
-		return ECMD_FAILED;

Why is vgmem being allocated? If something really is using it, why?

Alasdair

Zdenek Kabelac

author

2011-03-08 22:30 UTC #5 lore raw

PATCH 3/4 Use allog_vg for _free_vg in _pvsegs_sub_single

Dne 8.3.2011 18:09, Alasdair G Kergon napsal(a):

Quoted 11 lines · Zdenek Kabelac

Quoted 1 line
Switch to use alloc_vg function for allocation of VG struct.
This code looks wrong. Maybe it suffered from some previous global change.
tools/reporter.c static int _pvsegs_sub_single(struct cmd_context *cmd, · line 65
Quoted 4 lines
tools/reporter.c +0 -4
-	struct volume_group _free_vg = {
-		.cmd = cmd,
-		.name = "",
-	};
I think that should stay as it is.

Peter already spend quite some time debuging bug in missing initialization (he was not using this patchset in his code) of some VG list structure - because there were 5 copies of VG alloc code and some of them were not all properly setting all values to some defined state.

Quoted 5 lines

tools/reporter.c static int _pvsegs_sub_single(struct cmd_context *cmd, · line 68
Quoted 3 lines
tools/reporter.c +0 -2
-        if (!(_free_vg.vgmem = dm_pool_create("_free_vg", 10240)))
-		return ECMD_FAILED;
Why is vgmem being allocated? If something really is using it, why?

I've in my tree already slightly better patch which allocates and free _free_vg only if '!vg'.

The major purpose if the patch is to keep also reference count setting in one place (needed in my vg sharing patch) - also whenever someone adds any new member to VG structure - only one place needs to be updated for proper initialization. So I'd still prefer to have this code running through standard allocation procedure.

Zdenek

Alasdair G Kergon

2011-03-09 03:01 UTC #6 lore raw

PATCH 3/4 Use allog_vg for _free_vg in _pvsegs_sub_single

Quoted 5 lines · Zdenek Kabelac

The major purpose if the patch is to keep also reference count setting in one place (needed in my vg sharing patch) - also whenever someone adds any new member to VG structure - only one place needs to be updated for proper initialization. So I'd still prefer to have this code running through standard allocation procedure.

But the new function is not (yet?) initialising the whole structure.

The 'reporter' use is only as a read-only dummy structure to allow us to share some code paths. It should not need a vgmem (or reference counting), I'd have thought.

Alasdair

Zdenek Kabelac

author

2011-03-09 08:51 UTC #7 lore raw

PATCH 3/4 Use allog_vg for _free_vg in _pvsegs_sub_single

Dne 9.3.2011 04:01, Alasdair G Kergon napsal(a):

Quoted 8 lines · Zdenek Kabelac

Quoted 5 lines
The major purpose if the patch is to keep also reference count setting in one place (needed in my vg sharing patch) - also whenever someone adds any new member to VG structure - only one place needs to be updated for proper initialization. So I'd still prefer to have this code running through standard allocation procedure.
But the new function is not (yet?) initialising the whole structure.

Idea here was - to keep all dm_list initialization in one place - so whenever some new list or some similar structure which required to have some non-null value to be usable - it will be set in one place - so one doesn't need to track all such places in the code and add dm_list_init in more then one place. (object encapsulation).

Quoted 4 lines

The 'reporter' use is only as a read-only dummy structure to allow us to share some code paths. It should not need a vgmem (or reference counting), I'd have thought.

I've been not checking all related code paths which may be executed from this place - so it's been rather pure code refactoring.

It seems like the code below now should actually never allocate anything from this pool (seem like after my recent fix for allocation from vg mem and using reporter's mempool possibly fixed it?) and if it does some allocation somewhere it's bug which should crash the appliction, but currently IMHO there was no such restriction put or documented in the code.

Zdenek

Zdenek Kabelac

author

2011-03-02 18:20 UTC #8 lore raw

PATCH 4/4 Use alloc_vg for forma1_vg_read

In reply to Zdenek Kabelac

Update code to use alloc_vg.
Move remainder of the _build_vg to _format1_vg_read

Signed-off-by Zdenek Kabelac <[email protected]>

---
 lib/format1/format1.c |   72 +++++++++++++++---------------------------------
 1 files changed, 23 insertions(+), 49 deletions(-)

lib/format1/format1.c +23 ~~-49~~

index 9011982..98c1916 100644
@@ -177,84 +177,58 @@ out:
 	return 0;
 }
 
-static struct volume_group *_build_vg(struct format_instance *fid,
-				      struct dm_list *pvs,
-				      struct dm_pool *mem)
+static struct volume_group *_format1_vg_read(struct format_instance *fid,
+				     const char *vg_name,
+				     struct metadata_area *mda __attribute__((unused)))
 {
-	struct volume_group *vg = dm_pool_zalloc(mem, sizeof(*vg));
+	struct volume_group *vg;
 	struct disk_list *dl;
+	DM_LIST_INIT(pvs);
+
+	/* Strip dev_dir if present */
+	vg_name = strip_dir(vg_name, fid->fmt->cmd->dev_dir);
+
+	if (!(vg = alloc_vg("format1_vg_read", fid->fmt->cmd, NULL)))
+		return_NULL;
 
-	if (!vg)
+	if (!read_pvs_in_vg(fid->fmt, vg_name, fid->fmt->cmd->filter,
+			    vg->vgmem, &pvs))
 		goto_bad;
 
 	if (dm_list_empty(pvs))
 		goto_bad;
 
-	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);
 
-	if (!_check_vgs(pvs, vg))
+	if (!_check_vgs(&pvs, vg))
 		goto_bad;
 
-	dl = dm_list_item(pvs->n, struct disk_list);
+	dl = dm_list_item(pvs.n, struct disk_list);
 
-	if (!import_vg(mem, vg, dl))
+	if (!import_vg(vg->vgmem, vg, dl))
 		goto_bad;
 
-	if (!import_pvs(fid->fmt, mem, vg, pvs))
+	if (!import_pvs(fid->fmt, vg->vgmem, vg, &pvs))
 		goto_bad;
 
-	if (!import_lvs(mem, vg, pvs))
+	if (!import_lvs(vg->vgmem, vg, &pvs))
 		goto_bad;
 
-	if (!import_extents(fid->fmt->cmd, vg, pvs))
+	if (!import_extents(fid->fmt->cmd, vg, &pvs))
 		goto_bad;
 
-	if (!import_snapshots(mem, vg, pvs))
+	if (!import_snapshots(vg->vgmem, vg, &pvs))
 		goto_bad;
 
 	/* Fix extents counts by adding missing PV if partial VG */
-	if ((vg->status & PARTIAL_VG) && !_fix_partial_vg(vg, pvs))
+	if ((vg->status & PARTIAL_VG) && !_fix_partial_vg(vg, &pvs))
 		goto_bad;
 
 	return vg;
 
-      bad:
-	dm_pool_free(mem, vg);
-	return NULL;
-}
-
-static struct volume_group *_format1_vg_read(struct format_instance *fid,
-				     const char *vg_name,
-				     struct metadata_area *mda __attribute__((unused)))
-{
-	struct dm_pool *mem = dm_pool_create("lvm1 vg_read", VG_MEMPOOL_CHUNK);
-	struct dm_list pvs;
-	struct volume_group *vg = NULL;
-	dm_list_init(&pvs);
-
-	if (!mem)
-		return_NULL;
-
-	/* Strip dev_dir if present */
-	vg_name = strip_dir(vg_name, fid->fmt->cmd->dev_dir);
-
-	if (!read_pvs_in_vg
-	    (fid->fmt, vg_name, fid->fmt->cmd->filter, mem, &pvs))
-		goto_bad;
-
-	if (!(vg = _build_vg(fid, &pvs, mem)))
-		goto_bad;
-
-	return vg;
 bad:
-	dm_pool_destroy(mem);
+	free(vg);
+
 	return NULL;
 }

Signature · 1 line

1.7.4.1