[PATCH 0/8] Fixes and speedups

[PATCH 0/8] Fixes and speedups

[Zdenek Kabelac]

Patchset combines few fixes from the last review.
Hopefully they should be now in the 'acceptable' state.

And also fix for ocfdir and its selection is added.

I hope it'll hit 2.02.80 version.

Zdenek Kabelac (8):
  Add support for OCF dir selection
  Add default error path for get_property
  Change import_vg_from_buffer to use config_tree
  Cache config_tree
  Define DM_COOKIE_AUTO_CREATE
  Add internal fs cookie
  Stop calling fs_unlock() and dm_udev_wait()
  Add fs_unlock calls

 configure.in                  |    5 +++++
 daemons/clvmd/clvmd-command.c |    1 +
 daemons/clvmd/lvm-functions.c |    8 ++++++++
 daemons/clvmd/lvm-functions.h |    1 +
 include/.symlinks.in          |    1 +
 lib/activate/activate.c       |    2 --
 lib/activate/dev_manager.c    |   25 ++++++++-----------------
 lib/activate/fs.c             |   22 +++++++++++++++++++++-
 lib/activate/fs.h             |    2 ++
 lib/cache/lvmcache.c          |   19 +++++++++++++++++--
 lib/cache/lvmcache.h          |    3 +++
 lib/format_text/import.c      |    9 ++-------
 lib/locking/file_locking.c    |    2 ++
 lib/metadata/lv_manip.c       |    3 +++
 lib/metadata/metadata.h       |    3 +++
 libdm/libdevmapper.h          |    5 +++++
 liblvm/lvm_misc.c             |   26 +++++++++++---------------
 make.tmpl.in                  |    2 +-
 18 files changed, 94 insertions(+), 45 deletions(-)

-- 
1.7.3.4

[PATCH 1/8] Add support for OCF dir selection

[Zdenek Kabelac]

Add --with-ocfdir supported configure option.
Fixes also superfluous '/usr' in 'ocf_scriptdir'.

Note: After commit regeneration of configure file is needed.

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 configure.in |    5 +++++
 make.tmpl.in |    2 +-
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/configure.in b/configure.in
index 6cb5e8d..b58a6ba 100644
--- a/configure.in
+++ b/configure.in
@@ -368,6 +368,10 @@ AC_ARG_ENABLE(ocf,
 			     [enable Open Cluster Framework (OCF) compliant resource agents]),
 	      OCF=$enableval, OCF=no)
 AC_MSG_RESULT($OCF)
+AC_ARG_WITH(ocfdir,
+	    AC_HELP_STRING([--with-ocfdir=DIR],
+			   [install OCF files in DIR [[PREFIX/lib/ocf/resource.d/lvm2]]]),
+	    OCFDIR=$withval, OCFDIR='${prefix}/lib/ocf/resource.d/lvm2')
 
 ################################################################################
 dnl -- Init pkg-config with dummy invokation:
@@ -1328,6 +1332,7 @@ AC_SUBST(LVM_RELEASE)
 AC_SUBST(LVM_RELEASE_DATE)
 AC_SUBST(MIRRORS)
 AC_SUBST(OCF)
+AC_SUBST(OCFDIR)
 AC_SUBST(REPLICATORS)
 AC_SUBST(MSGFMT)
 AC_SUBST(PKGCONFIG)
diff --git a/make.tmpl.in b/make.tmpl.in
index 806f77a..48fd617 100644
--- a/make.tmpl.in
+++ b/make.tmpl.in
@@ -65,7 +65,7 @@ staticdir = $(DESTDIR)@STATICDIR@
 udevdir = $(DESTDIR)@udevdir@
 pkgconfigdir = $(usrlibdir)/pkgconfig
 initdir = $(DESTDIR)@sysconfdir@/rc.d/init.d
-ocf_scriptdir = $(DESTDIR)@prefix@/usr/lib/ocf/resource.d/lvm2
+ocf_scriptdir = $(DESTDIR)@OCFDIR@
 
 USRLIB_RELPATH = $(shell echo $(abspath $(usrlibdir) $(libdir)) | \
   $(AWK) -f $(top_srcdir)/scripts/relpath.awk)
-- 
1.7.3.4

[PATCH 1/8] Add support for OCF dir selection

[Alasdair G Kergon]

On Fri, Jan 07, 2011 at 12:17:29PM +0100, Zdenek Kabelac wrote:
> Add --with-ocfdir supported configure option.

Ack.

> Note: After commit regeneration of configure file is needed.
 
If you have F14 installed, use that now.

Alasdair

[PATCH 2/8] Add default error path for get_property

[Zdenek Kabelac]

Set invalid property value for error path when none of handlers is set.
Fixes use of uninitialized prop variable as we return 'v' by value.

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 liblvm/lvm_misc.c |   26 +++++++++++---------------
 1 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/liblvm/lvm_misc.c b/liblvm/lvm_misc.c
index 62fef61..e5464fe 100644
--- a/liblvm/lvm_misc.c
+++ b/liblvm/lvm_misc.c
@@ -52,33 +52,29 @@ struct lvm_property_value get_property(const pv_t pv, const vg_t vg,
 	struct lvm_property_type prop;
 	struct lvm_property_value v;
 
+	memset(&v, 0, sizeof(v));
 	prop.id = name;
+
 	if (pv) {
-		if (!pv_get_property(pv, &prop)) {
-			v.is_valid = 0;
+		if (!pv_get_property(pv, &prop))
 			return v;
-		}
 	} else if (vg) {
-		if (!vg_get_property(vg, &prop)) {
-			v.is_valid = 0;
+		if (!vg_get_property(vg, &prop))
 			return v;
-		}
 	} else if (lv) {
-		if (!lv_get_property(lv, &prop)) {
-			v.is_valid = 0;
+		if (!lv_get_property(lv, &prop))
 			return v;
-		}
 	} else if (lvseg) {
-		if (!lvseg_get_property(lvseg, &prop)) {
-			v.is_valid = 0;
+		if (!lvseg_get_property(lvseg, &prop))
 			return v;
-		}
 	} else if (pvseg) {
-		if (!pvseg_get_property(pvseg, &prop)) {
-			v.is_valid = 0;
+		if (!pvseg_get_property(pvseg, &prop))
+			return v;
+	} else {
+			log_errno(EINVAL, "Invalid query for property.");
 			return v;
-		}
 	}
+
 	v.is_settable = prop.is_settable;
 	v.is_string = prop.is_string;
 	v.is_integer = prop.is_integer;
-- 
1.7.3.4

[PATCH 2/8] Add default error path for get_property

[Alasdair G Kergon]

On Fri, Jan 07, 2011 at 12:17:30PM +0100, Zdenek Kabelac wrote:
> +			log_errno(EINVAL, "Invalid query for property.");

Do we have any information about the property to include in that error
message, such as its name?

Ack.

Alasdair

[PATCH 2/8] Add default error path for get_property

[Zdenek Kabelac]

Dne 7.1.2011 14:41, Alasdair G Kergon napsal(a):
> On Fri, Jan 07, 2011 at 12:17:30PM +0100, Zdenek Kabelac wrote:
>> +			log_errno(EINVAL, "Invalid query for property.");
> 
> Do we have any information about the property to include in that error
> message, such as its name?
> 

No - all parameters passed to get_property are NULL in this case -
so programmer's error.

Zdenek

[PATCH 2/8] Add default error path for get_property

[Alasdair G Kergon]

On Fri, Jan 07, 2011 at 02:49:16PM +0100, Zdenek Kabelac wrote:
> No - all parameters passed to get_property are NULL in this case -
> so programmer's error.
 
So in what sense is it invalid?
(I'm basically asking if a more-useful error message is possible.
So you could say in it the function was called without any parameters set?)

Alasdair

[PATCH 2/8] Add default error path for get_property

[Zdenek Kabelac]

Dne 7.1.2011 14:55, Alasdair G Kergon napsal(a):
> On Fri, Jan 07, 2011 at 02:49:16PM +0100, Zdenek Kabelac wrote:
>> No - all parameters passed to get_property are NULL in this case -
>> so programmer's error.
>  
> So in what sense is it invalid?
> (I'm basically asking if a more-useful error message is possible.
> So you could say in it the function was called without any parameters set?)
> 

Examle of such error use-case could be:

lv = NULL;
lvm_lv_get_property(lv, "name");

i.e. user will use invalid lv handle for some reason - code mistake, missing
check...

Maybe "Invalid library use." ?

Zdenek

[PATCH 2/8] Add default error path for get_property

[Dave Wysochanski]

On Fri, 2011-01-07 at 16:03 +0100, Zdenek Kabelac wrote:
> Dne 7.1.2011 14:55, Alasdair G Kergon napsal(a):
> > On Fri, Jan 07, 2011 at 02:49:16PM +0100, Zdenek Kabelac wrote:
> >> No - all parameters passed to get_property are NULL in this case -
> >> so programmer's error.
> >  
> > So in what sense is it invalid?
> > (I'm basically asking if a more-useful error message is possible.
> > So you could say in it the function was called without any parameters set?)
> > 
> 
> Examle of such error use-case could be:
> 
> lv = NULL;
> lvm_lv_get_property(lv, "name");
> 
> i.e. user will use invalid lv handle for some reason - code mistake, missing
> check...
> 
> Maybe "Invalid library use." ?
> 
> Zdenek
> 

Cleanup in this patch looks great.  Ack.

However, some handle validation should be done at the beginning of every
library call (problem not unique to this function), not just in this
function.

[PATCH 3/8] Change import_vg_from_buffer to use config_tree

[Zdenek Kabelac]

Change function import_vg_from_buffer() to import_vg_from_config_tree().
Instead of creating config tree inside the function allow cft to
be passed as parameter - usable later for caching.

Patch only moves the code and presents no functional change.

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 lib/cache/lvmcache.c     |   13 +++++++++++--
 lib/cache/lvmcache.h     |    2 ++
 lib/format_text/import.c |    9 ++-------
 lib/metadata/metadata.h  |    3 +++
 4 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c
index d545563..ebd6d44 100644
--- a/lib/cache/lvmcache.c
+++ b/lib/cache/lvmcache.c
@@ -26,6 +26,7 @@
 #include "format-text.h"
 #include "format_pool.h"
 #include "format1.h"
+#include "config.h"
 
 static struct dm_hash_table *_pvid_hash = NULL;
 static struct dm_hash_table *_vgid_hash = NULL;
@@ -650,12 +651,20 @@ struct volume_group *lvmcache_get_vg(const char *vgid, unsigned precommitted)
 						      vgid, NULL)))
 		return_NULL;
 
-	if (!(vg = import_vg_from_buffer(vginfo->vgmetadata, fid))) {
+	if (!(vginfo->cft =
+	      create_config_tree_from_string(fid->fmt->cmd,
+					     vginfo->vgmetadata))) {
 		_free_cached_vgmetadata(vginfo);
-		free_vg(vg);
 		return_NULL;
 	}
 
+	if (!(vg = import_vg_from_config_tree(vginfo->cft, fid))) {
+		_free_cached_vgmetadata(vginfo);
+		destroy_config_tree(vginfo->cft);
+		return_NULL;
+	}
+	destroy_config_tree(vginfo->cft);
+
 	log_debug("Using cached %smetadata for VG %s.",
 		  vginfo->precommitted ? "pre-committed" : "", vginfo->vgname);
 
diff --git a/lib/cache/lvmcache.h b/lib/cache/lvmcache.h
index 06838dc..8c07a26 100644
--- a/lib/cache/lvmcache.h
+++ b/lib/cache/lvmcache.h
@@ -33,6 +33,7 @@
 struct cmd_context;
 struct format_type;
 struct volume_group;
+struct config_tree;
 
 /* One per VG */
 struct lvmcache_vginfo {
@@ -46,6 +47,7 @@ struct lvmcache_vginfo {
 	struct lvmcache_vginfo *next; /* Another VG with same name? */
 	char *creation_host;
 	char *vgmetadata;	/* Copy of VG metadata as format_text string */
+	struct config_tree *cft; /* Config tree created from vgmetadata */
 	unsigned precommitted;	/* Is vgmetadata live or precommitted? */
 };
 
diff --git a/lib/format_text/import.c b/lib/format_text/import.c
index 60f465f..aa8c807 100644
--- a/lib/format_text/import.c
+++ b/lib/format_text/import.c
@@ -131,18 +131,14 @@ struct volume_group *text_vg_import_file(struct format_instance *fid,
 				 when, desc);
 }
 
-struct volume_group *import_vg_from_buffer(const char *buf,
-                                           struct format_instance *fid)
+struct volume_group *import_vg_from_config_tree(const struct config_tree *cft,
+						struct format_instance *fid)
 {
 	struct volume_group *vg = NULL;
-	struct config_tree *cft;
 	struct text_vg_version_ops **vsn;
 
 	_init_text_import();
 
-	if (!(cft = create_config_tree_from_string(fid->fmt->cmd, buf)))
-		return_NULL;
-
 	for (vsn = &_text_vsn_list[0]; *vsn; vsn++) {
 		if (!(*vsn)->check_version(cft))
 			continue;
@@ -155,6 +151,5 @@ struct volume_group *import_vg_from_buffer(const char *buf,
 		break;
 	}
 
-	destroy_config_tree(cft);
 	return vg;
 }
diff --git a/lib/metadata/metadata.h b/lib/metadata/metadata.h
index 08bff4a..c0f9148 100644
--- a/lib/metadata/metadata.h
+++ b/lib/metadata/metadata.h
@@ -100,6 +100,7 @@
 //#define FMT_RESIZE_PV		0x00000080U	/* Supports pvresize? */
 //#define FMT_UNLIMITED_STRIPESIZE 0x00000100U	/* Unlimited stripe size? */
 
+struct config_tree;
 struct metadata_area;
 
 /* Per-format per-metadata area operations */
@@ -388,6 +389,8 @@ void lv_calculate_readahead(const struct logical_volume *lv, uint32_t *read_ahea
 int export_vg_to_buffer(struct volume_group *vg, char **buf);
 struct volume_group *import_vg_from_buffer(const char *buf,
 					   struct format_instance *fid);
+struct volume_group *import_vg_from_config_tree(const struct config_tree *cft,
+						struct format_instance *fid);
 
 /*
  * Mirroring functions
-- 
1.7.3.4

[PATCH 3/8] Change import_vg_from_buffer to use config_tree

[Alasdair G Kergon]

On Fri, Jan 07, 2011 at 12:17:31PM +0100, Zdenek Kabelac wrote:
> Change function import_vg_from_buffer() to import_vg_from_config_tree().
> Instead of creating config tree inside the function allow cft to
> be passed as parameter - usable later for caching.
 
Ack.

Alasdair

[PATCH 4/8] Cache config_tree

[Zdenek Kabelac]

Start to use config_tree for cache just like vgmetedata.
When vgmetadata are erased - destroy its config tree.

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 lib/cache/lvmcache.c |   12 +++++++++---
 lib/cache/lvmcache.h |    1 +
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c
index ebd6d44..0e9cae1 100644
--- a/lib/cache/lvmcache.c
+++ b/lib/cache/lvmcache.c
@@ -83,6 +83,12 @@ static void _free_cached_vgmetadata(struct lvmcache_vginfo *vginfo)
 
 	vginfo->vgmetadata = NULL;
 
+	/* Release also cached config tree */
+	if (vginfo->cft) {
+		destroy_config_tree(vginfo->cft);
+		vginfo->cft = NULL;
+	}
+
 	log_debug("Metadata cache: VG %s wiped.", vginfo->vgname);
 }
 
@@ -651,7 +657,9 @@ struct volume_group *lvmcache_get_vg(const char *vgid, unsigned precommitted)
 						      vgid, NULL)))
 		return_NULL;
 
-	if (!(vginfo->cft =
+	/* Build config tree from vgmetadata, if not yet cached */
+	if (!vginfo->cft &&
+	    !(vginfo->cft =
 	      create_config_tree_from_string(fid->fmt->cmd,
 					     vginfo->vgmetadata))) {
 		_free_cached_vgmetadata(vginfo);
@@ -660,10 +668,8 @@ struct volume_group *lvmcache_get_vg(const char *vgid, unsigned precommitted)
 
 	if (!(vg = import_vg_from_config_tree(vginfo->cft, fid))) {
 		_free_cached_vgmetadata(vginfo);
-		destroy_config_tree(vginfo->cft);
 		return_NULL;
 	}
-	destroy_config_tree(vginfo->cft);
 
 	log_debug("Using cached %smetadata for VG %s.",
 		  vginfo->precommitted ? "pre-committed" : "", vginfo->vgname);
diff --git a/lib/cache/lvmcache.h b/lib/cache/lvmcache.h
index 8c07a26..080f3b5 100644
--- a/lib/cache/lvmcache.h
+++ b/lib/cache/lvmcache.h
@@ -48,6 +48,7 @@ struct lvmcache_vginfo {
 	char *creation_host;
 	char *vgmetadata;	/* Copy of VG metadata as format_text string */
 	struct config_tree *cft; /* Config tree created from vgmetadata */
+				/* Lifetime is directly tied to vgmetadata */
 	unsigned precommitted;	/* Is vgmetadata live or precommitted? */
 };
 
-- 
1.7.3.4

[PATCH 4/8] Cache config_tree

[Alasdair G Kergon]

On Fri, Jan 07, 2011 at 12:17:32PM +0100, Zdenek Kabelac wrote:
> Start to use config_tree for cache just like vgmetedata.
> When vgmetadata are erased - destroy its config tree.
 
Ack.

Alasdair

[PATCH 5/8] Define DM_COOKIE_AUTO_CREATE

[Zdenek Kabelac]

TODO: Use it also for already written code.

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 libdm/libdevmapper.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/libdm/libdevmapper.h b/libdm/libdevmapper.h
index 994a364..eea1a6c 100644
--- a/libdm/libdevmapper.h
+++ b/libdm/libdevmapper.h
@@ -1214,6 +1214,11 @@ void dm_udev_set_sync_support(int sync_with_udev);
 int dm_udev_get_sync_support(void);
 void dm_udev_set_checking(int checking);
 int dm_udev_get_checking(void);
+
+/*
+ * Default value to get new auto generated cookie created
+ */
+#define DM_COOKIE_AUTO_CREATE 0
 int dm_udev_create_cookie(uint32_t *cookie);
 int dm_udev_complete(uint32_t cookie);
 int dm_udev_wait(uint32_t cookie);
-- 
1.7.3.4

[PATCH 5/8] Define DM_COOKIE_AUTO_CREATE

[Alasdair G Kergon]

On Fri, Jan 07, 2011 at 12:17:33PM +0100, Zdenek Kabelac wrote:
> +#define DM_COOKIE_AUTO_CREATE 0

Ack.
(If anyone has a better idea for a name, we've got till Monday before this
one becomes part of the API.)

Alasdair

[PATCH 6/8] Add internal fs cookie

[Zdenek Kabelac]

Add functions for handling internal lvm cookie used for
all dm_tree operations until fs_unlock is called.

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 lib/activate/fs.c |   22 +++++++++++++++++++++-
 lib/activate/fs.h |    2 ++
 2 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/lib/activate/fs.c b/lib/activate/fs.c
index 1523115..cbe823a 100644
--- a/lib/activate/fs.c
+++ b/lib/activate/fs.c
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2001-2004 Sistina Software, Inc. All rights reserved.
- * Copyright (C) 2004-2007 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2004-2011 Red Hat, Inc. All rights reserved.
  *
  * This file is part of LVM2.
  *
@@ -26,6 +26,12 @@
 #include <limits.h>
 #include <dirent.h>
 
+/*
+ * Library cookie to combine multiple fs transactions.
+ * Supports to wait for udev device settle only when needed.
+ */
+static uint32_t _fs_cookie = DM_COOKIE_AUTO_CREATE;
+
 static int _mk_dir(const char *dev_dir, const char *vg_name)
 {
 	char vg_path[PATH_MAX];
@@ -396,7 +402,21 @@ int fs_rename_lv(struct logical_volume *lv, const char *dev,
 void fs_unlock(void)
 {
 	if (!memlock()) {
+		/* Wait for all processed udev devices */
+		if (!dm_udev_wait(_fs_cookie))
+			stack;
+		_fs_cookie = DM_COOKIE_AUTO_CREATE; /* Reset cookie */
 		dm_lib_release();
 		_pop_fs_ops();
 	}
 }
+
+uint32_t fs_get_cookie(void)
+{
+	return _fs_cookie;
+}
+
+void fs_set_cookie(uint32_t cookie)
+{
+	_fs_cookie = cookie;
+}
diff --git a/lib/activate/fs.h b/lib/activate/fs.h
index 28b2c73..695c529 100644
--- a/lib/activate/fs.h
+++ b/lib/activate/fs.h
@@ -30,5 +30,7 @@ int fs_del_lv_byname(const char *dev_dir, const char *vg_name,
 int fs_rename_lv(struct logical_volume *lv, const char *dev, 
 		 const char *old_vgname, const char *old_lvname);
 void fs_unlock(void);
+uint32_t fs_get_cookie(void);
+void fs_set_cookie(uint32_t cookie);
 
 #endif
-- 
1.7.3.4

[PATCH 6/8] Add internal fs cookie

[Alasdair G Kergon]

On Fri, Jan 07, 2011 at 12:17:34PM +0100, Zdenek Kabelac wrote:
> Add functions for handling internal lvm cookie used for
> all dm_tree operations until fs_unlock is called.
 
Ack.

> +		if (!dm_udev_wait(_fs_cookie))

(This function returns succesfully immediately if _fs_cookie still
holds DM_COOKIE_AUTO_CREATE.)

Alasdair

[PATCH 7/8] Stop calling fs_unlock() and dm_udev_wait()

[Zdenek Kabelac]

Stop calling fs_unlock() from lv_de/activate()

Start using internal lvm fs cookie for dm_tree.

Stop directly calling dm_udev_wait() and
dm_tree_set/get_cookie() from activate code -
it's now called through fs_unlock() function.

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 lib/activate/activate.c    |    2 --
 lib/activate/dev_manager.c |   25 ++++++++-----------------
 2 files changed, 8 insertions(+), 19 deletions(-)

diff --git a/lib/activate/activate.c b/lib/activate/activate.c
index 174f09e..e199fbb 100644
--- a/lib/activate/activate.c
+++ b/lib/activate/activate.c
@@ -1206,7 +1206,6 @@ int lv_deactivate(struct cmd_context *cmd, const char *lvid_s)
 	memlock_inc(cmd);
 	r = _lv_deactivate(lv);
 	memlock_dec(cmd);
-	fs_unlock();
 
 	if (!lv_info(cmd, lv, 0, &info, 1, 0) || info.exists)
 		r = 0;
@@ -1305,7 +1304,6 @@ static int _lv_activate(struct cmd_context *cmd, const char *lvid_s,
 	if (!(r = _lv_activate_lv(lv, 0)))
 		stack;
 	memlock_dec(cmd);
-	fs_unlock();
 
 	if (r && !monitor_dev_for_events(cmd, lv, 0, 1))
 		stack;
diff --git a/lib/activate/dev_manager.c b/lib/activate/dev_manager.c
index 6d7d905..1fbefc4 100644
--- a/lib/activate/dev_manager.c
+++ b/lib/activate/dev_manager.c
@@ -1622,7 +1622,6 @@ static int _clean_tree(struct dev_manager *dm, struct dm_tree_node *root, char *
 	struct dm_tree_node *child;
 	char *vgname, *lvname, *layer;
 	const char *name, *uuid;
-	int r;
 
 	while ((child = dm_tree_next_child(&handle, root, 0))) {
 		if (!(name = dm_tree_node_get_name(child)))
@@ -1644,12 +1643,7 @@ static int _clean_tree(struct dev_manager *dm, struct dm_tree_node *root, char *
 		if (non_toplevel_tree_dlid && !strcmp(non_toplevel_tree_dlid, uuid))
 			continue;
 
-		dm_tree_set_cookie(root, 0);
-		r = dm_tree_deactivate_children(root, uuid, strlen(uuid));
-		if (!dm_udev_wait(dm_tree_get_cookie(root)))
-			stack;
-
-		if (!r)
+		if (!dm_tree_deactivate_children(root, uuid, strlen(uuid)))
 			return_0;
 	}
 
@@ -1669,9 +1663,12 @@ static int _tree_action(struct dev_manager *dm, struct logical_volume *lv,
 
 	if (!(root = dm_tree_find_node(dtree, 0, 0))) {
 		log_error("Lost dependency tree root node");
-		goto out;
+		goto out_no_root;
 	}
 
+	/* Restore fs cookie */
+	dm_tree_set_cookie(root, fs_get_cookie());
+
 	if (!(dlid = build_dm_uuid(dm->mem, lv->lvid.s, origin_only ? "real" : NULL)))
 		goto_out;
 
@@ -1684,10 +1681,7 @@ static int _tree_action(struct dev_manager *dm, struct logical_volume *lv,
 		break;
 	case DEACTIVATE:
  		/* Deactivate LV and all devices it references that nothing else has open. */
-		dm_tree_set_cookie(root, 0);
 		r = dm_tree_deactivate_children(root, dlid, ID_LEN + sizeof(UUID_PREFIX) - 1);
-		if (!dm_udev_wait(dm_tree_get_cookie(root)))
-			stack;
 		if (!r)
 			goto_out;
 		if (!_remove_lv_symlinks(dm, root))
@@ -1708,10 +1702,7 @@ static int _tree_action(struct dev_manager *dm, struct logical_volume *lv,
 			goto_out;
 
 		/* Preload any devices required before any suspensions */
-		dm_tree_set_cookie(root, 0);
 		r = dm_tree_preload_children(root, dlid, ID_LEN + sizeof(UUID_PREFIX) - 1);
-		if (!dm_udev_wait(dm_tree_get_cookie(root)))
-			stack;
 		if (!r)
 			goto_out;
 
@@ -1719,10 +1710,7 @@ static int _tree_action(struct dev_manager *dm, struct logical_volume *lv,
 			dm->flush_required = 1;
 
 		if (action == ACTIVATE) {
-			dm_tree_set_cookie(root, 0);
 			r = dm_tree_activate_children(root, dlid, ID_LEN + sizeof(UUID_PREFIX) - 1);
-			if (!dm_udev_wait(dm_tree_get_cookie(root)))
-				stack;
 			if (!r)
 				goto_out;
 			if (!_create_lv_symlinks(dm, root)) {
@@ -1740,6 +1728,9 @@ static int _tree_action(struct dev_manager *dm, struct logical_volume *lv,
 	r = 1;
 
 out:
+	/* Save fs cookie for udev settle, do not wait here */
+	fs_set_cookie(dm_tree_get_cookie(root));
+out_no_root:
 	dm_tree_free(dtree);
 
 	return r;
-- 
1.7.3.4

[PATCH 7/8] Stop calling fs_unlock() and dm_udev_wait()

[Alasdair G Kergon]

On Fri, Jan 07, 2011 at 12:17:35PM +0100, Zdenek Kabelac wrote:
> Stop calling fs_unlock() from lv_de/activate()
 
Ack.

> @@ -1669,9 +1663,12 @@ static int _tree_action(struct dev_manager *dm, struct logical_volume *lv,
  
> +	/* Restore fs cookie */

Restore?

If a cookie is already allocated, re-use it; otherwise create a new one?

> +	dm_tree_set_cookie(root, fs_get_cookie());

Alasdair

[PATCH 8/8] Add fs_unlock calls

[Zdenek Kabelac]

Add lvm_do_fs_unlock()

Call fs_unlock() when unlocking vg where implicit unlock solves the
problem also for cluster - thus no extra command for clustering
environment is required - only lvm_do_fs_unlock() function is added
to call lvm's fs_unlock() while holding lvm_lock mutex in clvmd.

Add fs_unlock() also to set_lv() so the command waits until devices
are ready for regular open (i.e. wiping its begining).

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 daemons/clvmd/clvmd-command.c |    1 +
 daemons/clvmd/lvm-functions.c |    8 ++++++++
 daemons/clvmd/lvm-functions.h |    1 +
 include/.symlinks.in          |    1 +
 lib/locking/file_locking.c    |    2 ++
 lib/metadata/lv_manip.c       |    3 +++
 6 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/daemons/clvmd/clvmd-command.c b/daemons/clvmd/clvmd-command.c
index b1ea9fa..f613e0c 100644
--- a/daemons/clvmd/clvmd-command.c
+++ b/daemons/clvmd/clvmd-command.c
@@ -212,6 +212,7 @@ static int lock_vg(struct local_client *client)
 	if (lkid == 0)
 	    return EINVAL;
 
+	lvm_do_fs_unlock(); /* Wait for devices */
 	status = sync_unlock(lockname, lkid);
 	if (status)
 	    status = errno;
diff --git a/daemons/clvmd/lvm-functions.c b/daemons/clvmd/lvm-functions.c
index 214f229..b74d3da 100644
--- a/daemons/clvmd/lvm-functions.c
+++ b/daemons/clvmd/lvm-functions.c
@@ -30,6 +30,7 @@
 #include "activate.h"
 #include "archiver.h"
 #include "memlock.h"
+#include "fs.h"
 
 #include <syslog.h>
 
@@ -894,6 +895,13 @@ struct dm_hash_node *get_next_excl_lock(struct dm_hash_node *v, char **name)
 	return v;
 }
 
+void lvm_do_fs_unlock(void)
+{
+	pthread_mutex_lock(&lvm_lock);
+	fs_unlock();
+	pthread_mutex_unlock(&lvm_lock);
+}
+
 /* Called to initialise the LVM context of the daemon */
 int init_clvm(int using_gulm, char **argv)
 {
diff --git a/daemons/clvmd/lvm-functions.h b/daemons/clvmd/lvm-functions.h
index 97153d4..a132f4a 100644
--- a/daemons/clvmd/lvm-functions.h
+++ b/daemons/clvmd/lvm-functions.h
@@ -36,5 +36,6 @@ extern char *get_last_lvm_error(void);
 extern void do_lock_vg(unsigned char command, unsigned char lock_flags,
 		      char *resource);
 extern struct dm_hash_node *get_next_excl_lock(struct dm_hash_node *v, char **name);
+void lvm_do_fs_unlock(void);
 
 #endif
diff --git a/include/.symlinks.in b/include/.symlinks.in
index c16e107..263c530 100644
--- a/include/.symlinks.in
+++ b/include/.symlinks.in
@@ -2,6 +2,7 @@
 @top_srcdir@/daemons/dmeventd/libdevmapper-event.h
 @top_srcdir@/liblvm/lvm2app.h
 @top_srcdir@/lib/activate/activate.h
+ at top_srcdir@/lib/activate/fs.h
 @top_srcdir@/lib/activate/targets.h
 @top_srcdir@/lib/cache/lvmcache.h
 @top_srcdir@/lib/commands/errors.h
diff --git a/lib/locking/file_locking.c b/lib/locking/file_locking.c
index d74bfd1..566a05b 100644
--- a/lib/locking/file_locking.c
+++ b/lib/locking/file_locking.c
@@ -22,6 +22,7 @@
 #include "lvm-file.h"
 #include "lvm-string.h"
 #include "lvmcache.h"
+#include "fs.h"
 
 #include <limits.h>
 #include <unistd.h>
@@ -215,6 +216,7 @@ static int _lock_file(const char *file, uint32_t flags)
 		state = 'W';
 		break;
 	case LCK_UNLOCK:
+		fs_unlock(); /* Wait until devices are available */
 		return _release_lock(file, 1);
 	default:
 		log_error("Unrecognised lock type: %d", flags & LCK_TYPE_MASK);
diff --git a/lib/metadata/lv_manip.c b/lib/metadata/lv_manip.c
index 4a984a5..6480e18 100644
--- a/lib/metadata/lv_manip.c
+++ b/lib/metadata/lv_manip.c
@@ -26,6 +26,7 @@
 #include "archiver.h"
 #include "activate.h"
 #include "str_list.h"
+#include "fs.h"
 
 struct lv_names {
 	const char *old;
@@ -3022,6 +3023,8 @@ int set_lv(struct cmd_context *cmd, struct logical_volume *lv,
 		return 0;
 	}
 
+	fs_unlock();  /* Wait until devices are available */
+
 	log_verbose("Clearing start of logical volume \"%s\"", lv->name);
 
 	if (!(dev = dev_cache_get(name, NULL))) {
-- 
1.7.3.4

[PATCH 8/8] Add fs_unlock calls

[Alasdair G Kergon]

On Fri, Jan 07, 2011 at 12:17:36PM +0100, Zdenek Kabelac wrote:
> Add lvm_do_fs_unlock()
 
Ack.

> + at top_srcdir@/lib/activate/fs.h

Please would you export it via activate.h instead?
(I'd prefer not to expose the other functions in fs.h.)

Alasdair