Fix orphan-related locking in pvdisplay and pvs.

Fix missing VG unlocks in some pvchange error paths.
Add some missing validation of VG names.
Rename validate_vg_name() to validate_new_vg_name().
Change orphan lock to VG_ORPHANS.
Change format1 to use ORPHAN as orphan VG name.

7 files changed, 21 insertions, 14 deletions

diff --git a/lib/format1/disk-rep.c b/lib/format1/disk-rep.c
index c2f880dc..b127c625 100644
--- a/lib/format1/disk-rep.c
+++ b/lib/format1/disk-rep.c
@@ -415,19 +415,19 @@ static struct disk_list *__read_disk(const struct format_type *fmt,
 struct disk_list *read_disk(const struct format_type *fmt, struct device *dev,
 			    struct dm_pool *mem, const char *vg_name)
 {
-	struct disk_list *r;
+	struct disk_list *dl;
 
 	if (!dev_open(dev)) {
 		stack;
 		return NULL;
 	}
 
-	r = __read_disk(fmt, dev, mem, vg_name);
+	dl = __read_disk(fmt, dev, mem, vg_name);
 
 	if (!dev_close(dev))
 		stack;
 
-	return r;
+	return dl;
 }
 
 static void _add_pv_to_list(struct list *head, struct disk_list *data)
diff --git a/lib/format1/import-export.c b/lib/format1/import-export.c
index 7c61d7c5..e6b79926 100644
--- a/lib/format1/import-export.c
+++ b/lib/format1/import-export.c
@@ -25,6 +25,7 @@
 #include "segtype.h"
 #include "pv_alloc.h"
 #include "display.h"
+#include "lvmcache.h"
 
 #include <time.h>
 
@@ -59,8 +60,10 @@ int import_pv(const struct format_type *fmt, struct dm_pool *mem,
 	memcpy(&pv->id, pvd->pv_uuid, ID_LEN);
 
 	pv->dev = dev;
-	if (!(pv->vg_name = dm_pool_strdup(mem, (char *)pvd->vg_name))) {
-		stack;
+	if (!*pvd->vg_name)
+		pv->vg_name = ORPHAN;
+	else if (!(pv->vg_name = dm_pool_strdup(mem, (char *)pvd->vg_name))) {
+		log_error("Volume Group name allocation failed.");
 		return 0;
 	}
 
@@ -644,7 +647,7 @@ int import_snapshots(struct dm_pool *mem __attribute((unused)), struct volume_gr
 				continue;
 
 			/* insert the snapshot */
-			if (!vg_add_snapshot(vg, NULL, org, cow, NULL,
+			if (!vg_add_snapshot(NULL, org, cow, NULL,
 					     org->le_count, 
 					     lvd->lv_chunk_size)) {
 				log_err("Couldn't add snapshot.");
diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index d45e3411..396dc3b5 100644
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -412,7 +412,7 @@ struct lv_segment *find_cow(const struct logical_volume *lv);
 /* Given a cow LV, return its origin */
 struct logical_volume *origin_from_cow(const struct logical_volume *lv);
 
-int vg_add_snapshot(struct volume_group *vg, const char *name,
+int vg_add_snapshot(const char *name,
 		    struct logical_volume *origin, struct logical_volume *cow,
 		    union lvid *lvid, uint32_t extent_count,
 		    uint32_t chunk_size);
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 7f61b711..68844d6f 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -33,7 +33,7 @@
  * FIXME: Check for valid handle before dereferencing field or log error?
  */
 #define pv_field(handle, field)				\
-	(((struct physical_volume *)(handle))->field)
+	(((const struct physical_volume *)(handle))->field)
 
 static struct physical_volume *_pv_read(struct cmd_context *cmd, 
 					const char *pv_name,
@@ -1931,6 +1931,12 @@ vg_t *vg_lock_and_read(struct cmd_context *cmd, const char *vg_name,
 	if (!(misc_flags & CORRECT_INCONSISTENT))
 		consistent = 0;
 
+	if (!validate_name(vg_name)) {
+		log_error("Volume group name %s has invalid characters",
+			  vg_name);
+		return NULL;
+	}
+
 	if (!lock_vol(cmd, vg_name, lock_flags)) {
 		log_error("Can't get lock for %s", vg_name);
 		return NULL;
diff --git a/lib/metadata/pv_manip.c b/lib/metadata/pv_manip.c
index cf2dc2c2..91df78d1 100644
--- a/lib/metadata/pv_manip.c
+++ b/lib/metadata/pv_manip.c
@@ -456,8 +456,7 @@ int pv_resize_single(struct cmd_context *cmd,
 	list_init(&mdas);
 
 	if (is_orphan_vg(pv_vg_name(pv))) {
-		vg_name = ORPHAN;
-
+		vg_name = VG_ORPHANS;
 		if (!lock_vol(cmd, vg_name, LCK_VG_WRITE)) {
 			log_error("Can't get lock for orphans");
 			return 0;
@@ -583,7 +582,7 @@ int pv_resize_single(struct cmd_context *cmd,
 		unlock_vg(cmd, vg_name);
 	} else {
 		if (!(pv_write(cmd, pv, NULL, INT64_C(-1)))) {
-			unlock_vg(cmd, ORPHAN);
+			unlock_vg(cmd, VG_ORPHANS);
 			log_error("Failed to store physical volume \"%s\"",
 				  pv_name);
 			return 0;
diff --git a/lib/metadata/snapshot_manip.c b/lib/metadata/snapshot_manip.c
index d0a443ef..9e84a2a0 100644
--- a/lib/metadata/snapshot_manip.c
+++ b/lib/metadata/snapshot_manip.c
@@ -48,8 +48,7 @@ struct logical_volume *origin_from_cow(const struct logical_volume *lv)
 	return lv->snapshot->origin;
 }
 
-int vg_add_snapshot(struct volume_group *vg, const char *name,
-		    struct logical_volume *origin,
+int vg_add_snapshot(const char *name, struct logical_volume *origin,
 		    struct logical_volume *cow, union lvid *lvid,
 		    uint32_t extent_count, uint32_t chunk_size)
 {
diff --git a/lib/snapshot/snapshot.c b/lib/snapshot/snapshot.c
index 62d2b093..0e4a532d 100644
--- a/lib/snapshot/snapshot.c
+++ b/lib/snapshot/snapshot.c
@@ -69,7 +69,7 @@ static int _snap_text_import(struct lv_segment *seg, const struct config_node *s
 		return 0;
 	}
 
-	if (!vg_add_snapshot(seg->lv->vg, seg->lv->name, org, cow,
+	if (!vg_add_snapshot(seg->lv->name, org, cow,
 			     &seg->lv->lvid, seg->len, chunk_size)) {
 		stack;
 		return 0;