[PATCH 0/7] Add vg/lv tag addition/deletion to lvm2app

[PATCH 0/7] Add vg/lv tag addition/deletion to lvm2app

[Dave Wysochanski]

This patchset adds lv/vg tag addition/deletion to lvm2app.
pv tags will be handled later when the pv refactoring patches
are done.  Partially fixes rhbz 562909.

Patchset is fairly straightforward.  When adding a tag, we need
to copy the tag memory before adding it to the list (see patch #4).
The copy could be placed inside lvm2app or in the internal library.
I chose the internal library to minmize code in lvm2app.  Another
finer point is what to return when the 'get_tags' lvm2app function
is called but there's no tags - a NULL pointer or an empty list.
For now I've chosen a NULL pointer to avoid allocation of memory.
There is some inconsistency here in lvm2app but the behavior is
documented in the header for all functions that return lists.

[PATCH 1/7] Refactor _vgchange_tag() to vg_change_tag() library function.

[Dave Wysochanski]

Pull out common code to be called from tools as well as lvm2app.
Leave archive() at tool level so we can use from vgcreate
as well as vgchange.  Should be no functional change.

Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
---
 lib/metadata/metadata-exported.h |    1 +
 lib/metadata/metadata.c          |   23 +++++++++++++++++++++++
 tools/vgchange.c                 |   20 ++------------------
 3 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index 164ce36..fae9551 100644
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -463,6 +463,7 @@ int vg_rename(struct cmd_context *cmd, struct volume_group *vg,
 int vg_extend(struct volume_group *vg, int pv_count, char **pv_names,
 	      struct pvcreate_params *pp);
 int vg_reduce(struct volume_group *vg, char *pv_name);
+int vg_change_tag(struct volume_group *vg, const char *tag, int add_tag);
 int vg_set_extent_size(struct volume_group *vg, uint32_t new_extent_size);
 int vg_set_max_lv(struct volume_group *vg, uint32_t max_lv);
 int vg_set_max_pv(struct volume_group *vg, uint32_t max_pv);
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index b3b3778..52bbc46 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -665,6 +665,29 @@ int vg_reduce(struct volume_group *vg, char *pv_name)
 	return 0;
 }
 
+int vg_change_tag(struct volume_group *vg, const char *tag, int add_tag)
+{
+	if (!(vg->fid->fmt->features & FMT_TAGS)) {
+		log_error("Volume group %s does not support tags", vg->name);
+		return 0;
+	}
+
+	if (add_tag) {
+		if (!str_list_add(vg->vgmem, &vg->tags, tag)) {
+			log_error("Failed to add tag %s to volume group %s",
+				  tag, vg->name);
+			return 0;
+		}
+	} else {
+		if (!str_list_del(&vg->tags, tag)) {
+			log_error("Failed to remove tag %s from volume group "
+				  "%s", tag, vg->name);
+			return 0;
+		}
+	}
+	return 1;
+}
+
 const char *strip_dir(const char *vg_name, const char *dev_dir)
 {
 	size_t len = strlen(dev_dir);
diff --git a/tools/vgchange.c b/tools/vgchange.c
index b4f5d00..8f4439e 100644
--- a/tools/vgchange.c
+++ b/tools/vgchange.c
@@ -447,29 +447,13 @@ static int _vgchange_tag(struct cmd_context *cmd, struct volume_group *vg,
 		return ECMD_FAILED;
 	}
 
-	if (!(vg->fid->fmt->features & FMT_TAGS)) {
-		log_error("Volume group %s does not support tags", vg->name);
-		return ECMD_FAILED;
-	}
-
 	if (!archive(vg)) {
 		stack;
 		return ECMD_FAILED;
 	}
 
-	if ((arg == addtag_ARG)) {
-		if (!str_list_add(cmd->mem, &vg->tags, tag)) {
-			log_error("Failed to add tag %s to volume group %s",
-				  tag, vg->name);
-			return ECMD_FAILED;
-		}
-	} else {
-		if (!str_list_del(&vg->tags, tag)) {
-			log_error("Failed to remove tag %s from volume group "
-				  "%s", tag, vg->name);
-			return ECMD_FAILED;
-		}
-	}
+	if (!vg_change_tag(vg, tag, arg == addtag_ARG))
+		return ECMD_FAILED;
 
 	if (!vg_write(vg) || !vg_commit(vg)) {
 		stack;
-- 
1.6.0.6

[PATCH 2/7] Refactor vgcreate to call new vg_change_tag() function.

[Dave Wysochanski]

Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
---
 tools/vgcreate.c |   10 +---------
 1 files changed, 1 insertions(+), 9 deletions(-)

diff --git a/tools/vgcreate.c b/tools/vgcreate.c
index 5113b1f..75440c2 100644
--- a/tools/vgcreate.c
+++ b/tools/vgcreate.c
@@ -94,16 +94,8 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv)
 			goto bad;
 		}
 
-		if (!(vg->fid->fmt->features & FMT_TAGS)) {
-			log_error("Volume group format does not support tags");
+		if (!vg_change_tag(vg, tag, 1))
 			goto bad;
-		}
-
-		if (!str_list_add(cmd->mem, &vg->tags, tag)) {
-			log_error("Failed to add tag %s to volume group %s",
-				  tag, vp_new.vg_name);
-			goto bad;
-		}
 	}
 
 	if (vg_is_clustered(vg)) {
-- 
1.6.0.6

[PATCH 3/7] Refactor lvchange_tag() to call lv_change_tag() library function.

[Dave Wysochanski]

Similar refactoring to vgchange - pull out common parts and put into
library function for reuse.  Should be no functional change.

Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
---
 lib/metadata/metadata-exported.h |    2 ++
 lib/metadata/metadata.c          |   24 ++++++++++++++++++++++++
 tools/lvchange.c                 |   19 +------------------
 3 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index fae9551..9540285 100644
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -493,6 +493,8 @@ struct logical_volume *lv_create_empty(const char *name,
 int set_lv(struct cmd_context *cmd, struct logical_volume *lv,
            uint64_t sectors, int value);
 
+int lv_change_tag(struct logical_volume *lv, const char *tag, int add_tag);
+
 /* Reduce the size of an LV by extents */
 int lv_reduce(struct logical_volume *lv, uint32_t extents);
 
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 52bbc46..2b8a1be 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -665,6 +665,30 @@ int vg_reduce(struct volume_group *vg, char *pv_name)
 	return 0;
 }
 
+int lv_change_tag(struct logical_volume *lv, const char *tag, int add_tag)
+{
+	if (!(lv->vg->fid->fmt->features & FMT_TAGS)) {
+		log_error("Logical volume %s/%s does not support tags",
+			  lv->vg->name, lv->name);
+		return 0;
+	}
+
+	if (add_tag) {
+		if (!str_list_add(lv->vg->vgmem, &lv->tags, tag)) {
+			log_error("Failed to add tag %s to %s/%s",
+				  tag, lv->vg->name, lv->name);
+			return 0;
+		}
+	} else {
+		if (!str_list_del(&lv->tags, tag)) {
+			log_error("Failed to remove tag %s from %s/%s",
+				  tag, lv->vg->name, lv->name);
+			return 0;
+		}
+	}
+	return 1;
+}
+
 int vg_change_tag(struct volume_group *vg, const char *tag, int add_tag)
 {
 	if (!(vg->fid->fmt->features & FMT_TAGS)) {
diff --git a/tools/lvchange.c b/tools/lvchange.c
index 5118e51..862a709 100644
--- a/tools/lvchange.c
+++ b/tools/lvchange.c
@@ -500,25 +500,8 @@ static int lvchange_tag(struct cmd_context *cmd, struct logical_volume *lv,
 		return 0;
 	}
 
-	if (!(lv->vg->fid->fmt->features & FMT_TAGS)) {
-		log_error("Logical volume %s/%s does not support tags",
-			  lv->vg->name, lv->name);
+	if (!lv_change_tag(lv, tag, arg == addtag_ARG))
 		return 0;
-	}
-
-	if ((arg == addtag_ARG)) {
-		if (!str_list_add(cmd->mem, &lv->tags, tag)) {
-			log_error("Failed to add tag %s to %s/%s",
-				  tag, lv->vg->name, lv->name);
-			return 0;
-		}
-	} else {
-		if (!str_list_del(&lv->tags, tag)) {
-			log_error("Failed to remove tag %s from %s/%s",
-				  tag, lv->vg->name, lv->name);
-			return 0;
-		}
-	}
 
 	log_very_verbose("Updating logical volume \"%s\" on disk(s)", lv->name);
 
-- 
1.6.0.6

[PATCH 4/7] Add _tag_copy() to allocate memory and copy a tag in {lv|vg}_change_tag()

[Dave Wysochanski]

We need to allocate memory for the tag and copy the tag value before we
add it to the list of tags.  We could put this inside lvm2app since the
tools keep their memory around until vg_write/vg_commit is called.
Call _tag_copy() from {vg|lv}_change_tag().  We need to copy the tag passed
in by the caller to ensure the memory until the {vg|lv} handle is released.

Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
---
 lib/metadata/metadata.c |   22 ++++++++++++++++++++--
 1 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 2b8a1be..e0d63c3 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -665,8 +665,22 @@ int vg_reduce(struct volume_group *vg, char *pv_name)
 	return 0;
 }
 
+static char *_tag_copy(struct dm_pool *p, const char *tag)
+{
+	char *tag_new;
+
+	/* FIXME: verify tag length */
+	if (!(tag_new = dm_pool_alloc(p, strlen(tag)))) {
+		log_error("Failed to alloc memory for tag %s.", tag);
+		return NULL;
+	}
+	strcpy(tag_new, tag);
+	return tag_new;
+}
+
 int lv_change_tag(struct logical_volume *lv, const char *tag, int add_tag)
 {
+	char *tag_new;
 	if (!(lv->vg->fid->fmt->features & FMT_TAGS)) {
 		log_error("Logical volume %s/%s does not support tags",
 			  lv->vg->name, lv->name);
@@ -674,7 +688,8 @@ int lv_change_tag(struct logical_volume *lv, const char *tag, int add_tag)
 	}
 
 	if (add_tag) {
-		if (!str_list_add(lv->vg->vgmem, &lv->tags, tag)) {
+		tag_new = _tag_copy(lv->vg->vgmem, tag);
+		if (!str_list_add(lv->vg->vgmem, &lv->tags, tag_new)) {
 			log_error("Failed to add tag %s to %s/%s",
 				  tag, lv->vg->name, lv->name);
 			return 0;
@@ -691,13 +706,16 @@ int lv_change_tag(struct logical_volume *lv, const char *tag, int add_tag)
 
 int vg_change_tag(struct volume_group *vg, const char *tag, int add_tag)
 {
+	char *tag_new;
+
 	if (!(vg->fid->fmt->features & FMT_TAGS)) {
 		log_error("Volume group %s does not support tags", vg->name);
 		return 0;
 	}
 
 	if (add_tag) {
-		if (!str_list_add(vg->vgmem, &vg->tags, tag)) {
+		tag_new = _tag_copy(vg->vgmem, tag);
+		if (!str_list_add(vg->vgmem, &vg->tags, tag_new)) {
 			log_error("Failed to add tag %s to volume group %s",
 				  tag, vg->name);
 			return 0;
-- 
1.6.0.6

[PATCH 5/7] Add lvm_vg_get_tags(), lvm_vg_add_tag(), and lvm_vg_remove_tag().

[Dave Wysochanski]

Add lvm2app functions to manage VG tags.
For lvm_vg_get_tags(), we return a list of tags, similar to other
functions that return lists.  NULL is returned if there is no
tags.

Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
---
 liblvm/.exported_symbols |    3 ++
 liblvm/lvm2app.h         |   49 +++++++++++++++++++++++++++++++++++++++-
 liblvm/lvm_vg.c          |   55 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 105 insertions(+), 2 deletions(-)

diff --git a/liblvm/.exported_symbols b/liblvm/.exported_symbols
index c184a88..7431248 100644
--- a/liblvm/.exported_symbols
+++ b/liblvm/.exported_symbols
@@ -18,6 +18,7 @@ lvm_vg_get_extent_size
 lvm_vg_get_extent_count
 lvm_vg_get_free_extent_count
 lvm_vg_get_pv_count
+lvm_vg_get_tags
 lvm_lv_activate
 lvm_lv_deactivate
 lvm_lv_get_uuid
@@ -33,6 +34,8 @@ lvm_vg_write
 lvm_vg_open
 lvm_vg_close
 lvm_vg_remove
+lvm_vg_add_tag
+lvm_vg_remove_tag
 lvm_scan
 lvm_errno
 lvm_errmsg
diff --git a/liblvm/lvm2app.h b/liblvm/lvm2app.h
index 39bfff8..b21d0e9 100644
--- a/liblvm/lvm2app.h
+++ b/liblvm/lvm2app.h
@@ -159,10 +159,10 @@ typedef struct lvm_pv_list {
  * Lists of these structures are returned by lvm_list_vg_names and
  * lvm_list_vg_uuids.
  */
-struct lvm_str_list {
+typedef struct lvm_str_list {
 	struct dm_list list;
 	const char *str;
-};
+} lvm_str_list_t;
 
 /*************************** generic lvm handling ***************************/
 /**
@@ -458,6 +458,26 @@ int lvm_vg_extend(vg_t vg, const char *device);
 int lvm_vg_reduce(vg_t vg, const char *device);
 
 /**
+ * Add/remove a tag to/from a VG.
+ *
+ * These functions require calling lvm_vg_write to commit the change to disk.
+ * After successfully adding/removing a tag, 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 or lvm_vg_open.
+ *
+ * \param   tag
+ * Tag to add/remove to/from VG.
+ *
+ * \return
+ * 0 (success) or -1 (failure).
+ */
+int lvm_vg_add_tag(vg_t vg, const char *tag);
+int lvm_vg_remove_tag(vg_t vg, const char *tag);
+
+/**
  * Set the extent size of a VG.
  *
  * This function requires calling lvm_vg_write to commit the change to disk.
@@ -644,6 +664,31 @@ uint64_t lvm_vg_get_max_pv(const vg_t vg);
  */
 uint64_t lvm_vg_get_max_lv(const vg_t vg);
 
+/**
+ * Return the list of volume group tags.
+ *
+ * The memory allocated for the list is tied to the vg_t handle and will be
+ * released when lvm_vg_close is called.
+ *
+ * To process the list, use the dm_list iterator functions.  For example:
+ *      vg_t vg;
+ *      struct dm_list *tags;
+ *      struct lvm_str_list *strl;
+ *
+ *      tags = lvm_vg_get_tags(vg);
+ *	dm_list_iterate_items(strl, tags) {
+ *		tag = strl->str;
+ *              // do something with tag
+ *      }
+ *
+ *
+ * \return
+ * A list with entries of type struct lvm_str_list, containing the
+ * tag strings attached to volume group.
+ * If no tags are attached to the given VG, NULL is returned.
+ */
+struct dm_list *lvm_vg_get_tags(const vg_t vg);
+
 /************************** logical volume handling *************************/
 
 /**
diff --git a/liblvm/lvm_vg.c b/liblvm/lvm_vg.c
index 0c166d5..7486154 100644
--- a/liblvm/lvm_vg.c
+++ b/liblvm/lvm_vg.c
@@ -25,6 +25,34 @@
 #include <errno.h>
 #include <string.h>
 
+int lvm_vg_add_tag(vg_t vg, const char *tag)
+{
+	if (vg_read_error(vg))
+		return -1;
+
+	if (!vg_check_write_mode(vg))
+		return -1;
+
+	if (!vg_change_tag(vg, tag, 1))
+		return -1;
+	return 0;
+}
+
+
+int lvm_vg_remove_tag(vg_t vg, const char *tag)
+{
+	if (vg_read_error(vg))
+		return -1;
+
+	if (!vg_check_write_mode(vg))
+		return -1;
+
+	if (!vg_change_tag(vg, tag, 0))
+		return -1;
+	return 0;
+}
+
+
 vg_t lvm_vg_create(lvm_t libh, const char *vg_name)
 {
 	struct volume_group *vg;
@@ -233,6 +261,33 @@ struct dm_list *lvm_vg_list_lvs(vg_t vg)
 	return list;
 }
 
+struct dm_list *lvm_vg_get_tags(const vg_t vg)
+{
+	struct dm_list *list;
+	lvm_str_list_t *lsl;
+	struct str_list *sl;
+
+	if (dm_list_empty(&vg->tags))
+		return NULL;
+
+	if (!(list = dm_pool_zalloc(vg->vgmem, sizeof(*list)))) {
+		log_errno(ENOMEM, "Memory allocation fail for dm_list.");
+		return NULL;
+	}
+	dm_list_init(list);
+
+	dm_list_iterate_items(sl, &vg->tags) {
+		if (!(lsl = dm_pool_zalloc(vg->vgmem, sizeof(*lsl)))) {
+			log_errno(ENOMEM,
+				"Memory allocation fail for lvm_lv_list.");
+			return NULL;
+		}
+		lsl->str = dm_pool_strdup(vg->vgmem, sl->str);
+		dm_list_add(list, &lsl->list);
+	}
+	return list;
+}
+
 uint64_t lvm_vg_get_seqno(const vg_t vg)
 {
 	return vg_seqno(vg);
-- 
1.6.0.6

[PATCH 6/7] Add lvm_lv_get_tags(), lvm_lv_add_tag(), and lvm_lv_remove_tag().

[Dave Wysochanski]

Add lvm2app functions to manage VG tags.
For lvm_lv_get_tags(), we return a list of tags, similar to other
functions that return lists.  NULL is returned if there are no
tags.

Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
---
 liblvm/.exported_symbols |    3 ++
 liblvm/lvm2app.h         |   46 ++++++++++++++++++++++++++++++++++++
 liblvm/lvm_lv.c          |   58 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 107 insertions(+), 0 deletions(-)

diff --git a/liblvm/.exported_symbols b/liblvm/.exported_symbols
index 7431248..9b61d45 100644
--- a/liblvm/.exported_symbols
+++ b/liblvm/.exported_symbols
@@ -26,6 +26,9 @@ lvm_lv_get_name
 lvm_lv_get_size
 lvm_lv_is_active
 lvm_lv_is_suspended
+lvm_lv_add_tag
+lvm_lv_remove_tag
+lvm_lv_get_tags
 lvm_vg_create
 lvm_vg_extend
 lvm_vg_reduce
diff --git a/liblvm/lvm2app.h b/liblvm/lvm2app.h
index b21d0e9..46a9e74 100644
--- a/liblvm/lvm2app.h
+++ b/liblvm/lvm2app.h
@@ -833,6 +833,52 @@ uint64_t lvm_lv_is_active(const lv_t lv);
 uint64_t lvm_lv_is_suspended(const lv_t lv);
 
 /**
+ * Add/remove a tag to/from a LV.
+ *
+ * These functions require calling lvm_vg_write to commit the change to disk.
+ * After successfully adding/removing a tag, 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   lv
+ * Logical volume handle.
+ *
+ * \param   tag
+ * Tag to add/remove to/from LV.
+ *
+ * \return
+ * 0 (success) or -1 (failure).
+ */
+int lvm_lv_add_tag(lv_t lv, const char *tag);
+int lvm_lv_remove_tag(lv_t lv, const char *tag);
+
+/**
+ * Return the list of logical volume tags.
+ *
+ * The memory allocated for the list is tied to the vg_t handle and will be
+ * released when lvm_vg_close is called.
+ *
+ * To process the list, use the dm_list iterator functions.  For example:
+ *      lv_t lv;
+ *      struct dm_list *tags;
+ *      struct lvm_str_list *strl;
+ *
+ *      tags = lvm_lv_get_tags(lv);
+ *	dm_list_iterate_items(strl, tags) {
+ *		tag = strl->str;
+ *              // do something with tag
+ *      }
+ *
+ *
+ * \return
+ * A list with entries of type struct lvm_str_list, containing the
+ * tag strings attached to volume group.
+ * If no tags are attached to the given VG, NULL is returned.
+ */
+struct dm_list *lvm_lv_get_tags(const lv_t lv);
+
+
+/**
  * Resize logical volume to new_size bytes.
  *
  * NOTE: This function is currently not implemented.
diff --git a/liblvm/lvm_lv.c b/liblvm/lvm_lv.c
index 7a37d2c..0ac7f79 100644
--- a/liblvm/lvm_lv.c
+++ b/liblvm/lvm_lv.c
@@ -23,6 +23,15 @@
 
 #include <string.h>
 
+static int _lv_check_handle(const lv_t lv, const int vg_writeable)
+{
+	if (!lv || !lv->vg || vg_read_error(lv->vg))
+		return -1;
+	if (vg_writeable && !vg_check_write_mode(lv->vg))
+		return -1;
+	return 0;
+}
+
 /* FIXME: have lib/report/report.c _disp function call lv_size()? */
 uint64_t lvm_lv_get_size(const lv_t lv)
 {
@@ -68,6 +77,55 @@ uint64_t lvm_lv_is_suspended(const lv_t lv)
 	return 0;
 }
 
+int lvm_lv_add_tag(lv_t lv, const char *tag)
+{
+	if (_lv_check_handle(lv, 1))
+		return -1;
+	if (!lv_change_tag(lv, tag, 1))
+		return -1;
+	return 0;
+}
+
+
+int lvm_lv_remove_tag(lv_t lv, const char *tag)
+{
+	if (_lv_check_handle(lv, 1))
+		return -1;
+	if (!lv_change_tag(lv, tag, 0))
+		return -1;
+	return 0;
+}
+
+
+struct dm_list *lvm_lv_get_tags(const lv_t lv)
+{
+	struct dm_list *list;
+	lvm_str_list_t *lsl;
+	struct str_list *sl;
+
+	if (_lv_check_handle(lv, 0))
+		return NULL;
+	if (dm_list_empty(&lv->tags))
+		return NULL;
+
+	if (!(list = dm_pool_zalloc(lv->vg->vgmem, sizeof(*list)))) {
+		log_errno(ENOMEM, "Memory allocation fail for dm_list.");
+		return NULL;
+	}
+	dm_list_init(list);
+
+	dm_list_iterate_items(sl, &lv->tags) {
+		if (!(lsl = dm_pool_zalloc(lv->vg->vgmem, sizeof(*lsl)))) {
+			log_errno(ENOMEM,
+				"Memory allocation fail for lvm_lv_list.");
+			return NULL;
+		}
+		lsl->str = dm_pool_strdup(lv->vg->vgmem, sl->str);
+		dm_list_add(list, &lsl->list);
+	}
+	return list;
+}
+
 /* Set defaults for non-segment specific LV parameters */
 static void _lv_set_default_params(struct lvcreate_params *lp,
 				   vg_t vg, const char *lvname,
-- 
1.6.0.6

[PATCH 7/7] Update lvm2app interactive unit test for vg/lv tags.

[Dave Wysochanski]

Simple test of vg/lv tag addition/deletion.

Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
---
 test/api/test.c |  105 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 99 insertions(+), 6 deletions(-)

diff --git a/test/api/test.c b/test/api/test.c
index 6e222da..98f77e4 100644
--- a/test/api/test.c
+++ b/test/api/test.c
@@ -89,6 +89,14 @@ static void _show_help(void)
 	       "Issue a lvm_config_reload() API to reload LVM config\n");
 	printf("'config_override' device: "
 	       "Issue a lvm_config_override() with accept device filter\n");
+	printf("'vg_get_tags vgname': "
+	       "List the tags of a VG\n");
+	printf("'lv_get_tags vgname lvname': "
+	       "List the tags of a LV\n");
+	printf("'vg_{add|remove}_tag vgname tag': "
+	       "Add/remove a tag from a VG\n");
+	printf("'lv_{add|remove}_tag vgname lvname tag': "
+	       "Add/remove a tag from a LV\n");
 	printf("'quit': exit the program\n");
 }
 
@@ -468,13 +476,11 @@ static void _list_vg_names(lvm_t libh)
 {
 	struct dm_list *list;
 	struct lvm_str_list *strl;
-	const char *tmp;
 
 	list = lvm_list_vg_names(libh);
 	printf("VG names:\n");
 	dm_list_iterate_items(strl, list) {
-		tmp = strl->str;
-		printf("%s\n", tmp);
+		printf("%s\n", strl->str);
 	}
 }
 
@@ -482,17 +488,91 @@ static void _list_vg_ids(lvm_t libh)
 {
 	struct dm_list *list;
 	struct lvm_str_list *strl;
-	const char *tmp;
 
 	list = lvm_list_vg_uuids(libh);
 	printf("VG uuids:\n");
 	dm_list_iterate_items(strl, list) {
-		tmp = strl->str;
-		printf("%s\n", tmp);
+		printf("%s\n", strl->str);
 	}
 }
 
+static void _display_tags(struct dm_list *list)
+{
+	struct lvm_str_list *strl;
+	if (!list || dm_list_empty(list)) {
+		printf("No tags exist\n");
+		return;
+	}
+	dm_list_iterate_items(strl, list) {
+		printf("%s\n", strl->str);
+	}
+}
+
+static void _vg_get_tags(char **argv, int argc)
+{
+	vg_t vg;
+
+	if (!(vg = _lookup_vg_by_name(argv, argc)))
+		return;
+	printf("VG tags:\n");
+	_display_tags(lvm_vg_get_tags(vg));
+}
+
+static void _vg_tag(char **argv, int argc, int add)
+{
+	vg_t vg;
+
+	if (argc < 3) {
+		printf("Please enter vgname, tag\n");
+		return;
+	}
+	if (!(vg = _lookup_vg_by_name(argv, argc)))
+		return;
+	if (add && lvm_vg_add_tag(vg, argv[2]))
+		printf("Error ");
+	else if (!add && lvm_vg_remove_tag(vg, argv[2])){
+		printf("Error ");
+	} else {
+		printf("Success ");
+	}
+	printf("%s tag %s to VG %s\n",
+	       add ? "adding":"removing", argv[2], argv[1]);
+}
+
+static void _lv_get_tags(char **argv, int argc)
+{
+	lv_t lv;
+
+	if (argc < 3) {
+		printf("Please enter vgname, lvname\n");
+		return;
+	}
+	if (!(lv = _lookup_lv_by_name(argv[2])))
+		return;
+	printf("LV tags:\n");
+	_display_tags(lvm_lv_get_tags(lv));
+}
 
+static void _lv_tag(char **argv, int argc, int add)
+{
+	lv_t lv;
+
+	if (argc < 3) {
+		printf("Please enter vgname, lvname\n");
+		return;
+	}
+	if (!(lv = _lookup_lv_by_name(argv[2])))
+		return;
+	if (add && lvm_lv_add_tag(lv, argv[3]))
+		printf("Error ");
+	else if (!add && lvm_lv_remove_tag(lv, argv[3])){
+		printf("Error ");
+	} else {
+		printf("Success ");
+	}
+	printf("%s tag %s to LV %s\n",
+	       add ? "adding":"removing", argv[3], argv[2]);
+}
 static void _lvs_in_vg(char **argv, int argc)
 {
 	struct dm_list *lvs;
@@ -549,6 +629,7 @@ static void _lv_activate(char **argv, int argc)
 	printf("activating LV %s in VG %s\n",
 		argv[2], argv[1]);
 }
+
 static void _vg_remove_lv(char **argv, int argc)
 {
 	lv_t lv;
@@ -670,6 +751,18 @@ static int lvmapi_test_shell(lvm_t libh)
 			_scan_vgs(libh);
 		} else if (!strcmp(argv[0], "vg_create_lv_linear")) {
 			_vg_create_lv_linear(argv, argc);
+		} else if (!strcmp(argv[0], "vg_add_tag")) {
+			_vg_tag(argv, argc, 1);
+		} else if (!strcmp(argv[0], "vg_remove_tag")) {
+			_vg_tag(argv, argc, 0);
+		} else if (!strcmp(argv[0], "vg_get_tags")) {
+			_vg_get_tags(argv, argc);
+		} else if (!strcmp(argv[0], "lv_add_tag")) {
+			_lv_tag(argv, argc, 1);
+		} else if (!strcmp(argv[0], "lv_remove_tag")) {
+			_lv_tag(argv, argc, 0);
+		} else if (!strcmp(argv[0], "lv_get_tags")) {
+			_lv_get_tags(argv, argc);
 		} else {
 			printf ("Unrecognized command %s\n", argv[0]);
 		}
-- 
1.6.0.6

[PATCH 7/7] Update lvm2app interactive unit test for vg/lv tags.

[Zdenek Kabelac]

On 16.2.2010 20:39, Dave Wysochanski wrote:
> Simple test of vg/lv tag addition/deletion.
> 
> Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
> ---
>  
>  	list = lvm_list_vg_uuids(libh);
>  	printf("VG uuids:\n");
>  	dm_list_iterate_items(strl, list) {
> -		tmp = strl->str;
> -		printf("%s\n", tmp);
> +		printf("%s\n", strl->str);


Simplier:  puts(strl->str);

adds trailing '\n' automaticaly.

Zdenek

[PATCH 4/7] Add _tag_copy() to allocate memory and copy a tag in {lv|vg}_change_tag()

[Zdenek Kabelac]

On 16.2.2010 20:39, Dave Wysochanski wrote:
> We need to allocate memory for the tag and copy the tag value before we
> add it to the list of tags.  We could put this inside lvm2app since the
> tools keep their memory around until vg_write/vg_commit is called.
> Call _tag_copy() from {vg|lv}_change_tag().  We need to copy the tag passed
> in by the caller to ensure the memory until the {vg|lv} handle is released.
> 
> Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
> ---
>  lib/metadata/metadata.c |   22 ++++++++++++++++++++--
>  1 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
> index 2b8a1be..e0d63c3 100644
> --- a/lib/metadata/metadata.c
> +++ b/lib/metadata/metadata.c
> @@ -665,8 +665,22 @@ int vg_reduce(struct volume_group *vg, char *pv_name)
>  	return 0;
>  }
>  
> +static char *_tag_copy(struct dm_pool *p, const char *tag)
> +{
> +	char *tag_new;
> +
> +	/* FIXME: verify tag length */
> +	if (!(tag_new = dm_pool_alloc(p, strlen(tag)))) {

strlen(tag) + 1
(for '\0')

But better way is to use directly  dm_pool_strdup() - and avoid whole
_tag_copy() function.

> +		log_error("Failed to alloc memory for tag %s.", tag);
> +		return NULL;
> +	}
> +	strcpy(tag_new, tag);
> +	return tag_new;
> +}
> +