[PATCH 0/3] lvm_vg_reduce and vg_reduce

[PATCH 0/3] lvm_vg_reduce and vg_reduce

[Thomas Woerner]

This patch series adds lvm_vg_reduce and vg_reduce. For more information 
please have a look at the patches.

Dave Wysochanski (1):
  Initialize removed_pvs list in format-specific volume_group
    constructors.

Thomas Woerner (2):
  Add vg_reduce to metadata.c and metadata-exported.h
  Add lvm_vg_reduce to liblvm2app

 lib/format1/format1.c            |    1 +
 lib/format_pool/format_pool.c    |    1 +
 lib/format_text/import_vsn1.c    |    1 +
 lib/metadata/metadata-exported.h |    7 ++
 lib/metadata/metadata.c          |   61 +++++++++++++++++++
 liblvm/.exported_symbols         |    1 +
 liblvm/lvm.h                     |   18 ++++++
 liblvm/lvm_vg.c                  |   25 ++++++++
 test/api/vgtest.c                |  118 ++++++++++++++++++++++++--------------
 9 files changed, 190 insertions(+), 43 deletions(-)

[PATCH 1/3] Add vg_reduce to metadata.c and metadata-exported.h

[Thomas Woerner]

This function behaves a little bit different than vg_reduce_single, because
it allowes to remove even the latest pv. This has been done to be consistent
to lvm_vg_create, which creates an empty vg.

removed_pvs has been added to the volume_group struct. vg_reduce adds removed
pvs to this list to be able to commit the changes for the pvs in lvm_vg_commit
in liblvm2app.

Signed-off-by: Thomas Woerner <twoerner@redhat.com>
---
 lib/metadata/metadata-exported.h |    7 ++++
 lib/metadata/metadata.c          |   61 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+), 0 deletions(-)

diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index 142381b..bc946da 100644
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -243,6 +243,12 @@ struct volume_group {
 	 */
 
 	/*
+	 * List of removed physical volumes by pvreduce.
+	 * They have to get cleared on vg_commit.
+	 */
+	struct dm_list removed_pvs;
+
+	/*
 	 * Store result of the last vg_read().
 	 * 0 for success else appropriate FAILURE_* bits set.
 	 */
@@ -437,6 +443,7 @@ int vg_remove_single(vg_t *vg);
 int vg_rename(struct cmd_context *cmd, struct volume_group *vg,
 	      const char *new_name);
 int vg_extend(struct volume_group *vg, int pv_count, char **pv_names);
+int vg_reduce(struct volume_group *vg, char *pv_name);
 int vg_set_extent_size(vg_t *vg, uint32_t new_extent_size);
 int vg_set_max_lv(vg_t *vg, uint32_t max_lv);
 int vg_set_max_pv(vg_t *vg, uint32_t max_pv);
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 838add8..fb6f91b 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -532,6 +532,63 @@ int vg_extend(struct volume_group *vg, int pv_count, char **pv_names)
 	return 0;
 }
 
+int vg_reduce(struct volume_group *vg, char *pv_name)
+{
+	struct physical_volume *pv;
+	struct cmd_context *cmd = vg->cmd;
+	struct pv_list *pvl;
+
+	if (_vg_bad_status_bits(vg, RESIZEABLE_VG))
+		return 0;
+
+	if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE)) {
+		log_error("Can't get lock for orphan PVs");
+		return 0;
+	}
+
+	if (!archive(vg))
+		goto bad;
+
+	/* remove each pv */
+	if (!(pvl = find_pv_in_vg(vg, pv_name))) {
+		log_error("Physical volume %s not in volume group %s.",
+			  pv_name, vg->name);
+		goto bad;
+	}
+
+	pv = pvl->pv;
+
+	if (pv_pe_alloc_count(pv)) {
+		log_error("Physical volume %s still in use.",
+			  pv_name);
+		goto bad;
+	}
+
+	if (!dev_get_size(pv_dev(pv), &pv->size)) {
+		log_error("%s: Couldn't get size.", pv_name);
+		goto bad;
+	}
+
+	vg->pv_count--;
+	vg->free_count -= pv_pe_count(pv) - pv_pe_alloc_count(pv);
+	vg->extent_count -= pv_pe_count(pv);
+
+	/* add pv to the remove_pvs list */
+	dm_list_del(&pvl->list);
+	dm_list_add(&vg->removed_pvs, &pvl->list);
+
+	unlock_vg(cmd, VG_ORPHANS);
+	return 1;
+
+      bad:
+	log_error("Unable to add physical volume '%s' to "
+		  "volume group '%s'.", pv_name, vg->name);
+	unlock_vg(cmd, VG_ORPHANS);
+	if (pvl)
+		dm_list_del(&pvl->list);
+	return 0;
+}
+
 const char *strip_dir(const char *vg_name, const char *dev_dir)
 {
 	size_t len = strlen(dev_dir);
@@ -660,6 +717,9 @@ vg_t *vg_create(struct cmd_context *cmd, const char *vg_name)
 
 	dm_list_init(&vg->tags);
 
+	/* initialize removed_pvs list */
+	dm_list_init(&vg->removed_pvs);
+
 	if (!(vg->fid = cmd->fmt->ops->create_instance(cmd->fmt, vg_name,
 						       NULL, NULL))) {
 		log_error("Failed to create format instance");
@@ -2165,6 +2225,7 @@ static struct volume_group *_vg_read_orphans(struct cmd_context *cmd,
 	dm_list_init(&vg->pvs);
 	dm_list_init(&vg->lvs);
 	dm_list_init(&vg->tags);
+	dm_list_init(&vg->removed_pvs);
 	vg->vgmem = mem;
 	vg->cmd = cmd;
 	if (!(vg->name = dm_pool_strdup(mem, orphan_vgname))) {
-- 
1.6.2.5

[PATCH 2/3] Add lvm_vg_reduce to liblvm2app

[Thomas Woerner]

Extend lvm_vg_write to remove pvs removed in lvm_vg_reduce. The lvm
volume_group internal structure removed_pvs is used for that. The list is
empty afterwards.

Add new test for vg->pvs emptyness to lvm_vg_write to prevent to write empty
vgs. This is needed cbecause of lvm_vg_reduce and lv_vg_create, which creates
empty vgs.

Signed-off-by: Thomas Woerner <twoerner@redhat.com>
---
 liblvm/.exported_symbols |    1 +
 liblvm/lvm.h             |   18 +++++++
 liblvm/lvm_vg.c          |   25 ++++++++++
 test/api/vgtest.c        |  118 +++++++++++++++++++++++++++++-----------------
 4 files changed, 119 insertions(+), 43 deletions(-)

diff --git a/liblvm/.exported_symbols b/liblvm/.exported_symbols
index 9911423..68edb87 100644
--- a/liblvm/.exported_symbols
+++ b/liblvm/.exported_symbols
@@ -21,6 +21,7 @@ lvm_lv_is_active
 lvm_lv_is_suspended
 lvm_vg_create
 lvm_vg_extend
+lvm_vg_reduce
 lvm_vg_set_extent_size
 lvm_vg_write
 lvm_vg_open
diff --git a/liblvm/lvm.h b/liblvm/lvm.h
index 328b33e..92e946c 100644
--- a/liblvm/lvm.h
+++ b/liblvm/lvm.h
@@ -319,6 +319,24 @@ int lvm_vg_close(vg_t *vg);
 int lvm_vg_extend(vg_t *vg, const char *device);
 
 /**
+ * Reduce a VG by removeing an unused device.
+ *
+ * This API requires calling lvm_vg_write to commit the change to disk.
+ * After successfully removing a device, use lvm_vg_write to commit the new VG
+ * to disk.  Upon failure, retry the operation or release the VG handle with
+ * lvm_vg_close.
+ *
+ * \param   vg
+ *          VG handle obtained from lvm_vg_create.
+ *
+ * \param   device
+ *          Name of device to remove from VG.
+ *
+ * \return  Status code of 1 (success) or 0 (failure).
+ */
+int lvm_vg_reduce(vg_t *vg, const char *device);
+
+/**
  * Set the extent size of a VG.
  *
  * This API requires calling lvm_vg_write to commit the change to disk.
diff --git a/liblvm/lvm_vg.c b/liblvm/lvm_vg.c
index 4800555..60c6b31 100644
--- a/liblvm/lvm_vg.c
+++ b/liblvm/lvm_vg.c
@@ -68,6 +68,14 @@ int lvm_vg_extend(vg_t *vg, const char *device)
 	return 0;
 }
 
+int lvm_vg_reduce(vg_t *vg, const char *device)
+{
+	if (vg_read_error(vg))
+		return 0;
+
+	return vg_reduce(vg, (char *)device);
+}
+
 int lvm_vg_set_extent_size(vg_t *vg, uint32_t new_size)
 {
 	if (vg_read_error(vg))
@@ -80,15 +88,32 @@ int lvm_vg_set_extent_size(vg_t *vg, uint32_t new_size)
 
 int lvm_vg_write(vg_t *vg)
 {
+	struct pv_list *pvl;
+
 	if (vg_read_error(vg))
 		return -1;
 
+	if (dm_list_empty(&vg->pvs)) {
+		log_error("Volume group %s does not contain any physical volumes.\n", vg->name);
+		return -1;
+	}
+
 	if (!archive(vg))
 		return -1;
 
 	/* Store VG on disk(s) */
 	if (!vg_write(vg) || !vg_commit(vg))
 		return -1;
+
+	if (! dm_list_empty(&vg->removed_pvs)) {
+		dm_list_iterate_items(pvl, &vg->removed_pvs) {
+			/* FIXME: either use pv_write_orphan or remove the pv */
+			/* pv_write_orphan(vg->cmd, pvl->pv); */
+			label_remove(pv_dev(pvl->pv));
+		}
+		dm_list_init(&vg->removed_pvs);
+	}
+
 	return 0;
 }
 
diff --git a/test/api/vgtest.c b/test/api/vgtest.c
index 4ab9a46..0570e7a 100644
--- a/test/api/vgtest.c
+++ b/test/api/vgtest.c
@@ -26,8 +26,63 @@ lvm_t handle;
 vg_t *vg;
 const char *vg_name = "my_vg";
 const char *device = "/dev/loop3";
+const char *device2 = "/dev/loop4";
 uint64_t size = 1024;
 
+#define vg_create(vg_name) \
+	printf("Creating VG %s\n", vg_name); \
+	vg = lvm_vg_create(handle, vg_name); \
+	if (!vg) { \
+		fprintf(stderr, "Error creating volume group %s\n", vg_name); \
+		goto bad; \
+	}
+#define vg_extend(vg, dev) \
+	printf("Extending VG %s by %s\n", vg_name, dev); \
+	status = lvm_vg_extend(vg, dev); \
+	if (status) { \
+		fprintf(stderr, "Error extending volume group %s " \
+			"with device %s\n", vg_name, dev); \
+		goto bad; \
+	}
+#define vg_commit(vg) \
+	printf("Committing VG %s to disk\n", vg_name); \
+	status = lvm_vg_write(vg); \
+	if (status) { \
+		fprintf(stderr, "Creation of volume group '%s' on " \
+			"device '%s' failed\n", \
+			lvm_vg_get_name(vg), device); \
+		goto bad; \
+	}
+#define vg_open(vg_name, mode) \
+	printf("Opening VG %s %s\n", vg_name, mode); \
+	vg = lvm_vg_open(handle, vg_name, mode, 0); \
+	if (!vg) { \
+		fprintf(stderr, "Error opening volume group %s\n", vg_name); \
+		goto bad; \
+	}
+#define vg_close(vg) \
+	printf("Closing VG %s\n", vg_name); \
+	if (lvm_vg_close(vg)) { \
+		fprintf(stderr, "Error closing volume group %s\n", vg_name); \
+		goto bad; \
+	}
+#define vg_reduce(vg, dev) \
+ 	printf("Reducing VG %s by %s\n", vg_name, dev); \
+	status = lvm_vg_reduce(vg, dev); \
+	if (!status) { \
+		fprintf(stderr, "Error reducing volume group %s " \
+			"by device %s\n", vg_name, dev); \
+		goto bad; \
+	}
+#define vg_remove(vg) \
+ 	printf("Removing VG %s from system\n", vg_name); \
+	status = lvm_vg_remove(vg); \
+	if (status) { \
+		fprintf(stderr, "Revmoval of volume group '%s' failed\n", \
+			vg_name); \
+		goto bad; \
+	}
+
 int main(int argc, char *argv[])
 {
 	int status;
@@ -41,20 +96,8 @@ int main(int argc, char *argv[])
 		goto bad;
 	}
 
-	printf("Creating VG %s\n", vg_name);
-	vg = lvm_vg_create(handle, vg_name);
-	if (!vg) {
-		fprintf(stderr, "Error creating volume group %s\n", vg_name);
-		goto bad;
-	}
-
-	printf("Extending VG %s\n", vg_name);
-	status = lvm_vg_extend(vg, device);
-	if (status) {
-		fprintf(stderr, "Error extending volume group %s "
-			"with device %s\n", vg_name, device);
-		goto bad;
-	}
+	vg_create(vg_name);
+	vg_extend(vg, device);
 
 	printf("Setting VG %s extent_size to %"PRIu64"\n", vg_name, size);
 	status = lvm_vg_set_extent_size(vg, size);
@@ -65,36 +108,25 @@ int main(int argc, char *argv[])
 		goto bad;
 	}
 
-	printf("Committing VG %s to disk\n", vg_name);
-	status = lvm_vg_write(vg);
-	if (status) {
-		fprintf(stderr, "Creation of volume group '%s' on "
-			"device '%s' failed\n",
-			vg_name, device);
-		goto bad;
-	}
+	vg_commit(vg);
+	vg_close(vg);
+	
+	vg_open(vg_name, "r");
+	vg_close(vg);
 
-	printf("Closing VG %s\n", vg_name);
-	if (lvm_vg_close(vg))
-		goto bad;
-	printf("Re-opening VG %s for reading\n", vg_name);
-	vg = lvm_vg_open(handle, vg_name, "r", 0);
-	if (!vg)
-		goto bad;
-	printf("Closing VG %s\n", vg_name);
-	if (lvm_vg_close(vg))
-		goto bad;
-	printf("Re-opening VG %s for writing\n", vg_name);
-	vg = lvm_vg_open(handle, vg_name, "w", 0);
-	if (!vg)
-		goto bad;
-	printf("Removing VG %s from system\n", vg_name);
-	status = lvm_vg_remove(vg);
-	if (status) {
-		fprintf(stderr, "Revmoval of volume group '%s' failed\n",
-			vg_name);
-		goto bad;
-	}
+	vg_open(vg_name, "w");
+	vg_extend(vg, device2);
+	vg_reduce(vg, device);
+	vg_commit(vg);
+	vg_close(vg);
+
+	vg_open(vg_name, "w");
+	vg_extend(vg, device);
+	vg_commit(vg);
+	vg_close(vg);
+
+	vg_open(vg_name, "w");
+	vg_remove(vg);
 
 	lvm_vg_close(vg);
 	lvm_destroy(handle);
-- 
1.6.2.5

[PATCH 3/3] Initialize removed_pvs list in format-specific volume_group constructors.

[Thomas Woerner]

From: Dave Wysochanski <dwysocha@redhat.com>

I think this is the reason the 'n' and 'p' members were still NULL.
Ideally, we should have a base constructor here that initializes the general,
non-format specific members of struct volume_group.  But until then, there
are multiple places to initialize these members.  Maybe a better patch would
be a base constructor patch for struct volume_group.  That is more work
though.  Maybe you could start one - might get bonus points.  ;-)

Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
Signed-off-by: Thomas Woerner <twoerner@redhat.com>
---
 lib/format1/format1.c         |    1 +
 lib/format_pool/format_pool.c |    1 +
 lib/format_text/import_vsn1.c |    1 +
 3 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/lib/format1/format1.c b/lib/format1/format1.c
index c8f9519..aa1483a 100644
--- a/lib/format1/format1.c
+++ b/lib/format1/format1.c
@@ -132,6 +132,7 @@ static struct volume_group *_build_vg(struct format_instance *fid,
 	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))
 		goto_bad;
diff --git a/lib/format_pool/format_pool.c b/lib/format_pool/format_pool.c
index 3f31bba..6f7e4b4 100644
--- a/lib/format_pool/format_pool.c
+++ b/lib/format_pool/format_pool.c
@@ -124,6 +124,7 @@ static struct volume_group *_build_vg_from_pds(struct format_instance
 	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;
diff --git a/lib/format_text/import_vsn1.c b/lib/format_text/import_vsn1.c
index 629310a..3c41675 100644
--- a/lib/format_text/import_vsn1.c
+++ b/lib/format_text/import_vsn1.c
@@ -753,6 +753,7 @@ static struct volume_group *_read_vg(struct format_instance *fid,
 
 	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")) &&
-- 
1.6.2.5