[RFC PATCH 0/12] dm_report attribute handling and liblvm vg attributes

[RFC PATCH 0/12] dm_report attribute handling and liblvm vg attributes

[Dave Wysochanski]

The purpose of this patchset is to get feedback on one approach to handling
attributes in liblvm.  The most important patches are:
1/12: Adds dm_report_output_attribute() function and related structures.
9/12: Updates vg_open to store vg attributes in the handle.  This allows us
to use the report memory safely (we free it in vg_close().
10/12: Add lvm_vg_get_attr_list() and lvm_vg_get_attr().

What is missing in the patch is the mode (read/write) of the attribute.
I have not thought extensively about this but one option is as follows.
Add a 'mode' or 'is_writable' field to the dm structure, and set it to
'readonly' for all attributes output by dm_report_output_attribute().
Then in vg_open(), depending on the mode and capabilities of the current
release of liblvm code, we can gradually change various attributes to
writeable.

This patchset should build fine but with some warnings.  Attribute code
was exercised with test code in last patch.

[PATCH 01/12] Add dm_report_output_attributes() API to dm_report framework.

[Dave Wysochanski]

Add dm_report_output_attributes() which provides an alternative
output for a report - a list of attributes.
Add two new structs to support this function:
1. dm_report_attribute_type.  This describes a single attribute,
which is a field name,value pair
2. dm_report_attribute_list_type. This describes one row of a report,
as a list of attributes.

Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
---
 libdm/.exported_symbols |    1 +
 libdm/libdevmapper.h    |   36 +++++++++++++++++++++-
 libdm/libdm-report.c    |   79 +++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 113 insertions(+), 3 deletions(-)

diff --git a/libdm/.exported_symbols b/libdm/.exported_symbols
index a6e04f8..651f2b4 100644
--- a/libdm/.exported_symbols
+++ b/libdm/.exported_symbols
@@ -124,6 +124,7 @@ dm_asprintf
 dm_report_init
 dm_report_object
 dm_report_output
+dm_report_output_attributes
 dm_report_free
 dm_report_get_private
 dm_report_field_string
diff --git a/libdm/libdevmapper.h b/libdm/libdevmapper.h
index 93aecbb..b050212 100644
--- a/libdm/libdevmapper.h
+++ b/libdm/libdevmapper.h
@@ -30,7 +30,7 @@
 #include <stdio.h>
 
 /*****************************************************************
- * The first section of this file provides direct access to the 
+ * The first section of this file provides direct access to the
  * individual device-mapper ioctls.
  ****************************************************************/
 
@@ -925,6 +925,33 @@ struct dm_report_field_type {
 };
 
 /*
+ * An attribute is just a (field,value) pair from a dm_report.
+ * We use the list of fields in a report to build a list of
+ * attributes for an object.  The report defines the list of
+ * attributes desired.  Once the report is built with one
+ * or more calls to dm_report_object(), report output can
+ * be obtained by either dm_report_output(), which displays
+ * the report to the screen, or dm_report_output_attributes(),
+ * which returns a list of report attributes for each object
+ * that produced a row of data in the report.
+ */
+struct dm_report_attribute_type {
+	struct dm_list list;
+	const char *name;
+	unsigned is_string;
+	union {
+		char *s_val;
+		uint64_t n_val;
+	} u;
+};
+
+struct dm_report_attribute_list_type {
+	struct dm_list list;
+	void *object;
+	struct dm_list attrs; /* dm_report_attribute_type */
+};
+
+/*
  * dm_report_init output_flags
  */
 #define DM_REPORT_OUTPUT_MASK			0x000000FF
@@ -945,8 +972,15 @@ struct dm_report *dm_report_init(uint32_t *report_types,
 				 void *private);
 int dm_report_object(struct dm_report *rh, void *object);
 int dm_report_output(struct dm_report *rh);
+/*
+ * Return a list of object attributes described by the report.
+ * Each list entry described by struct dm_report_attribute_list_type
+ */
+int dm_report_output_attributes(struct dm_report *rh,
+				struct dm_list *list);
 void dm_report_free(struct dm_report *rh);
 
+
 /*
  * Prefix added to each field name with DM_REPORT_OUTPUT_FIELD_NAME_PREFIX
  */
diff --git a/libdm/libdm-report.c b/libdm/libdm-report.c
index 6784a11..723473f 100644
--- a/libdm/libdm-report.c
+++ b/libdm/libdm-report.c
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2002-2004 Sistina Software, Inc. All rights reserved.
- * Copyright (C) 2004-2007 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2004-2009 Red Hat, Inc. All rights reserved.
  *
  * This file is part of the device-mapper userspace tools.
  *
@@ -81,6 +81,7 @@ struct row {
 	struct dm_report *rh;
 	struct dm_list fields;			  /* Fields in display order */
 	struct dm_report_field *(*sort_fields)[]; /* Fields in sort order */
+	void *object; /* object that created this row of data */
 };
 
 static const struct dm_report_object_type *_find_type(struct dm_report *rh,
@@ -666,7 +667,7 @@ int dm_report_set_output_field_name_prefix(struct dm_report *rh, const char *out
 	}
 
 	rh->output_field_name_prefix = _toupperstr(prefix);
-	
+
 	return 1;
 }
 
@@ -741,6 +742,7 @@ int dm_report_object(struct dm_report *rh, void *object)
 		}
 		dm_list_add(&row->fields, &field->list);
 	}
+	row->object = object;
 
 	if (!(rh->flags & DM_REPORT_OUTPUT_BUFFERED))
 		return dm_report_output(rh);
@@ -1089,3 +1091,76 @@ int dm_report_output(struct dm_report *rh)
 	else
 		return _output_as_columns(rh);
 }
+
+/*
+ * Create a list of attributes from a row of report output.
+ */
+static struct dm_report_attribute_list_type *
+_output_row_to_attribute_list(struct dm_report *rh, struct row *row)
+
+{
+	struct dm_list *fh, *ftmp;
+	struct dm_report_field *field;
+	struct dm_report_attribute_type *attr;
+	struct dm_report_attribute_list_type *attr_list;
+	uint32_t i;
+
+	if (dm_list_empty(&row->fields))
+		return NULL;
+
+	if (!(attr_list = dm_pool_zalloc(rh->mem, sizeof(*attr_list)))) {
+			log_error("_output_row_to_attribute: "
+				  "memory allocation failed\n");
+			return NULL;
+	}
+	attr_list->object = row->object;
+	dm_list_init(&attr_list->attrs);
+	
+	dm_list_iterate_safe(fh, ftmp, &row->fields) {
+		field = dm_list_item(fh, struct dm_report_field);
+		if (field->props->flags & FLD_HIDDEN)
+			continue;
+		if (!(attr = dm_pool_zalloc(rh->mem, sizeof(*attr)))) {
+			log_error("_output_row_to_attribute: "
+				  "memory allocation failed\n");
+			return NULL;
+		}
+		i = field->props->field_num;
+		if (!(attr->name = dm_pool_strdup(rh->mem, rh->fields[i].id))) {
+			log_error("_output_row_to_attribute: "
+				  "memory allocation failed\n");
+			return NULL;
+		}
+		if ((field->props->flags & DM_REPORT_FIELD_TYPE_MASK) ==
+		    DM_REPORT_FIELD_TYPE_STRING) {
+			attr->is_string = 1;
+			attr->u.s_val = dm_pool_strdup(rh->mem,
+							      field->sort_value);
+		} else {
+			attr->is_string = 0;
+			attr->u.n_val = *(const uint64_t *)field->sort_value;
+		}
+		dm_list_add(&attr_list->attrs, &attr->list);
+	}
+	return attr_list;
+}
+
+int dm_report_output_attributes(struct dm_report *rh, struct dm_list *list)
+{
+	struct row *row = NULL;
+	struct dm_report_attribute_list_type *attr_list;
+
+	if (dm_list_empty(&rh->rows))
+		return 1;
+
+	if ((rh->flags & RH_SORT_REQUIRED))
+		_sort_rows(rh);
+
+	dm_list_iterate_items(row, &rh->rows) {
+		if (!(attr_list = _output_row_to_attribute_list(rh, row))) {
+			return 0;
+		}
+		dm_list_add(list, &attr_list->list);
+	}
+	return 1;
+}
-- 
1.6.0.6

[PATCH 02/12] Add system_dir to create_toolcontext().

[Dave Wysochanski]

Signed-off-by: Thomas Woerner <twoerner@redhat.com>
Signed-off-by: Dave Wysochanski <wysochanski@pobox.com>
Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
---
 daemons/clvmd/lvm-functions.c |    2 +-
 lib/commands/toolcontext.c    |    6 +++++-
 lib/commands/toolcontext.h    |    2 +-
 tools/lvmcmdline.c            |    2 +-
 4 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/daemons/clvmd/lvm-functions.c b/daemons/clvmd/lvm-functions.c
index 5cf7eff..3db2756 100644
--- a/daemons/clvmd/lvm-functions.c
+++ b/daemons/clvmd/lvm-functions.c
@@ -724,7 +724,7 @@ void lvm_do_backup(const char *vgname)
 /* Called to initialise the LVM context of the daemon */
 int init_lvm(int using_gulm)
 {
-	if (!(cmd = create_toolcontext(1))) {
+	if (!(cmd = create_toolcontext(1, NULL))) {
 		log_error("Failed to allocate command context");
 		return 0;
 	}
diff --git a/lib/commands/toolcontext.c b/lib/commands/toolcontext.c
index 0a98325..c481a2b 100644
--- a/lib/commands/toolcontext.c
+++ b/lib/commands/toolcontext.c
@@ -998,7 +998,8 @@ static void _init_globals(struct cmd_context *cmd)
 }
 
 /* Entry point */
-struct cmd_context *create_toolcontext(unsigned is_long_lived)
+struct cmd_context *create_toolcontext(unsigned is_long_lived,
+				       const char *system_dir)
 {
 	struct cmd_context *cmd;
 
@@ -1030,6 +1031,9 @@ struct cmd_context *create_toolcontext(unsigned is_long_lived)
 
 	strcpy(cmd->sys_dir, DEFAULT_SYS_DIR);
 
+        if (system_dir)
+                strncpy(cmd->sys_dir, system_dir, PATH_MAX);
+
 	if (!_get_env_vars(cmd))
 		goto error;
 
diff --git a/lib/commands/toolcontext.h b/lib/commands/toolcontext.h
index 7d2aef9..ba6f1ed 100644
--- a/lib/commands/toolcontext.h
+++ b/lib/commands/toolcontext.h
@@ -94,7 +94,7 @@ struct cmd_context {
 	char sysfs_dir[PATH_MAX];
 };
 
-struct cmd_context *create_toolcontext(unsigned is_long_lived);
+struct cmd_context *create_toolcontext(unsigned is_long_lived, const char *system_dir);
 void destroy_toolcontext(struct cmd_context *cmd);
 int refresh_toolcontext(struct cmd_context *cmd);
 int config_files_changed(struct cmd_context *cmd);
diff --git a/tools/lvmcmdline.c b/tools/lvmcmdline.c
index 880f31e..bafc6a5 100644
--- a/tools/lvmcmdline.c
+++ b/tools/lvmcmdline.c
@@ -1158,7 +1158,7 @@ struct cmd_context *init_lvm(void)
 
 	_cmdline.the_args = &_the_args[0];
 
-	if (!(cmd = create_toolcontext(0)))
+	if (!(cmd = create_toolcontext(0, NULL)))
 		return_NULL;
 
 	return cmd;
-- 
1.6.0.6

[PATCH 03/12] Add lvm_create, lvm_destroy, lvm_reload_config() APIs.

[Dave Wysochanski]

Add lvm2.c file.
Update test code.
Call init_locking() with locking_type = -1 to read config file value.

Signed-off-by: Thomas Woerner <twoerner@redhat.com>
Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
Signed-off-by: Dave Wysochanski <wysochanski@pobox.com>
---
 lib/Makefile.in |    3 +-
 lib/lvm2.c      |   63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/lvm2.h      |   44 +++++++++++++++++++++++--------------
 test/api/test.c |   13 +++++------
 4 files changed, 98 insertions(+), 25 deletions(-)
 create mode 100644 lib/lvm2.c

diff --git a/lib/Makefile.in b/lib/Makefile.in
index 54092cd..c0b58b9 100644
--- a/lib/Makefile.in
+++ b/lib/Makefile.in
@@ -86,7 +86,8 @@ SOURCES =\
 	report/report.c \
 	striped/striped.c \
 	uuid/uuid.c \
-	zero/zero.c
+	zero/zero.c \
+	lvm2.c
 
 ifeq ("@LVM1@", "internal")
   SOURCES +=\
diff --git a/lib/lvm2.c b/lib/lvm2.c
new file mode 100644
index 0000000..d4d3637
--- /dev/null
+++ b/lib/lvm2.c
@@ -0,0 +1,63 @@
+/*
+ * Copyright (C) 2004-2008 Red Hat, Inc. All rights reserved.
+ *
+ * This file is part of LVM2.
+ *
+ * This copyrighted material is made available to anyone wishing to use,
+ * modify, copy, or redistribute it subject to the terms and conditions
+ * of the GNU Lesser General Public License v.2.1.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include "lvm2.h"
+#include "lib.h"
+#include "toolcontext.h"
+#include "locking.h"
+#include "metadata-exported.h"
+#include "report.h"
+
+
+lvm_handle_t lvm_create(const char *system_dir)
+{
+	struct cmd_context *cmd;
+
+	/* To be done:
+	 * logging bound to handle
+	 */
+
+	/* create context */
+	cmd = create_toolcontext(1, system_dir);
+
+	/* initilize remaining */
+	if (cmd) {
+		/* initilization from lvm_run_command */
+		init_error_message_produced(0);
+		sigint_clear();
+
+		/* FIXME: locking_type config option needed? */
+		/* initialize locking */
+		if (! init_locking(-1, cmd)) {
+			printf("Locking initialisation failed.");
+			lvm_destroy((lvm_handle_t) cmd);
+			return NULL;
+		}
+	}
+
+	return (lvm_handle_t) cmd;
+}
+
+
+void lvm_destroy(lvm_handle_t libh)
+{
+	destroy_toolcontext((struct cmd_context *)libh);
+	/* no error handling here */
+}
+
+
+int lvm_reload_config(lvm_handle_t libh)
+{
+	return refresh_toolcontext((struct cmd_context *)libh);
+}
diff --git a/lib/lvm2.h b/lib/lvm2.h
index b18cead..f348035 100644
--- a/lib/lvm2.h
+++ b/lib/lvm2.h
@@ -18,36 +18,46 @@
 #include <stdint.h>
 
 /*
- * Library Initialisation
- * FIXME: For now just #define lvm2_create() and lvm2_destroy() to 
- * create_toolcontext() and destroy_toolcontext()
+ * lvm_handle_t
  */
-struct arg;
-struct cmd_context;
-struct cmd_context *create_toolcontext(unsigned is_long_lived);
-void destroy_toolcontext(struct cmd_context *cmd);
+struct lib_context; /* internal context data */
+typedef struct lib_context *lvm_handle_t;
 
+#ifdef FUTURE_FEATURE
 /*
- * lvm2_create
-lvm_handle_t lvm2_create(void);
+ * lvm_config_t
+ */
+struct lvm_config {
+	const char *system_dir;
+	unsigned is_long_lived;
+};
+#endif /* FUTURE_FEATURE */
+
+/*
+ * lvm_create
  *
- * Description: Create an LVM2 handle used in many other APIs.
+ * Description: Create an LVM handle used in many other APIs.
  *
  * Returns:
  * NULL: Fail - unable to initialise handle.
- * non-NULL: Success - valid LVM2 handle returned
+ * non-NULL: Success - valid LVM handle returned
  */
-#define lvm2_create(X) create_toolcontext(1)
+lvm_handle_t lvm_create(const char *system_dir);
 
 /*
- * lvm2_destroy
-void lvm2_destroy(lvm_handle_t h);
+ * lvm_destroy
  *
- * Description: Destroy an LVM2 handle allocated with lvm2_create
+ * Description: Destroy an LVM handle allocated with lvm_create
  *
  * Parameters:
- * - h (IN): handle obtained from lvm2_create
+ * - h (IN): handle obtained from lvm_create
+ */
+void lvm_destroy(lvm_handle_t libh);
+
+/*
+ * lvm_reload_config
+ * Reload configuration files
  */
-#define lvm2_destroy(X) destroy_toolcontext(X)
+int lvm_reload_config(lvm_handle_t libh);
 
 #endif
diff --git a/test/api/test.c b/test/api/test.c
index de53c46..9411663 100644
--- a/test/api/test.c
+++ b/test/api/test.c
@@ -48,7 +48,7 @@ static int lvm_split(char *str, int *argc, char **argv, int max)
 	return *argc;
 }
 
-static int lvmapi_test_shell(void *h)
+static int lvmapi_test_shell(lvm_handle_t libh)
 {
 	int argc, i;
 	char *input = NULL, *args[MAX_ARGS], **argv;
@@ -99,18 +99,17 @@ static int lvmapi_test_shell(void *h)
 		      
 int main (int argc, char *argv[])
 {
-	void *h;
+	lvm_handle_t libh;
 
-	h = lvm2_create();
-	if (!h) {
+	libh = lvm_create(NULL);
+	if (!libh) {
 		printf("Unable to open lvm library instance\n");
 		return 1;
 	}
 
-	lvmapi_test_shell(h);
+	lvmapi_test_shell(libh);
 
-	if (h)
-		lvm2_destroy(h);
+	lvm_destroy(libh);
 	return 0;
 }
 
-- 
1.6.0.6

[PATCH 04/12] Move vg_t, lv_t, and pv_t from metadata-exported.h into lvm2.h

[Dave Wysochanski]

Signed-off-by: Dave Wysochanski <wysochanski@pobox.com>
Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
---
 lib/lvm2.h                       |    9 +++++++++
 lib/metadata/metadata-exported.h |    8 +-------
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/lib/lvm2.h b/lib/lvm2.h
index f348035..bfa1852 100644
--- a/lib/lvm2.h
+++ b/lib/lvm2.h
@@ -60,4 +60,13 @@ void lvm_destroy(lvm_handle_t libh);
  */
 int lvm_reload_config(lvm_handle_t libh);
 
+struct volume_group;
+typedef struct volume_group vg_t;
+
+struct physical_volume;
+typedef struct physical_volume pv_t;
+
+struct logical_volume;
+typedef struct logical_volume lv_t;
+
 #endif
diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index a1c45ce..257bf91 100644
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -22,13 +22,7 @@
 #define _LVM_METADATA_EXPORTED_H
 
 #include "uuid.h"
-
-struct physical_volume;
-typedef struct physical_volume pv_t;
-struct volume_group;
-typedef struct volume_group vg_t;
-
-struct logical_volume;
+#include "lvm2.h"
 
 struct lv_segment;
 struct pv_segment;
-- 
1.6.0.6

[PATCH 05/12] Add lvm_pv_name, lvm_vg_name, and lvm_lv_name accessors.

[Dave Wysochanski]

RFC - liblvm could provide accessor functions such as these along with
the generic attribute functions.  These specific accessor functions will
make application code simpler and more readable and may be desired by
application programmers over the generic attribute functions for single
attributes.

Signed-off-by: Dave Wysochanski <wysochanski@pobox.com>
Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
---
 lib/lvm2.h              |    7 +++++++
 lib/metadata/metadata.c |   15 +++++++++++++++
 2 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/lib/lvm2.h b/lib/lvm2.h
index bfa1852..5f44614 100644
--- a/lib/lvm2.h
+++ b/lib/lvm2.h
@@ -69,4 +69,11 @@ typedef struct physical_volume pv_t;
 struct logical_volume;
 typedef struct logical_volume lv_t;
 
+/*
+ * PV, VG, and LV object attribute functions.
+ */
+const char *lvm_pv_name(const pv_t *pv);
+const char *lvm_vg_name(const vg_t *vg);
+const char *lvm_lv_name(const lv_t *lv);
+
 #endif
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index a2b5f2f..4c9f939 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -2673,6 +2673,21 @@ vg_t *vg_read_for_update(struct cmd_context *cmd, const char *vg_name,
 	return vg_read(cmd, vg_name, vgid, flags | READ_FOR_UPDATE);
 }
 
+const char *lvm_pv_name(const pv_t *pv)
+{
+	return dev_name(pv->dev);
+}
+
+const char *lvm_vg_name(const vg_t *vg)
+{
+	return vg->name;
+}
+
+const char *lvm_lv_name(const lv_t *lv)
+{
+	return lv->name;
+}
+
 /*
  * Test the validity of a VG handle returned by vg_read() or vg_read_for_update().
  *
-- 
1.6.0.6

[PATCH 06/12] Add lvm_vg_open().

[Dave Wysochanski]

RFC - lvm_vg_open().  The new vg_read() function is a step in the right
direction but still contains a lot of new options / flags that will
be new to people and we can hopefully simplify.  lvm_vg_open() was
modelled after 'open' which gives us a potential model that others are
familiar with.  Can we use the modes and flags of 'open' in a similar way
or do we need to introduce new flags/concepts to people?  For example,
can we use a some of the open flags such as O_CREAT and O_EXCL to
implement READ_CHECK_EXISTENCE?

Signed-off-by: Dave Wysochanski <wysochanski@pobox.com>
Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
---
 lib/lvm2.h              |    2 ++
 lib/metadata/metadata.c |   14 ++++++++++++++
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/lib/lvm2.h b/lib/lvm2.h
index 5f44614..20c8ce0 100644
--- a/lib/lvm2.h
+++ b/lib/lvm2.h
@@ -16,6 +16,7 @@
 #define _LIB_LVM2_H
 
 #include <stdint.h>
+#include <fcntl.h>
 
 /*
  * lvm_handle_t
@@ -76,4 +77,5 @@ const char *lvm_pv_name(const pv_t *pv);
 const char *lvm_vg_name(const vg_t *vg);
 const char *lvm_lv_name(const lv_t *lv);
 
+vg_t *lvm_vg_open(lvm_handle_t libh, const char *vg_name, mode_t mode);
 #endif
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 4c9f939..38b6e09 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -2472,6 +2472,20 @@ vg_t *vg_lock_and_read(struct cmd_context *cmd, const char *vg_name,
 }
 
 /*
+ * Open / read a VG.
+ * FIXME: Only read access allowed.
+ */
+vg_t *lvm_vg_open(lvm_handle_t libh, const char *vg_name, mode_t mode)
+{
+	if ((mode & O_ACCMODE) != O_RDONLY) {
+		log_error("Invalid access mode 0x%x for lvm_vg_read()\n",
+			mode);
+		return NULL;
+	}
+	return vg_read((struct cmd_context *)libh, vg_name, NULL, 0);
+}
+
+/*
  * Create a (vg_t) volume group handle from a struct volume_group pointer and a
  * possible failure code or zero for success.
  */
-- 
1.6.0.6

[PATCH 07/12] Add lvm_vg_close().

[Dave Wysochanski]

For now we cannot free the handle but just check to see if we should
unlock it.

Signed-off-by: Dave Wysochanski <wysochanski@pobox.com>
Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
---
 lib/lvm2.h              |    1 +
 lib/metadata/metadata.c |   11 +++++++++++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/lib/lvm2.h b/lib/lvm2.h
index 20c8ce0..85014f5 100644
--- a/lib/lvm2.h
+++ b/lib/lvm2.h
@@ -78,4 +78,5 @@ const char *lvm_vg_name(const vg_t *vg);
 const char *lvm_lv_name(const lv_t *lv);
 
 vg_t *lvm_vg_open(lvm_handle_t libh, const char *vg_name, mode_t mode);
+void lvm_vg_close(vg_t *vg);
 #endif
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 38b6e09..0fa5db4 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -2486,6 +2486,17 @@ vg_t *lvm_vg_open(lvm_handle_t libh, const char *vg_name, mode_t mode)
 }
 
 /*
+ * Close a VG handle.
+ * If lock is still held, unlock the VG.
+ * FIXME: We cannot free the memory as it's tied to the cmd->mem pool.
+ */
+void lvm_vg_close(vg_t *vg)
+{
+	if (vgname_is_locked(vg->name))
+		unlock_vg(vg->cmd, vg->name);
+}
+
+/*
  * Create a (vg_t) volume group handle from a struct volume_group pointer and a
  * possible failure code or zero for success.
  */
-- 
1.6.0.6

[PATCH 08/12] Add lvm_errno(), lvm_strerror, lvm_vg_mode.

[Dave Wysochanski]

Create struct lib_context to store lvm_errno field as well as cmd_context.
Move lvm_vg_open, lvm_vg_close to lvm2.c (b/c lib_context defined there).
Add 'mode' field to struct volume_group.
Create error APIs - lvm_errno, lvm_strerror.
Update lvm_vg_open to set lvm_errno if invalid mode given (mode != O_RDONLY).
Add lvm-types.h to lvm2.h file.

Signed-off-by: Dave Wysochanski <wysochanski@pobox.com>
Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
---
 lib/lvm2.c                       |   98 ++++++++++++++++++++++++++++++++-----
 lib/lvm2.h                       |   15 ++++++
 lib/metadata/metadata-exported.h |    1 +
 lib/metadata/metadata.c          |   25 ----------
 4 files changed, 100 insertions(+), 39 deletions(-)

diff --git a/lib/lvm2.c b/lib/lvm2.c
index d4d3637..f48b451 100644
--- a/lib/lvm2.c
+++ b/lib/lvm2.c
@@ -12,6 +12,8 @@
  * Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
+#include <errno.h>
+#include <string.h>
 #include "lvm2.h"
 #include "lib.h"
 #include "toolcontext.h"
@@ -19,10 +21,70 @@
 #include "metadata-exported.h"
 #include "report.h"
 
+struct lib_context {
+	struct cmd_context *cmd;
+	int lvm_errno;
+};
+
+mode_t lvm_vg_mode(vg_t *vg)
+{
+	return vg->mode;
+}
+
+int lvm_errno(lvm_handle_t libh)
+{
+	return libh->lvm_errno;
+}
+
+char *lvm_strerror(lvm_handle_t libh)
+{
+	return strerror(libh->lvm_errno);
+}
+
+/*
+ * Open / read a VG.
+ * FIXME: Only read access allowed.
+ */
+vg_t *lvm_vg_open(lvm_handle_t libh, const char *vg_name, mode_t mode)
+{
+	vg_t *vg;
+
+	if ((mode & O_ACCMODE) != O_RDONLY) {
+		log_error("Invalid access mode 0x%x for lvm_vg_read()\n",
+			  mode);
+		libh->lvm_errno = EPERM;
+		return NULL;
+	}
+	vg = vg_read(libh->cmd, vg_name, NULL, 0);
+
+	if (vg_read_error(vg)) {
+		/* FIXME: fill in vg->errno */
+		return NULL;
+	}
+	vg->mode = mode;
+	return vg;
+}
+
+/*
+ * Close a VG handle.
+ * If lock is still held, unlock the VG.
+ * FIXME: We cannot free the memory as it's tied to the cmd->mem pool.
+ */
+void lvm_vg_close(vg_t *vg)
+{
+	if (vgname_is_locked(vg->name))
+		unlock_vg(vg->cmd, vg->name);
+}
 
 lvm_handle_t lvm_create(const char *system_dir)
 {
 	struct cmd_context *cmd;
+	struct lib_context *libh;
+
+	if (!(libh = dm_malloc(sizeof(*libh)))) {
+		log_error("Failed to allocate library context");
+		return NULL;
+	}
 
 	/* To be done:
 	 * logging bound to handle
@@ -32,27 +94,34 @@ lvm_handle_t lvm_create(const char *system_dir)
 	cmd = create_toolcontext(1, system_dir);
 
 	/* initilize remaining */
-	if (cmd) {
-		/* initilization from lvm_run_command */
-		init_error_message_produced(0);
-		sigint_clear();
-
-		/* FIXME: locking_type config option needed? */
-		/* initialize locking */
-		if (! init_locking(-1, cmd)) {
-			printf("Locking initialisation failed.");
-			lvm_destroy((lvm_handle_t) cmd);
-			return NULL;
-		}
+	if (!cmd) {
+		dm_free(libh);
+		return NULL;
 	}
 
-	return (lvm_handle_t) cmd;
+	libh->cmd = cmd;
+
+	/* initilization from lvm_run_command */
+	init_error_message_produced(0);
+	sigint_clear();
+
+	/* FIXME: locking_type config option needed? */
+	/* initialize locking */
+	if (! init_locking(-1, cmd)) {
+		printf("Locking initialisation failed.");
+		lvm_destroy(libh);
+		dm_free(libh);
+		return NULL;
+	}
+
+	return libh;
 }
 
 
 void lvm_destroy(lvm_handle_t libh)
 {
-	destroy_toolcontext((struct cmd_context *)libh);
+	destroy_toolcontext(libh->cmd);
+	dm_free(libh);
 	/* no error handling here */
 }
 
@@ -61,3 +130,4 @@ int lvm_reload_config(lvm_handle_t libh)
 {
 	return refresh_toolcontext((struct cmd_context *)libh);
 }
+
diff --git a/lib/lvm2.h b/lib/lvm2.h
index 85014f5..d957db9 100644
--- a/lib/lvm2.h
+++ b/lib/lvm2.h
@@ -17,6 +17,7 @@
 
 #include <stdint.h>
 #include <fcntl.h>
+#include "lvm-types.h" /* needed for str_list */
 
 /*
  * lvm_handle_t
@@ -77,6 +78,20 @@ const char *lvm_pv_name(const pv_t *pv);
 const char *lvm_vg_name(const vg_t *vg);
 const char *lvm_lv_name(const lv_t *lv);
 
+/*
+ * Open volume group 'vg_name' for reading or writing.
+ *
+ * Returns:
+ * NULL - failure; use liblvm error APIs for specifics
+ * non-NULL - success; VG handle (vg_t *) returned may be used in future calls.
+ *
+ * liblvm error code explanations:
+ * EPERM: Invalid access mode given.
+ */
 vg_t *lvm_vg_open(lvm_handle_t libh, const char *vg_name, mode_t mode);
 void lvm_vg_close(vg_t *vg);
+
+int lvm_errno(lvm_handle_t libh);
+char *lvm_strerror(lvm_handle_t libh);
+mode_t lvm_vg_mode(vg_t *vg);
 #endif
diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index 257bf91..3063abb 100644
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -212,6 +212,7 @@ struct volume_group {
 	struct id id;
 	char *name;
 	char *system_id;
+	mode_t mode;
 
 	uint32_t status;
 	alloc_policy_t alloc;
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 0fa5db4..4c9f939 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -2472,31 +2472,6 @@ vg_t *vg_lock_and_read(struct cmd_context *cmd, const char *vg_name,
 }
 
 /*
- * Open / read a VG.
- * FIXME: Only read access allowed.
- */
-vg_t *lvm_vg_open(lvm_handle_t libh, const char *vg_name, mode_t mode)
-{
-	if ((mode & O_ACCMODE) != O_RDONLY) {
-		log_error("Invalid access mode 0x%x for lvm_vg_read()\n",
-			mode);
-		return NULL;
-	}
-	return vg_read((struct cmd_context *)libh, vg_name, NULL, 0);
-}
-
-/*
- * Close a VG handle.
- * If lock is still held, unlock the VG.
- * FIXME: We cannot free the memory as it's tied to the cmd->mem pool.
- */
-void lvm_vg_close(vg_t *vg)
-{
-	if (vgname_is_locked(vg->name))
-		unlock_vg(vg->cmd, vg->name);
-}
-
-/*
  * Create a (vg_t) volume group handle from a struct volume_group pointer and a
  * possible failure code or zero for success.
  */
-- 
1.6.0.6

[PATCH 09/12] Update lvm_vg_open() to read and store the list of vg attributes.

[Dave Wysochanski]

We read all vg attributes and store them on a list inside struct
volume_group.  The list remains valid and the report handle
open until vg_close is called where we call dm_report_free().

Signed-off-by: Dave Wysochanski <wysochanski@pobox.com>
Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
---
 lib/lvm2.c                       |   42 +++++++++++++++++++++++++++++++++++++-
 lib/metadata/metadata-exported.h |    6 +++++
 2 files changed, 47 insertions(+), 1 deletions(-)

diff --git a/lib/lvm2.c b/lib/lvm2.c
index f48b451..c8c65ab 100644
--- a/lib/lvm2.c
+++ b/lib/lvm2.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2004-2008 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2004-2009 Red Hat, Inc. All rights reserved.
  *
  * This file is part of LVM2.
  *
@@ -41,6 +41,36 @@ char *lvm_strerror(lvm_handle_t libh)
 	return strerror(libh->lvm_errno);
 }
 
+static int _vg_attribute_init(vg_t *vg)
+{
+	report_type_t report_type = VGS;
+
+	dm_list_init(&vg->attrs);
+
+	/*
+	 * Create a report so we can return the field ids / attribute names
+	 */
+	if (!(vg->rh = report_init(vg->cmd, "all", "", &report_type,
+				   " ", 1, 1, 1, 0, 0, 0))) {
+		stack;
+		/* FIXME: set errno */
+		return 0;
+	}
+
+	if (!report_object(vg->rh, vg, NULL, NULL, NULL, NULL)) {
+		stack;
+		return 0;
+	}
+
+	if (!dm_report_output_attributes(vg->rh, &vg->attrs)) {
+		dm_report_free(vg->rh);
+		/* FIXME: set errno */
+		return 0;
+	}
+	return 1;
+}
+
+
 /*
  * Open / read a VG.
  * FIXME: Only read access allowed.
@@ -62,6 +92,15 @@ vg_t *lvm_vg_open(lvm_handle_t libh, const char *vg_name, mode_t mode)
 		return NULL;
 	}
 	vg->mode = mode;
+
+	/*
+	 * Fill in all attributes
+	 */
+	if (!_vg_attribute_init(vg)) {
+		/* FIXME: set errno */
+		return NULL;
+	}
+
 	return vg;
 }
 
@@ -72,6 +111,7 @@ vg_t *lvm_vg_open(lvm_handle_t libh, const char *vg_name, mode_t mode)
  */
 void lvm_vg_close(vg_t *vg)
 {
+	dm_report_free(vg->rh);
 	if (vgname_is_locked(vg->name))
 		unlock_vg(vg->cmd, vg->name);
 }
diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index 3063abb..c1d771b 100644
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -261,6 +261,12 @@ struct volume_group {
 	 * 0 for success else appropriate FAILURE_* bits set.
 	 */
 	uint32_t read_status;
+
+	/*
+	 * Store all fields of a VG report here, the vg attributes.
+	 */
+	struct dm_list attrs;
+	void *rh;
 };
 
 /* There will be one area for each stripe */
-- 
1.6.0.6

[PATCH 10/12] Add lvm_vg_get_attr_list() and lvm_vg_get_attr().

[Dave Wysochanski]

For vg attributes, we use dm_report_output_attributes() to read/store
the attributes at lvm_vg_open().  This patch adds two liblvm vg
attribute functions to obtain a full list of attributes as well as
a single attribute:
1. lvm_vg_get_attr_list().  Given a vg, returns a list of attributes
2. lvm_vg_get_attr(). Given a vg and attribute name, returns a single
attribute.

The original lvm_vg_get_attr_list() code used rh->mem, then called
dm_report_free(rh) which released the memory for the attributes.
This patch relies on the change to vg_open(), which reads all vg
attributes and stores them on a list inside struct volume_group.
The list remains valid and the report handle open until vg_close
is called.

TODO: provide an access mode for the attribute to state whether the
attribute is readonly, or read-write.  This may require a specific
liblvm struct.

Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
---
 lib/lvm2.h          |    3 +++
 lib/report/report.c |   35 ++++++++++++++++++++++++++++++++++-
 2 files changed, 37 insertions(+), 1 deletions(-)

diff --git a/lib/lvm2.h b/lib/lvm2.h
index d957db9..92f60fc 100644
--- a/lib/lvm2.h
+++ b/lib/lvm2.h
@@ -78,6 +78,9 @@ const char *lvm_pv_name(const pv_t *pv);
 const char *lvm_vg_name(const vg_t *vg);
 const char *lvm_lv_name(const lv_t *lv);
 
+struct dm_list *lvm_vg_get_attr_list(vg_t *vg);
+struct dm_report_attribute_type *lvm_vg_get_attr(vg_t *vg, const char *name);
+
 /*
  * Open volume group 'vg_name' for reading or writing.
  *
diff --git a/lib/report/report.c b/lib/report/report.c
index f603eeb..01ce670 100644
--- a/lib/report/report.c
+++ b/lib/report/report.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2002-2004 Sistina Software, Inc. All rights reserved.
+ * Copyright (C) 2002-2004, 2009 Sistina Software, Inc. All rights reserved.
  * Copyright (C) 2004-2009 Red Hat, Inc. All rights reserved.
  *
  * This file is part of LVM2.
@@ -1190,3 +1190,36 @@ int report_object(void *handle, struct volume_group *vg,
 
 	return dm_report_object(handle, &obj);
 }
+
+/*
+ * Get a list of attributes for a vg
+ */
+struct dm_list *lvm_vg_get_attr_list(vg_t *vg)
+{
+	struct dm_report_attribute_list_type *attr_list;
+
+	dm_list_iterate_items(attr_list, &vg->attrs) {
+		/*
+		 * FIXME: check vg against object.  Currently
+		 * we cannot do this since report_object() allocates
+		 * lvm_report_object on the stack.
+		 */
+		return (&attr_list->attrs);
+	}
+	return NULL;
+}
+
+struct dm_report_attribute_type *lvm_vg_get_attr(vg_t *vg, const char *name)
+{
+	struct dm_report_attribute_type *attr;
+	struct dm_list *attrs;
+
+	if (!(attrs = lvm_vg_get_attr_list(vg))) {
+		return NULL;
+	}
+	dm_list_iterate_items(attr, attrs) {
+		if (!strcmp(attr->name, name))
+			return attr;
+	}
+	return NULL;
+}
-- 
1.6.0.6

[PATCH 11/12] Add lvm_vgname_from_vgid() and lvm_get_vgids()

[Dave Wysochanski]

Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
---
 lib/lvm2.c |   10 ++++++++++
 lib/lvm2.h |    7 ++++++-
 2 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/lib/lvm2.c b/lib/lvm2.c
index c8c65ab..603669e 100644
--- a/lib/lvm2.c
+++ b/lib/lvm2.c
@@ -41,6 +41,16 @@ char *lvm_strerror(lvm_handle_t libh)
 	return strerror(libh->lvm_errno);
 }
 
+struct dm_list *lvm_get_vgids(lvm_handle_t libh)
+{
+	return get_vgids(libh->cmd, 0);
+}
+
+char *lvm_vgname_from_vgid(lvm_handle_t libh, const char *vgid)
+{
+	return vgname_from_vgid(libh->cmd->mem, vgid);
+}
+
 static int _vg_attribute_init(vg_t *vg)
 {
 	report_type_t report_type = VGS;
diff --git a/lib/lvm2.h b/lib/lvm2.h
index 92f60fc..77544a7 100644
--- a/lib/lvm2.h
+++ b/lib/lvm2.h
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2001-2004 Sistina Software, Inc. All rights reserved.  
- * Copyright (C) 2004-2008 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2004-2009 Red Hat, Inc. All rights reserved.
  *
  * This file is part of LVM2.
  *
@@ -17,6 +17,7 @@
 
 #include <stdint.h>
 #include <fcntl.h>
+#include "libdevmapper.h"
 #include "lvm-types.h" /* needed for str_list */
 
 /*
@@ -97,4 +98,8 @@ void lvm_vg_close(vg_t *vg);
 int lvm_errno(lvm_handle_t libh);
 char *lvm_strerror(lvm_handle_t libh);
 mode_t lvm_vg_mode(vg_t *vg);
+
+char *lvm_vgname_from_vgid(lvm_handle_t libh, const char *vgid);
+struct dm_list *lvm_get_vgids(lvm_handle_t libh);
+
 #endif
-- 
1.6.0.6

[PATCH 12/12] Add test code.

[Dave Wysochanski]

lvm_vg_open, lvm_vg_close()
vg_get_attr_list and vg_get_attr
lvs_in_vg(), pvs_in_vg()

- don't ignore orphan vg names (e.g. #orphan_lvm2) but allow comments
- when vg_open called, store vg, lvs, and pvs inside hashes for later lookup

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

diff --git a/test/api/test.c b/test/api/test.c
index 9411663..82bd7ae 100644
--- a/test/api/test.c
+++ b/test/api/test.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2001-2004 Sistina Software, Inc. All rights reserved.  
+ * Copyright (C) 2001-2004, 2009 Sistina Software, Inc. All rights reserved.
  * Copyright (C) 2004-2008 Red Hat, Inc. All rights reserved.
  *
  * This file is part of LVM2.
@@ -29,7 +29,7 @@ static int lvm_split(char *str, int *argc, char **argv, int max)
 		while (*b && isspace(*b))
 			b++;
 
-		if ((!*b) || (*b == '#'))
+		if ((!*b) || ((*argc == 0)&&(*b == '#')))
 			break;
 
 		e = b;
@@ -48,11 +48,224 @@ static int lvm_split(char *str, int *argc, char **argv, int max)
 	return *argc;
 }
 
-static int lvmapi_test_shell(lvm_handle_t libh)
+static void _show_help(void)
 {
-	int argc, i;
+	printf("'pvs_in_vg vgname': "
+	       "Issue lvm_pvs_in_vg() API; must follow vg_open\n");
+	printf("'vgs_open': "
+	       "List the VGs that are currently open\n");
+	printf("'vgs': "
+	       "List all VGs known to the system\n");
+	printf("'vg_open vgname ['r' | 'w']': "
+	       "Issue a lvm_vg_open() API call on VG 'vgname'\n");
+	printf("'vg_close vgname': "
+	       "Issue a lvm_vg_close() API call on VG 'vgname'\n");
+	printf("'vg_get_attr_list vgname':"
+	       "Issue lvm_vg_get_attr_list() API; must follow vg_open\n");
+	printf("'vg_get_attr_value vgname [attr_name]':"
+	       "Issue lvm_vg_get_attr_value() API; must follow vg_open\n");
+	printf("'lvs_in_vg vgname': "
+	       "Issue lvm_lvs_in_vg() API; must follow vg_open\n");
+	printf("'quit': exit the program\n");
+}
+
+static struct dm_hash_table *_vgname_hash = NULL;
+static struct dm_hash_table *_pvname_hash = NULL;
+static struct dm_hash_table *_lvname_hash = NULL;
+
+static void _hash_destroy_single(struct dm_hash_table **htable)
+{
+	if (htable && *htable) {
+		dm_hash_destroy(*htable);
+		*htable = NULL;
+	}
+}
+
+static void _hash_destroy(void)
+{
+	_hash_destroy_single(&_vgname_hash);
+	_hash_destroy_single(&_pvname_hash);
+	_hash_destroy_single(&_lvname_hash);
+}
+
+static int _hash_create(void)
+{
+	if (!(_vgname_hash = dm_hash_create(128)))
+		return 0;
+	if (!(_pvname_hash = dm_hash_create(128))) {
+		_hash_destroy_single(&_vgname_hash);
+		return 0;
+	}
+	if (!(_lvname_hash = dm_hash_create(128))) {
+		_hash_destroy_single(&_vgname_hash);
+		_hash_destroy_single(&_pvname_hash);
+		return 0;
+	}
+	return 1;
+}
+
+static vg_t *_lookup_vg_by_name(char **argv, int argc)
+{
+	vg_t *vg;
+
+	if (argc < 2) {
+		printf ("Please enter vg_name\n");
+		return NULL;
+	}
+	if (!(vg = dm_hash_lookup(_vgname_hash, argv[1]))) {
+		printf ("Can't find %s in open VGs - run vg_open first\n",
+			argv[1]);
+		return NULL;
+	}
+	if (vg_read_error(vg)) {
+		printf("Error on VG - try vg_close/vg_open\n");
+		return NULL;
+	}
+	return vg;
+}
+static void _print_attr(struct dm_report_attribute_type *attr)
+{
+	if (attr->is_string)
+		printf("%s = %s\n", attr->name, attr->u.s_val);
+	else
+		printf("%s = %llu\n", attr->name, (unsigned long long)attr->u.n_val);
+}
+static void get_and_print_vg_attr(vg_t *vg, const char *name)
+{
+	struct dm_report_attribute_type *attr;
+
+	if (!(attr = lvm_vg_get_attr(vg, name))) {
+		printf("Error reading vg attribute %s value \n",
+		       name);
+		return;
+	}
+	_print_attr(attr);
+}
+static void _vg_open(char **argv, int argc, lvm_handle_t libh)
+{
+	vg_t *vg;
+	mode_t mode;
+
+	if (argc < 2) {
+		printf ("Please enter vg_name\n");
+		return;
+	}
+	if ((vg = dm_hash_lookup(_vgname_hash, argv[1]))) {
+		printf ("VG already open\n");
+		return;
+	}
+	if ((argc == 3) && (argv[2][0] == 'w')) {
+		mode = O_RDWR;
+	} else
+		mode = O_RDONLY;
+	vg = lvm_vg_open(libh, argv[1], mode);
+	if (!vg) {
+		printf("Error opening %s: %s\n", argv[1],
+		       lvm_strerror(libh));
+		return;
+	}
+
+	printf("Success opening vg %s\n", argv[1]);
+	dm_hash_insert(_vgname_hash, lvm_vg_name(vg), vg);
+
+}
+
+static void _vg_close(char **argv, int argc)
+{
+	vg_t *vg;
+
+	if (!(vg = _lookup_vg_by_name(argv, argc)))
+		return;
+	if (argc < 2) {
+		printf ("Please enter vg_name\n");
+		return;
+	}
+	if (!(vg = dm_hash_lookup(_vgname_hash, argv[1]))) {
+		printf ("Can't find %s in open VGs - run vg_open first\n",
+			argv[1]);
+		return;
+	}
+	while((vg = dm_hash_lookup(_vgname_hash, argv[1]))) {
+		dm_hash_remove(_vgname_hash, lvm_vg_name(vg));
+		lvm_vg_close(vg);
+	}
+}
+
+static void _show_one_vg(vg_t *vg)
+{
+	printf("%s: mode = %s\n", lvm_vg_name(vg),
+	       lvm_vg_mode(vg) == O_RDWR ? "READ/WRITE" :
+	       lvm_vg_mode(vg) == O_RDONLY ? "READ" :
+	       "UNKNOWN");
+}
+
+static void _list_open_vgs(void)
+{
+	dm_hash_iter(_vgname_hash, (dm_hash_iterate_fn) _show_one_vg);
+}
+
+static void _list_all_vgs(lvm_handle_t libh)
+{
+	struct dm_list *vgids;
+	struct str_list *vgid;
+	const char *vgname;
+
+	/* FIXME: properly export str_list and API function */
+	vgids = lvm_get_vgids(libh);
+	dm_list_iterate_items(vgid, vgids) {
+		vgname = lvm_vgname_from_vgid(libh, vgid->str);
+		printf("%s (%s)\n", vgname, vgid->str);
+	}
+}
+
+static void _vg_get_attr_list(char **argv, int argc)
+{
+	struct dm_report_attribute_type *attr;
+	struct dm_list *attrs;
+	vg_t *vg;
+
+	if (!(vg = _lookup_vg_by_name(argv, argc)))
+		return;
+	if (!(attrs = lvm_vg_get_attr_list(vg))) {
+		printf("Error reading vg attribute list\n");
+		return;
+	}
+	printf("VG attribute names:\n");
+	dm_list_iterate_items(attr, attrs) {
+		printf("%s\n", attr->name);
+	}
+}
+
+static void _vg_get_attr_value(char **argv, int argc)
+{
+	struct dm_report_attribute_type *attr;
+	struct dm_list *attrs;
+	int i;
+	vg_t *vg;
+
+	if (!(vg = _lookup_vg_by_name(argv, argc)))
+		return;
+	if (argc > 2) {
+		for (i=2; i<argc; i++)
+			get_and_print_vg_attr(vg, argv[i]);
+		return;
+	}
+	if (!(attrs = lvm_vg_get_attr_list(vg))) {
+		printf("Error reading vg attribute list\n");
+		return;
+	}
+	dm_list_iterate_items(attr, attrs) {
+		_print_attr(attr);
+	}
+}
+
+int lvmapi_test_shell(lvm_handle_t libh)
+{
+	int argc;
 	char *input = NULL, *args[MAX_ARGS], **argv;
 
+	_hash_create();
+	argc=0;
 	while (1) {
 		free(input);
 		input = readline("lvm> ");
@@ -86,17 +299,30 @@ static int lvmapi_test_shell(lvm_handle_t libh)
 			printf("Exiting.\n");
 			break;
 		} else if (!strcmp(argv[0], "?") || !strcmp(argv[0], "help")) {
-			printf("No commands defined\n");
-		} else if (!strcmp(argv[0], "scan")) {
-			for (i=1; i < argc; i++)
-				printf("Scan a path!\n");
+			_show_help();
+		} else if (!strcmp(argv[0], "vg_open")) {
+			_vg_open(argv, argc, libh);
+		} else if (!strcmp(argv[0], "vg_close")) {
+			_vg_close(argv, argc);
+		} else if (!strcmp(argv[0], "vgs_open")) {
+			_list_open_vgs();
+		} else if (!strcmp(argv[0], "vgs")) {
+			_list_all_vgs(libh);
+		} else if (!strcmp(argv[0], "vg_get_attr_list")) {
+			_vg_get_attr_list(argv, argc);
+		} else if (!strcmp(argv[0], "vg_get_attr_value")) {
+			_vg_get_attr_value(argv, argc);
+		} else {
+			printf ("Unrecognized command %s\n", argv[0]);
 		}
 	}
 
+	dm_hash_iter(_vgname_hash, (dm_hash_iterate_fn) lvm_vg_close);
+	_hash_destroy();
 	free(input);
 	return 0;
 }
-		      
+
 int main (int argc, char *argv[])
 {
 	lvm_handle_t libh;
-- 
1.6.0.6

[RFC PATCH 0/12] dm_report attribute handling and liblvm vg attributes

[Dave Wysochanski]

On Thu, 2009-02-12 at 14:30 -0500, Dave Wysochanski wrote:
> The purpose of this patchset is to get feedback on one approach to handling
> attributes in liblvm.  The most important patches are:
> 1/12: Adds dm_report_output_attribute() function and related structures.
> 9/12: Updates vg_open to store vg attributes in the handle.  This allows us
> to use the report memory safely (we free it in vg_close().
> 10/12: Add lvm_vg_get_attr_list() and lvm_vg_get_attr().
> 
> What is missing in the patch is the mode (read/write) of the attribute.
> I have not thought extensively about this but one option is as follows.
> Add a 'mode' or 'is_writable' field to the dm structure, and set it to
> 'readonly' for all attributes output by dm_report_output_attribute().
> Then in vg_open(), depending on the mode and capabilities of the current
> release of liblvm code, we can gradually change various attributes to
> writeable.
> 
> This patchset should build fine but with some warnings.  Attribute code
> was exercised with test code in last patch.
> 

These patches should apply cleanly on top of the following commit:
commit 9487820c32ded20b71422e79e74732273f1c4bef
Author: Christine Caulfield <ccaulfie@redhat.com>
Date:   Wed Feb 11 10:13:20 2009 +0000

    Add a fully-functional get_cluster_name() to clvmd corosync
interface.



> --
> lvm-devel mailing list
> lvm-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/lvm-devel

[PATCH 04/12] Move vg_t, lv_t, and pv_t from metadata-exported.h into lvm2.h

[Alasdair G Kergon]

Yes, update this after the file renaming and check it in.
I think the abbreviations are OK like this.

> +struct volume_group;
> +typedef struct volume_group vg_t;
> +
> +struct physical_volume;
> +typedef struct physical_volume pv_t;
> +
> +struct logical_volume;
> +typedef struct logical_volume lv_t;

Alasdair
-- 
agk at redhat.com

[PATCH 05/12] Add lvm_pv_name, lvm_vg_name, and lvm_lv_name accessors.

[Alasdair G Kergon]

On Thu, Feb 12, 2009 at 02:30:34PM -0500, Dave Wysochanski wrote:
> RFC - liblvm could provide accessor functions such as these along with
> the generic attribute functions.  These specific accessor functions will
> make application code simpler and more readable and may be desired by
> application programmers over the generic attribute functions for single
> attributes.
 
> +/*
> + * PV, VG, and LV object attribute functions.
> + */
> +const char *lvm_pv_name(const pv_t *pv);
> +const char *lvm_vg_name(const vg_t *vg);
> +const char *lvm_lv_name(const lv_t *lv);

Maybe, but let's see how the rest of the interface looks.
There are several ways this could be done, including generating
parts of the interface from templates or using the preprocessor
to offer a clean interface to underlying generic functions.

Alasdair
-- 
agk at redhat.com

[PATCH 08/12] Add lvm_errno(), lvm_strerror, lvm_vg_mode.

[Alasdair G Kergon]

On Thu, Feb 12, 2009 at 02:30:37PM -0500, Dave Wysochanski wrote:
> Create struct lib_context to store lvm_errno field as well as cmd_context.
> Create error APIs - lvm_errno, lvm_strerror.

As we discussed on the call last week, let't see first if we can get an error
number and messages integrated into the existing log_error() reporting
mechanism.

> +int lvm_errno(lvm_handle_t libh)
> +{
> +	return libh->lvm_errno;
> +}

Eventually, yes, but as we discussed last week, a static global variable
is likely to be the quickest way to get this working until libh is
available at all levels of the stack.

> +char *lvm_strerror(lvm_handle_t libh)
> +{
> +	return strerror(libh->lvm_errno);
> +}

No need for a wrapper around strerror - call it directly!

What we should do though is store the specific error messages issued
and provide a way to return them all in one go.
Again, this is likely to be a simple multi-line string for now, that gets
cleared on each new library call.

Alasdair
-- 
agk at redhat.com

[PATCH 08/12] Add lvm_errno(), lvm_strerror, lvm_vg_mode.

[Alasdair G Kergon]

On Sun, Feb 22, 2009 at 11:11:11PM +0000, Alasdair G Kergon wrote:
> As we discussed on the call last week, let't see first if we can get an error
> number and messages integrated into the existing log_error() reporting
> mechanism.
 
So take those as two separate problems.

(1) Add another output mechanism to print_log() to cache messages in memory.
Like the other mechanisms, the application should be able to configure what severity
of message it wants recording.
Needs a way to obtain the cached messages (returned as a single multi-line
string?) and a way to clear them (hooked automatically at the start of every
library function).

(2) Setting the library return code.
Have the level that wants to generate the error set it explicitly.
There'd be a default value that would be returned if it has not
been set to something else.  Only the first value would be returned - if
the function to set it is called a second time, that call would not
change the value.  This has the advantage that we only have to deal with
the places where it's actually useful to distinguish between different
errors - and I still don't think there are many such places.

Alasdair
-- 
agk at redhat.com