o Added dev_open and dev_close functions

o  Changed disk-rep to use these

o  if NDEBUG is not defined the dev_cache will check for open devices on
   teardown.

I was hoping this would speed things up.  But I'm still getting:

reti:/home/joe/sistina/LVM2/tools# time ./lvm vgchange -a n
  Volume group vg0 successfully changed

real    0m5.751s
user    0m0.060s
sys     0m0.070s

even though I have only 1 device with the vg on it passing the filters.

lib/device/dev-cache.c

@@ -62,6 +62,7 @@ static struct device *_create_dev(dev_t d)
 
 	list_init(&dev->aliases);
 	dev->dev = d;
+	dev->fd = -1;
 	return dev;
 }
 
@@ -297,8 +298,31 @@ int dev_cache_init(void)
 	return 0;
 }
 
+static inline void _check_for_open_devices(void)
+{
+#ifndef NDEBUG
+	struct dev_iter *i = dev_iter_create(NULL);
+	struct device *dev;
+
+	if (!i) {
+		stack;
+		return;
+	}
+
+	while ((dev = dev_iter_get(i))) {
+		if (dev->fd >= 0)
+			log_err("Device '%s' has been left open.",
+				dev_name(dev));
+	}
+
+	dev_iter_destroy(i);
+#endif
+}
+
 void dev_cache_exit(void)
 {
+	_check_for_open_devices();
+
 	pool_destroy(_cache.mem);
 	if (_cache.names)
 		hash_destroy(_cache.names);

lib/device/dev-io.c

@@ -15,10 +15,6 @@
 #include <sys/ioctl.h>
 #include <sys/mount.h>
 
-/*
- * FIXME: all this opening and closing devices is
- * killing performance.
- */
 
 int dev_get_size(struct device *dev, uint64_t *size)
 {
@@ -44,10 +40,40 @@ int dev_get_size(struct device *dev, uint64_t *size)
 	return 1;
 }
 
+int dev_open(struct device *dev, int flags)
+{
+	const char *name = dev_name(dev);
+
+	if (dev->fd >= 0) {
+		log_err("Device '%s' has already been opened", name);
+		return 0;
+	}
+
+	if ((dev->fd = open(name, flags)) < 0) {
+		log_sys_error("open", "opening device");
+		return 0;
+	}
+
+	return 1;
+}
+
+int dev_close(struct device *dev)
+{
+	if (dev->fd < 0) {
+		log_err("Request to close device '%s', "
+			"which has not been opened.", dev_name(dev));
+		return 0;
+	}
+
+	close(dev->fd);
+	dev->fd = -1;
+
+	return 1;
+}
+
 /*
  *  FIXME: factor common code out.
  */
-
 int _read(int fd, void *buf, size_t count)
 {
 	size_t n = 0;
@@ -71,12 +97,11 @@ int _read(int fd, void *buf, size_t count)
 int64_t dev_read(struct device *dev, uint64_t offset,
 		 int64_t len, void *buffer)
 {
-	int64_t r;
 	const char *name = dev_name(dev);
-	int fd = open(name, O_RDONLY);
+	int fd = dev->fd;
 
 	if (fd < 0) {
-		log_sys_very_verbose("open", name);
+		log_err("Attempt to read an unopened device (%s).", name);
 		return 0;
 	}
 
@@ -85,9 +110,7 @@ int64_t dev_read(struct device *dev, uint64_t offset,
 		return 0;
 	}
 
-	r = _read(fd, buffer, len);
+	return _read(fd, buffer, len);
-	close(fd);
-	return r;
 }
 
 int _write(int fd, const void *buf, size_t count)
@@ -113,12 +136,11 @@ int _write(int fd, const void *buf, size_t count)
 int64_t dev_write(struct device *dev, uint64_t offset,
 		  int64_t len, void *buffer)
 {
-	int64_t r;
 	const char *name = dev_name(dev);
-	int fd = open(name, O_WRONLY);
+	int fd = dev->fd;
 
 	if (fd < 0) {
-		log_sys_error("open", name);
+		log_err("Attempt to write to an unopened device (%s).", name);
 		return 0;
 	}
 
@@ -127,9 +149,7 @@ int64_t dev_write(struct device *dev, uint64_t offset,
 		return 0;
 	}
 
-	r = _write(fd, buffer, len);
+	return _write(fd, buffer, len);
-	close(fd);
-	return r;
 }
 
 int dev_zero(struct device *dev, uint64_t offset, int64_t len)
@@ -137,10 +157,10 @@ int dev_zero(struct device *dev, uint64_t offset, int64_t len)
 	int64_t r, s;
 	char buffer[4096];
 	const char *name = dev_name(dev);
-	int fd = open(name, O_WRONLY);
+	int fd = dev->fd;
 
 	if (fd < 0) {
-		log_sys_error("open", name);
+		log_err("Attempt to zero an unopened device (%s).", name);
 		return 0;
 	}
 
@@ -164,6 +184,5 @@ int dev_zero(struct device *dev, uint64_t offset, int64_t len)
 		}
 	}
 
-	close(fd);
+	return (len == 0);
-	return r;
 }

lib/device/device.h

@@ -17,20 +17,26 @@
 struct device {
 	struct list aliases; /* struct str_list from lvm-types.h */
 	dev_t dev;
+
+	/* private */
+	int fd;
 };
 
 /*
- * All io should use these routines, rather than opening the devices
+ * All io should use these routines.
- * by hand.  You do not have to call an open routine.  ATM all io is
- * immediately flushed.
  */
 int dev_get_size(struct device *dev, uint64_t *size);
+
+int dev_open(struct device *dev, int flags);
+int dev_close(struct device *dev);
+
 int64_t dev_read(struct device *dev,
 		 uint64_t offset, int64_t len, void *buffer);
 int64_t dev_write(struct device *dev,
 		  uint64_t offset, int64_t len, void *buffer);
 int dev_zero(struct device *dev, uint64_t offset, int64_t len);
 
+
 static inline const char *dev_name(struct device *dev) {
 	return list_item(dev->aliases.n, struct str_list)->str;
 }

lib/format1/disk-rep.c

@@ -9,6 +9,10 @@
 #include "xlate.h"
 #include "log.h"
 
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
 #define fail do {stack; return 0;} while(0)
 #define xx16(v) disk->v = xlate16(disk->v)
 #define xx32(v) disk->v = xlate32(disk->v)
@@ -229,8 +233,8 @@ static int _read_extents(struct disk_list *data)
 	return 1;
 }
 
-struct disk_list *read_disk(struct device *dev, struct pool *mem,
+static struct disk_list *__read_disk(struct device *dev, struct pool *mem,
-			  const char *vg_name)
+				     const char *vg_name)
 {
 	struct disk_list *data = pool_alloc(mem, sizeof(*data));
 	const char *name = dev_name(dev);
@@ -305,6 +309,25 @@ struct disk_list *read_disk(struct device *dev, struct pool *mem,
 	return NULL;
 }
 
+struct disk_list *read_disk(struct device *dev, struct pool *mem,
+			    const char *vg_name)
+{
+	struct disk_list *r;
+
+	if (!dev_open(dev, O_RDONLY)) {
+		stack;
+		return NULL;
+	}
+
+	r = __read_disk(dev, mem, vg_name);
+
+	if (!dev_close(dev))
+		stack;
+
+	return r;
+}
+
+
 /*
  * Build a list of pv_d's structures, allocated
  * from mem.  We keep track of the first object
@@ -433,7 +456,10 @@ static int _write_pvd(struct disk_list *data)
 	return 1;
 }
 
-static int _write_all_pvd(struct disk_list *data)
+/*
+ * assumes the device has been opened.
+ */
+static int __write_all_pvd(struct disk_list *data)
 {
 	const char *pv_name = dev_name(data->dev);
 
@@ -471,12 +497,33 @@ static int _write_all_pvd(struct disk_list *data)
 	return 1;
 }
 
+/*
+ * opens the device and hands to the above fn.
+ */
+static int _write_all_pvd(struct disk_list *data)
+{
+	int r;
+
+	if (!dev_open(data->dev, O_WRONLY)) {
+		stack;
+		return 0;
+	}
+
+	r = __write_all_pvd(data);
+
+	if (!dev_close(data->dev))
+		stack;
+
+	return r;
+}
+
+
 /*
  * Writes all the given pv's to disk.  Does very
  * little sanity checking, so make sure correct
  * data is passed to here.
  */
-int write_pvds(struct list *pvs)
+int write_disks(struct list *pvs)
 {
 	struct list *pvh;
 	struct disk_list *dl;

lib/format1/disk-rep.h

@@ -179,12 +179,12 @@ int calculate_extent_count(struct physical_volume *pv);
  * disk_lists.
  */
 struct disk_list *read_disk(struct device *dev, struct pool *mem,
-			   const char *vg_name);
+			    const char *vg_name);
 
 int read_pvs_in_vg(const char *vg_name, struct dev_filter *filter,
 		   struct pool *mem, struct list *results);
 
-int write_pvds(struct list *pvs);
+int write_disks(struct list *pvs);
 
 
 /*

lib/format1/format1.c

@@ -194,7 +194,7 @@ static int _vg_write(struct format_instance *fi, struct volume_group *vg)
 	list_init(&pvs);
 
 	r = (_flatten_vg(mem, vg, &pvs, fi->cmd->dev_dir, fi->cmd->filter) &&
-	     write_pvds(&pvs));
+	     write_disks(&pvs));
 	pool_destroy(mem);
 	return r;
 }
@@ -392,7 +392,7 @@ static int _pv_write(struct format_instance *fi, struct physical_volume *pv)
 	}
 
 	list_add(&pvs, &dl->list);
-	if (!write_pvds(&pvs)) {
+	if (!write_disks(&pvs)) {
 		stack;
 		goto bad;
 	}