[PATCH 0/5] ShareVG

[PATCH 0/5] ShareVG

[Zdenek Kabelac]

New version of shared VG.
Upon review the order in patch has been a bit modified.
I've moded holder counter from vg to vginfo which simplified patch.
It's now better vissible 1-to-1 mapping between vg and vginfo.
Updated some comments.
Renamed verify_vg_crc to detect_internal_vg_cache_corruption.

Added udev patch to this patch set so it's not forgotten.

Patch was not yet well tested - I'll try to do more testing in 
the afternoon. But test-suite has passed.

Zdenek Kabelac (5):
  Add cache_vg
  Pool locking code
  Lock memory for shared VG
  Add detect_internal_vg_cache_corruption option
  Do not decode DM flags for device removal

 WHATS_NEW                  |    1 +
 doc/example.conf.in        |    5 ++
 lib/cache/lvmcache.c       |   50 +++++++++++++++++++--
 lib/cache/lvmcache.h       |    7 +++
 lib/commands/toolcontext.c |    4 ++
 lib/config/defaults.h      |    1 +
 lib/metadata/metadata.c    |   22 +++++++++
 lib/metadata/vg.c          |    9 ++++
 lib/metadata/vg.h          |    1 +
 lib/misc/lvm-globals.c     |   12 +++++
 lib/misc/lvm-globals.h     |    2 +
 libdm/libdevmapper.h       |   16 +++++++
 libdm/mm/pool-debug.c      |   30 ++++++++++++
 libdm/mm/pool-fast.c       |   66 ++++++++++++++++++++++++++-
 libdm/mm/pool.c            |  107 +++++++++++++++++++++++++++++++++++++++++++-
 make.tmpl.in               |    4 ++
 test/lib/aux.sh            |    1 +
 udev/10-dm.rules.in        |   12 +++---
 18 files changed, 337 insertions(+), 13 deletions(-)

-- 
1.7.6

[PATCH 1/5] Add cache_vg

[Zdenek Kabelac]

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 WHATS_NEW               |    1 +
 lib/cache/lvmcache.c    |   41 +++++++++++++++++++++++++++++++++++++----
 lib/cache/lvmcache.h    |    7 +++++++
 lib/metadata/metadata.c |    6 ++++++
 lib/metadata/vg.c       |    9 +++++++++
 lib/metadata/vg.h       |    1 +
 6 files changed, 61 insertions(+), 4 deletions(-)

diff --git a/WHATS_NEW b/WHATS_NEW
index 85398ad..22d4eb8 100644
--- a/WHATS_NEW
+++ b/WHATS_NEW
@@ -1,5 +1,6 @@
 Version 2.02.87 - 
 ===============================
+  Cache and share generated VG structs.
   Add dmeventd monitoring shared library for RAID.
   Add RAID metadata devices to considered devices in _add_lv_to_dtree.
   Fix renaming of RAID logical volumes.
diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c
index 517d2f3..45965cc 100644
--- a/lib/cache/lvmcache.c
+++ b/lib/cache/lvmcache.c
@@ -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-2011 Red Hat, Inc. All rights reserved.
  *
  * This file is part of LVM2.
  *
@@ -90,6 +90,8 @@ static void _free_cached_vgmetadata(struct lvmcache_vginfo *vginfo)
 	}
 
 	log_debug("Metadata cache: VG %s wiped.", vginfo->vgname);
+
+	release_vg(vginfo->cached_vg);
 }
 
 /*
@@ -662,6 +664,10 @@ struct volume_group *lvmcache_get_vg(const char *vgid, unsigned precommitted)
 	    (!precommitted && vginfo->precommitted && !critical_section()))
 		return NULL;
 
+	/* Use already-cached VG struct when available */
+	if ((vg = vginfo->cached_vg))
+		goto out;
+
 	fic.type = FMT_INSTANCE_VG | FMT_INSTANCE_MDAS | FMT_INSTANCE_AUX_MDAS;
 	fic.context.vg_ref.vg_name = vginfo->vgname;
 	fic.context.vg_ref.vg_id = vgid;
@@ -677,17 +683,44 @@ struct volume_group *lvmcache_get_vg(const char *vgid, unsigned precommitted)
 	if (!(vg = import_vg_from_config_tree(vginfo->cft, fid)))
 		goto_bad;
 
-	log_debug("Using cached %smetadata for VG %s.",
-		  vginfo->precommitted ? "pre-committed" : "", vginfo->vgname);
+	/* Cache VG struct for reuse */
+	vginfo->cached_vg = vg;
+	vginfo->holders = 1;
+	vginfo->use_count = 0;
+	vg->vginfo = vginfo;
+
+out:
+	vginfo->holders++;
+	vginfo->use_count++;
+	log_debug("Using cached %smetadata for VG %s with %u holder(s).",
+		  vginfo->precommitted ? "pre-committed " : "",
+		  vginfo->vgname, vginfo->holders);
 
 	return vg;
 
 bad:
-	release_vg(vg);
 	_free_cached_vgmetadata(vginfo);
 	return NULL;
 }
 
+int lvmcache_decrement_holders(struct lvmcache_vginfo *vginfo)
+{
+	log_debug("VG %s decrementing %d holder(s) at %p.",
+		  vginfo->cached_vg->name, vginfo->holders, vginfo->cached_vg);
+
+	if (--vginfo->holders)
+		return 1;
+
+	if (vginfo->use_count > 1)
+		log_debug("VG %s reused %d times.",
+			  vginfo->cached_vg->name, vginfo->use_count);
+
+	vginfo->cached_vg->vginfo = NULL;
+	vginfo->cached_vg = NULL;
+
+	return 0;
+}
+
 struct dm_list *lvmcache_get_vgids(struct cmd_context *cmd,
 				   int include_internal)
 {
diff --git a/lib/cache/lvmcache.h b/lib/cache/lvmcache.h
index 9aafff5..444ed33 100644
--- a/lib/cache/lvmcache.h
+++ b/lib/cache/lvmcache.h
@@ -50,6 +50,9 @@ struct lvmcache_vginfo {
 	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 */
+	struct volume_group *cached_vg;
+	unsigned holders;
+	unsigned use_count;     /* Counter of vg reusage */
 	unsigned precommitted;	/* Is vgmetadata live or precommitted? */
 };
 
@@ -122,6 +125,10 @@ struct dm_list *lvmcache_get_pvids(struct cmd_context *cmd, const char *vgname,
 
 /* Returns cached volume group metadata. */
 struct volume_group *lvmcache_get_vg(const char *vgid, unsigned precommitted);
+
+/* Decrement vginfo holders in lvmcache. */
+int lvmcache_decrement_holders(struct lvmcache_vginfo *vginfo);
+
 void lvmcache_drop_metadata(const char *vgname, int drop_precommitted);
 void lvmcache_commit_metadata(const char *vgname);
 
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index ee29f25..44f5fa5 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -866,6 +866,12 @@ static struct volume_group *_vg_make_handle(struct cmd_context *cmd,
 					    struct volume_group *vg,
 					    uint32_t failure)
 {
+	/* Never return a cached VG structure for a failure */
+	if (vg && vg->vginfo && failure != SUCCESS) {
+		release_vg(vg);
+		vg = NULL;
+	}
+
 	if (!vg && !(vg = alloc_vg("vg_make_handle", cmd, NULL)))
 		return_NULL;
 
diff --git a/lib/metadata/vg.c b/lib/metadata/vg.c
index ef88a4d..07336c5 100644
--- a/lib/metadata/vg.c
+++ b/lib/metadata/vg.c
@@ -18,6 +18,7 @@
 #include "display.h"
 #include "activate.h"
 #include "toolcontext.h"
+#include "lvmcache.h"
 
 struct volume_group *alloc_vg(const char *pool_name, struct cmd_context *cmd,
 			      const char *vg_name)
@@ -49,6 +50,8 @@ struct volume_group *alloc_vg(const char *pool_name, struct cmd_context *cmd,
 	dm_list_init(&vg->tags);
 	dm_list_init(&vg->removed_pvs);
 
+	log_debug("Allocated VG %s at %p.", vg->name, vg);
+
 	return vg;
 }
 
@@ -62,6 +65,8 @@ static void _free_vg(struct volume_group *vg)
 		return;
 	}
 
+	log_debug("Freeing VG %s at %p.", vg->name, vg);
+
 	dm_pool_destroy(vg->vgmem);
 }
 
@@ -70,6 +75,10 @@ void release_vg(struct volume_group *vg)
 	if (!vg)
 		return;
 
+	/* Decrement holders and check if vg is still in use */
+	if (vg->vginfo && lvmcache_decrement_holders(vg->vginfo))
+		return;
+
 	_free_vg(vg);
 }
 
diff --git a/lib/metadata/vg.h b/lib/metadata/vg.h
index 5248888..9c21f37 100644
--- a/lib/metadata/vg.h
+++ b/lib/metadata/vg.h
@@ -41,6 +41,7 @@ struct volume_group {
 	struct cmd_context *cmd;
 	struct dm_pool *vgmem;
 	struct format_instance *fid;
+	struct lvmcache_vginfo *vginfo;
 	struct dm_list *cmd_vgs;/* List of wanted/locked and opened VGs */
 	uint32_t cmd_missing_vgs;/* Flag marks missing VG */
 	uint32_t seqno;		/* Metadata sequence number */
-- 
1.7.6

[PATCH 2/5] Pool locking code

[Zdenek Kabelac]

Adding specific debug functions to lock and unlock memory pool.

2 ways to debug code:
crc - is default checksum/hash of the locked pool.
      It gets slower when the pool is larger - so the check is only
      made when VG is finaly released and it has been used more then
      once.Thus the result is rather informative.

mprotect - quite fast all the time - but requires more memory and
           currently it is using posix_memalign() - this could be
	   later modified to use dm_malloc() and align internally.
           Tool segfaults when locked memory is modified and core
	   could be examined for faulty code section (backtrace).

Only fast memory pools are using mprotect -
so such debug builds cannot be combined with DEBUG_POOL.

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 libdm/libdevmapper.h  |   16 +++++++
 libdm/mm/pool-debug.c |   30 ++++++++++++++
 libdm/mm/pool-fast.c  |   66 +++++++++++++++++++++++++++++-
 libdm/mm/pool.c       |  107 ++++++++++++++++++++++++++++++++++++++++++++++++-
 make.tmpl.in          |    4 ++
 5 files changed, 220 insertions(+), 3 deletions(-)

diff --git a/libdm/libdevmapper.h b/libdm/libdevmapper.h
index 02f8ed6..8a51509 100644
--- a/libdm/libdevmapper.h
+++ b/libdm/libdevmapper.h
@@ -614,6 +614,22 @@ void dm_pool_empty(struct dm_pool *p);
 void dm_pool_free(struct dm_pool *p, void *ptr);
 
 /*
+ * To aid debugging, a pool can be locked. Any modifications made
+ * to the content of the pool while it is locked can be detected.
+ * Default compilation is using a crc checksum to notice modifications.
+ * The pool locking is using the mprotect with the compilation flag
+ * DEBUG_ENFORCE_POOL_LOCKING to enforce the memory protection.
+ */
+/* query pool lock status */
+int dm_pool_locked(struct dm_pool *p);
+/* mark pool as locked */
+int dm_pool_lock(struct dm_pool *p, int crc)
+	__attribute__((__warn_unused_result__));
+/* mark pool as unlocked */
+int dm_pool_unlock(struct dm_pool *p, int crc)
+	__attribute__((__warn_unused_result__));
+
+/*
  * Object building routines:
  *
  * These allow you to 'grow' an object, useful for
diff --git a/libdm/mm/pool-debug.c b/libdm/mm/pool-debug.c
index af1a912..7f9a0e4 100644
--- a/libdm/mm/pool-debug.c
+++ b/libdm/mm/pool-debug.c
@@ -33,6 +33,8 @@ struct dm_pool {
 	struct dm_list list;
 	const char *name;
 	void *orig_pool;	/* to pair it with first allocation call */
+	unsigned locked;
+	long crc;
 
 	int begun;
 	struct block *object;
@@ -71,6 +73,10 @@ static void _free_blocks(struct dm_pool *p, struct block *b)
 {
 	struct block *n;
 
+	if (p->locked)
+		log_error(INTERNAL_ERROR "_free_blocks from locked pool %s",
+			  p->name);
+
 	while (b) {
 		p->stats.bytes -= b->size;
 		p->stats.blocks_allocated--;
@@ -109,6 +115,10 @@ void *dm_pool_alloc(struct dm_pool *p, size_t s)
 
 static void _append_block(struct dm_pool *p, struct block *b)
 {
+	if (p->locked)
+		log_error(INTERNAL_ERROR "_append_blocks to locked pool %s",
+			  p->name);
+
 	if (p->tail) {
 		p->tail->next = b;
 		p->tail = b;
@@ -216,6 +226,10 @@ int dm_pool_grow_object(struct dm_pool *p, const void *extra, size_t delta)
 	struct block *new;
 	size_t new_size;
 
+	if (p->locked)
+		log_error(INTERNAL_ERROR "Grow objects in locked pool %s",
+			  p->name);
+
 	if (!delta)
 		delta = strlen(extra);
 
@@ -260,3 +274,19 @@ void dm_pool_abandon_object(struct dm_pool *p)
 	p->begun = 0;
 	p->object = NULL;
 }
+
+static long _pool_crc(const struct dm_pool *p)
+{
+#ifndef DEBUG_ENFORCE_POOL_LOCKING
+#warning pool crc not implemented with pool debug
+#endif
+	return 0;
+}
+
+static int _pool_protect(struct dm_pool *p, int prot)
+{
+#ifdef DEBUG_ENFORCE_POOL_LOCKING
+#warning pool mprotect not implemented with pool debug
+#endif
+	return 1;
+}
diff --git a/libdm/mm/pool-fast.c b/libdm/mm/pool-fast.c
index 29d61a4..45b319f 100644
--- a/libdm/mm/pool-fast.c
+++ b/libdm/mm/pool-fast.c
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2001-2004 Sistina Software, Inc. All rights reserved.
- * Copyright (C) 2004-2010 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2004-2011 Red Hat, Inc. All rights reserved.
  *
  * This file is part of the device-mapper userspace tools.
  *
@@ -18,6 +18,7 @@
 #endif
 
 #include "dmlib.h"
+#include <malloc.h>
 
 struct chunk {
 	char *begin, *end;
@@ -32,6 +33,8 @@ struct dm_pool {
 	size_t chunk_size;
 	size_t object_len;
 	unsigned object_alignment;
+	int locked;
+	long crc;
 };
 
 static void _align_chunk(struct chunk *c, unsigned alignment);
@@ -260,7 +263,23 @@ static struct chunk *_new_chunk(struct dm_pool *p, size_t s)
 		c = p->spare_chunk;
 		p->spare_chunk = 0;
 	} else {
-		if (!(c = dm_malloc(s))) {
+#ifdef DEBUG_ENFORCE_POOL_LOCKING
+		if (!pagesize) {
+			pagesize = getpagesize(); /* lvm_pagesize(); */
+			pagesize_mask = pagesize - 1;
+		}
+		/*
+		 * Allocate page aligned size so malloc could work.
+		 * Otherwise page fault would happen from pool unrelated
+		 * memory writes of internal malloc pointers.
+		 */
+#  define aligned_malloc(s)	(posix_memalign((void**)&c, pagesize, \
+						ALIGN_ON_PAGE(s)) == 0)
+#else
+#  define aligned_malloc(s)	(c = dm_malloc(s))
+#endif /* DEBUG_ENFORCE_POOL_LOCKING */
+		if (!aligned_malloc(s)) {
+#undef aligned_malloc
 			log_error("Out of memory.  Requested %" PRIsize_t
 				  " bytes.", s);
 			return NULL;
@@ -283,3 +302,46 @@ static void _free_chunk(struct chunk *c)
 {
 	dm_free(c);
 }
+
+
+/**
+ * Calc crc/hash from pool's memory chunks with internal pointers
+ */
+static long _pool_crc(const struct dm_pool *p)
+{
+	long crc_hash = 0;
+#ifndef DEBUG_ENFORCE_POOL_LOCKING
+	const struct chunk *c;
+	const long *ptr, *end;
+
+	for (c = p->chunk; c; c = c->prev) {
+		end = (const long *) (c->begin < c->end ? (long) c->begin & ~7: (long) c->end);
+		ptr = (const long *) c;
+#ifdef VALGRIND_POOL
+		VALGRIND_MAKE_MEM_DEFINED(ptr, (end - ptr) * sizeof(*end));
+#endif
+		while (ptr < end) {
+			crc_hash += *ptr++;
+			crc_hash += (crc_hash << 10);
+			crc_hash ^= (crc_hash >> 6);
+		}
+	}
+#endif /* DEBUG_ENFORCE_POOL_LOCKING */
+
+	return crc_hash;
+}
+
+static int _pool_protect(struct dm_pool *p, int prot)
+{
+#ifdef DEBUG_ENFORCE_POOL_LOCKING
+	struct chunk *c;
+
+	for (c = p->chunk; c; c = c->prev) {
+		if (mprotect(c, (size_t) ((c->end - (char *) c) - 1), prot) != 0) {
+			log_sys_error("mprotect", "");
+			return 0;
+		}
+	}
+#endif
+	return 1;
+}
diff --git a/libdm/mm/pool.c b/libdm/mm/pool.c
index 3df6071..b81a9b6 100644
--- a/libdm/mm/pool.c
+++ b/libdm/mm/pool.c
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2001-2004 Sistina Software, Inc. All rights reserved.  
- * Copyright (C) 2004-2005 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2004-2011 Red Hat, Inc. All rights reserved.
  *
  * This file is part of the device-mapper userspace tools.
  *
@@ -14,11 +14,31 @@
  */
 
 #include "dmlib.h"
+#include <sys/mman.h>
 
 /* FIXME: thread unsafe */
 static DM_LIST_INIT(_dm_pools);
 void dm_pools_check_leaks(void);
 
+#ifdef DEBUG_ENFORCE_POOL_LOCKING
+#ifdef DEBUG_POOL
+#error Do not use DEBUG_POOL with DEBUG_ENFORCE_POOL_LOCKING
+#endif
+
+/*
+ * Use mprotect system call to ensure all locked pages are not writable.
+ * Generates segmentation fault with write access to the locked pool.
+ *
+ * - Implementation is using posix_memalign() to get page aligned
+ *   memory blocks (could be implemented also through malloc).
+ * - Only pool-fast is properly handled for now.
+ * - Checksum is slower compared to mprotect.
+ */
+static size_t pagesize = 0;
+static size_t pagesize_mask = 0;
+#define ALIGN_ON_PAGE(size) (((size) + (pagesize_mask)) & ~(pagesize_mask))
+#endif
+
 #ifdef DEBUG_POOL
 #include "pool-debug.c"
 #else
@@ -76,3 +96,88 @@ void dm_pools_check_leaks(void)
 	}
 	log_error(INTERNAL_ERROR "Unreleased memory pool(s) found.");
 }
+
+/**
+ * Status of locked pool.
+ *
+ * \param p
+ * Pool to be tested for lock status.
+ *
+ * \return
+ * 1 when the pool is locked, 0 otherwise.
+ */
+int dm_pool_locked(struct dm_pool *p)
+{
+	return p->locked;
+}
+
+/**
+ * Lock memory pool.
+ *
+ * \param p
+ * Pool to be locked.
+ *
+ * \param crc
+ * Bool specifies whether to store the pool crc/hash checksum.
+ *
+ * \return
+ * 1 (success) when the pool was preperly locked, 0 otherwise.
+ */
+int dm_pool_lock(struct dm_pool *p, int crc)
+{
+	if (p->locked) {
+		log_error(INTERNAL_ERROR "Pool %s is already locked.",
+			  p->name);
+		return 0;
+	}
+
+	if (crc)
+		p->crc = _pool_crc(p);  /* Get crc for pool */
+
+	if (!_pool_protect(p, PROT_READ)) {
+		_pool_protect(p, PROT_READ | PROT_WRITE);
+		return_0;
+	}
+
+	p->locked = 1;
+
+	log_debug("Pool %s is locked.", p->name);
+
+	return 1;
+}
+
+/**
+ * Unlock memory pool.
+ *
+ * \param p
+ * Pool to be unlocked.
+ *
+ * \param crc
+ * Bool enables compare of the pool crc/hash with the stored value
+ * at pool lock. The pool is not properly unlocked if there is a mismatch.
+ *
+ * \return
+ * 1 (success) when the pool was properly unlocked, 0 otherwise.
+ */
+int dm_pool_unlock(struct dm_pool *p, int crc)
+{
+	if (!p->locked) {
+		log_error(INTERNAL_ERROR "Pool %s is already unlocked.",
+			  p->name);
+		return 0;
+	}
+
+	p->locked = 0;
+
+	if (!_pool_protect(p, PROT_READ | PROT_WRITE))
+		return_0;
+
+	log_debug("Pool %s is unlocked.", p->name);
+
+	if (crc && (p->crc != _pool_crc(p))) {
+		log_error(INTERNAL_ERROR "Pool %s crc mismatch.", p->name);
+		return 0;
+	}
+
+	return 1;
+}
diff --git a/make.tmpl.in b/make.tmpl.in
index f003149..a58b313 100644
--- a/make.tmpl.in
+++ b/make.tmpl.in
@@ -149,7 +149,11 @@ ifeq ("@DM_IOCTLS@", "yes")
   DEFS += -DDM_IOCTLS
 endif
 
+# Combination of DEBUG_POOL and DEBUG_ENFORCE_POOL_LOCKING is not suppored.
 #DEFS += -DDEBUG_POOL
+# Default pool locking is using the crc checksum. With mprotect memory
+# enforcing compilation faulty memory write could be easily found.
+#DEFS += -DDEBUG_ENFORCE_POOL_LOCKING
 #DEFS += -DBOUNDS_CHECK
 
 #CFLAGS += -pg
-- 
1.7.6

[PATCH 3/5] Lock memory for shared VG

[Zdenek Kabelac]

Add debug pool locking functionality. So the command could check,
whether the memory in the pool was not modified.

For lv_postoder() instead of unlocking and locking for every changed
struct status member do it once when entering and leaving function.
Currently lv_postoder() does not modify other part of vg structure
then status flags of each LV with flags that are reverted back to
its original state after fucntion exit.

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 lib/cache/lvmcache.c    |    8 ++++++++
 lib/metadata/metadata.c |   16 ++++++++++++++++
 2 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c
index 45965cc..38d8934 100644
--- a/lib/cache/lvmcache.c
+++ b/lib/cache/lvmcache.c
@@ -689,6 +689,9 @@ struct volume_group *lvmcache_get_vg(const char *vgid, unsigned precommitted)
 	vginfo->use_count = 0;
 	vg->vginfo = vginfo;
 
+	if (!dm_pool_lock(vg->vgmem, 1))
+		goto_bad;
+
 out:
 	vginfo->holders++;
 	vginfo->use_count++;
@@ -715,6 +718,11 @@ int lvmcache_decrement_holders(struct lvmcache_vginfo *vginfo)
 		log_debug("VG %s reused %d times.",
 			  vginfo->cached_vg->name, vginfo->use_count);
 
+	/* Debug perform crc check only when it's been used more then once */
+	if (!dm_pool_unlock(vginfo->cached_vg->vgmem,
+			    (vginfo->use_count > 1)))
+		stack;
+
 	vginfo->cached_vg->vginfo = NULL;
 	vginfo->cached_vg = NULL;
 
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 44f5fa5..f846f6e 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -2107,8 +2107,17 @@ static int _lv_postorder(struct logical_volume *lv,
 			       void *data)
 {
 	int r;
+	int pool_locked = dm_pool_locked(lv->vg->vgmem);
+
+	if (pool_locked && !dm_pool_unlock(lv->vg->vgmem, 0))
+		return_0;
+
 	r = _lv_postorder_visit(lv, fn, data);
 	_lv_postorder_cleanup(lv, 0);
+
+	if (pool_locked && !dm_pool_lock(lv->vg->vgmem, 0))
+		return_0;
+
 	return r;
 }
 
@@ -2122,6 +2131,10 @@ static int _lv_postorder_vg(struct volume_group *vg,
 {
 	struct lv_list *lvl;
 	int r = 1;
+	int pool_locked = dm_pool_locked(vg->vgmem);
+
+	if (pool_locked && !dm_pool_unlock(vg->vgmem, 0))
+		return_0;
 
 	dm_list_iterate_items(lvl, &vg->lvs)
 		if (!_lv_postorder_visit(lvl->lv, fn, data)) {
@@ -2132,6 +2145,9 @@ static int _lv_postorder_vg(struct volume_group *vg,
 	dm_list_iterate_items(lvl, &vg->lvs)
 		_lv_postorder_cleanup(lvl->lv, 0);
 
+	if (pool_locked && !dm_pool_lock(vg->vgmem, 0))
+		return_0;
+
 	return r;
 }
 
-- 
1.7.6

[PATCH 4/5] Add detect_internal_vg_cache_corruption option

[Zdenek Kabelac]

Add config option to enable crc checking of VG structures.

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 doc/example.conf.in        |    5 +++++
 lib/cache/lvmcache.c       |    3 ++-
 lib/commands/toolcontext.c |    4 ++++
 lib/config/defaults.h      |    1 +
 lib/misc/lvm-globals.c     |   12 ++++++++++++
 lib/misc/lvm-globals.h     |    2 ++
 test/lib/aux.sh            |    1 +
 7 files changed, 27 insertions(+), 1 deletions(-)

diff --git a/doc/example.conf.in b/doc/example.conf.in
index 7fe8d59..5788060 100644
--- a/doc/example.conf.in
+++ b/doc/example.conf.in
@@ -402,6 +402,11 @@ global {
     # encountered the internal error. Please only enable for debugging.
     abort_on_internal_errors = 0
 
+    # Check whether CRC is matching when parsed VG is used multiple times.
+    # This is useful to catch unexpected internal cached volume group
+    # structure modification. Please only enable for debugging.
+    detect_internal_vg_cache_corruption = 0
+
     # If set to 1, no operations that change on-disk metadata will be permitted.
     # Additionally, read-only commands that encounter metadata in need of repair
     # will still be allowed to proceed exactly as if the repair had been 
diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c
index 38d8934..4a9f2cb 100644
--- a/lib/cache/lvmcache.c
+++ b/lib/cache/lvmcache.c
@@ -689,7 +689,7 @@ struct volume_group *lvmcache_get_vg(const char *vgid, unsigned precommitted)
 	vginfo->use_count = 0;
 	vg->vginfo = vginfo;
 
-	if (!dm_pool_lock(vg->vgmem, 1))
+	if (!dm_pool_lock(vg->vgmem, detect_internal_vg_cache_corruption()))
 		goto_bad;
 
 out:
@@ -720,6 +720,7 @@ int lvmcache_decrement_holders(struct lvmcache_vginfo *vginfo)
 
 	/* Debug perform crc check only when it's been used more then once */
 	if (!dm_pool_unlock(vginfo->cached_vg->vgmem,
+			    detect_internal_vg_cache_corruption() &&
 			    (vginfo->use_count > 1)))
 		stack;
 
diff --git a/lib/commands/toolcontext.c b/lib/commands/toolcontext.c
index b3d368a..3cbd29d 100644
--- a/lib/commands/toolcontext.c
+++ b/lib/commands/toolcontext.c
@@ -373,6 +373,10 @@ static int _process_config(struct cmd_context *cmd)
 	/* LVM stores sizes internally in units of 512-byte sectors. */
 	init_pv_min_size((uint64_t)pv_min_kb * (1024 >> SECTOR_SHIFT));
 
+	init_detect_internal_vg_cache_corruption
+		(find_config_tree_int(cmd, "global/detect_internal_vg_cache_corruption()",
+				      DEFAULT_DETECT_INTERNAL_VG_CACHE_CORRUPTION));
+
 	return 1;
 }
 
diff --git a/lib/config/defaults.h b/lib/config/defaults.h
index cfac4d5..8b8e09b 100644
--- a/lib/config/defaults.h
+++ b/lib/config/defaults.h
@@ -103,6 +103,7 @@
 #define DEFAULT_LOGLEVEL 0
 #define DEFAULT_INDENT 1
 #define DEFAULT_ABORT_ON_INTERNAL_ERRORS 0
+#define DEFAULT_DETECT_INTERNAL_VG_CACHE_CORRUPTION 0
 #define DEFAULT_UNITS "h"
 #define DEFAULT_SUFFIX 1
 #define DEFAULT_HOSTTAGS 0
diff --git a/lib/misc/lvm-globals.c b/lib/misc/lvm-globals.c
index b9ece7f..f774512 100644
--- a/lib/misc/lvm-globals.c
+++ b/lib/misc/lvm-globals.c
@@ -46,6 +46,8 @@ static int _activation_checks = 0;
 static char _sysfs_dir_path[PATH_MAX] = "";
 static int _dev_disable_after_error_count = DEFAULT_DISABLE_AFTER_ERROR_COUNT;
 static uint64_t _pv_min_size = (DEFAULT_PV_MIN_SIZE_KB * 1024L >> SECTOR_SHIFT);
+static int _detect_internal_vg_cache_corruption =
+	DEFAULT_DETECT_INTERNAL_VG_CACHE_CORRUPTION;
 
 void init_verbose(int level)
 {
@@ -150,6 +152,11 @@ void init_pv_min_size(uint64_t sectors)
 	_pv_min_size = sectors;
 }
 
+void init_detect_internal_vg_cache_corruption(int detect)
+{
+	_detect_internal_vg_cache_corruption = detect;
+}
+
 void set_cmd_name(const char *cmd)
 {
 	strncpy(_cmd_name, cmd, sizeof(_cmd_name));
@@ -284,3 +291,8 @@ uint64_t pv_min_size(void)
 {
 	return _pv_min_size;
 }
+
+int detect_internal_vg_cache_corruption(void)
+{
+	return _detect_internal_vg_cache_corruption;
+}
diff --git a/lib/misc/lvm-globals.h b/lib/misc/lvm-globals.h
index 2cdfdd1..fcba687 100644
--- a/lib/misc/lvm-globals.h
+++ b/lib/misc/lvm-globals.h
@@ -41,6 +41,7 @@ void init_udev_checking(int checking);
 void init_dev_disable_after_error_count(int value);
 void init_pv_min_size(uint64_t sectors);
 void init_activation_checks(int checks);
+void init_detect_internal_vg_cache_corruption(int detect);
 
 void set_cmd_name(const char *cmd_name);
 void set_sysfs_dir_path(const char *path);
@@ -65,6 +66,7 @@ int udev_checking(void);
 const char *sysfs_dir_path(void);
 uint64_t pv_min_size(void);
 int activation_checks(void);
+int detect_internal_vg_cache_corruption(void);
 
 #define DMEVENTD_MONITOR_IGNORE -1
 int dmeventd_monitor_mode(void);
diff --git a/test/lib/aux.sh b/test/lib/aux.sh
index 5a9bf91..4410007 100644
--- a/test/lib/aux.sh
+++ b/test/lib/aux.sh
@@ -386,6 +386,7 @@ log/activation = 1
 backup/backup = 0
 backup/archive = 0
 global/abort_on_internal_errors = 1
+global/detect_internal_vg_cache_corruption = 1
 global/library_dir = "$TESTDIR/lib"
 global/locking_dir = "$TESTDIR/var/lock/lvm"
 global/locking_type=$LVM_TEST_LOCKING
-- 
1.7.6

[PATCH 5/5] Do not decode DM flags for device removal

[Zdenek Kabelac]

Proposing to skip decoding of DM flags when device is removed.

We currently do some actions only on add|change event. So forking
dmsetup process for device removal is just waste of CPU time for now.

Udev is already quite slow - so make it just a tiny bit faster with
this patch.

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 udev/10-dm.rules.in |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/udev/10-dm.rules.in b/udev/10-dm.rules.in
index 606a13d..9e8ad0f 100644
--- a/udev/10-dm.rules.in
+++ b/udev/10-dm.rules.in
@@ -24,12 +24,6 @@ ENV{DM_SBIN_PATH}="/sbin"
 TEST!="$env{DM_SBIN_PATH}/dmsetup", ENV{DM_SBIN_PATH}="/usr/sbin"
 TEST!="$env{DM_SBIN_PATH}/dmsetup", GOTO="dm_end"
 
-# Decode udev control flags and set environment variables appropriately.
-# These flags are encoded in DM_COOKIE variable that was introduced in
-# kernel version 2.6.31. Therefore, we can use this feature with
-# kernels >= 2.6.31 only.
-ENV{DM_COOKIE}=="?*", IMPORT{program}="$env{DM_SBIN_PATH}/dmsetup udevflags $env{DM_COOKIE}"
-
 # Device created, major and minor number assigned - "add" event generated.
 # Table loaded - no event generated.
 # Device resumed (or renamed) - "change" event generated.
@@ -42,6 +36,12 @@ ENV{DM_COOKIE}=="?*", IMPORT{program}="$env{DM_SBIN_PATH}/dmsetup udevflags $env
 # is not recommended.
 ACTION!="add|change", GOTO="dm_end"
 
+# Decode udev control flags and set environment variables appropriately.
+# These flags are encoded in DM_COOKIE variable that was introduced in
+# kernel version 2.6.31. Therefore, we can use this feature with
+# kernels >= 2.6.31 only.
+ENV{DM_COOKIE}=="?*", IMPORT{program}="$env{DM_SBIN_PATH}/dmsetup udevflags $env{DM_COOKIE}"
+
 # Rule out easy-to-detect inappropriate events first.
 ENV{DISK_RO}=="1", GOTO="dm_disable"
 
-- 
1.7.6