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.

diff --git a/lib/device/dev-cache.c b/lib/device/dev-cache.c
index dc68667a3750ca0ecff0e9a886ba6a1b77ba2770..f4bce7f780a966279cdd5c20a7051a75acb44149 100644 (file)
--- a/lib/device/dev-cache.c
+++ b/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);

diff --git a/lib/device/dev-io.c b/lib/device/dev-io.c
index 1ece40263098ebad6db50ba378018453d3886f46..552cb5e0ba193d000d494405f0e4fa8f5e88e124 100644 (file)
--- a/lib/device/dev-io.c
+++ b/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);
-       close(fd);
-       return r;
+       return _read(fd, buffer, len);
 }
 
 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);
-       close(fd);
-       return r;
+       return _write(fd, buffer, len);
 }
 
 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 r;
+       return (len == 0);
 }

diff --git a/lib/device/device.h b/lib/device/device.h
index cdea303183c42d4e29121b038ab29d2919912ee0..263a55d747d129840dad5e9af68fb2a40d1729ea 100644 (file)
--- a/lib/device/device.h
+++ b/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
- * by hand.  You do not have to call an open routine.  ATM all io is
- * immediately flushed.
+ * All io should use these routines.
  */
 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;
 }

diff --git a/lib/format1/disk-rep.c b/lib/format1/disk-rep.c
index f06ece0bec04a77f3176265d51cb68118241f2fe..df61448d1c8cd0b804d15c6ec7efc95233b50ed0 100644 (file)
--- a/lib/format1/disk-rep.c
+++ b/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,
-                         const char *vg_name)
+static struct disk_list *__read_disk(struct device *dev, struct pool *mem,
+                                    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;

diff --git a/lib/format1/disk-rep.h b/lib/format1/disk-rep.h
index 20b1678ed1152123d8da0177cb06c97ebf17e9e4..051f25e237914edf479ff4b267d72a0bf09467a5 100644 (file)
--- a/lib/format1/disk-rep.h
+++ b/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);
 
 
 /*

diff --git a/lib/format1/format1.c b/lib/format1/format1.c
index 2eef659ff32383bd1df78134c4f91d579f34a96e..fdfe2da7dc28a3d97f2283e28c4ed44368b30e82 100644 (file)
--- a/lib/format1/format1.c
+++ b/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;
        }