Maintain a count of the number of suspended devices in libdevmapper

and use this for the LVM critical section logic.  Also report an error if
code tries to load a table while any device is known to be in the
suspended state.
(If the variety of problems these changes are showing up can't be fixed
before the next release, the error messages can be reduced to debug
level.)

8 files changed, 81 insertions, 24 deletions

diff --git a/WHATS_NEW b/WHATS_NEW
index 4f5b4a3a..c2af237d 100644
--- a/WHATS_NEW
+++ b/WHATS_NEW
@@ -1,5 +1,6 @@
 Version 2.02.86 -  
 =================================
+  Use dm_get_suspended_counter in replacement critical_section logic.
   Downgrade critical_section errors to debug level until it is moved to libdm.
   Fix ignored background polling default in vgchange -ay.
   Fix pvmove activation sequences to avoid trapped I/O with multiple LVs.
diff --git a/WHATS_NEW_DM b/WHATS_NEW_DM
index 038c18d8..5be3acf8 100644
--- a/WHATS_NEW_DM
+++ b/WHATS_NEW_DM
@@ -1,5 +1,7 @@
 Version 1.02.65 - 
 ==================================
+  Warn if a table is loaded while a device is known to be in suspended state.
+  Add dm_get_suspended_counter() for number of devs in suspended state by lib.
   Fix "all" report field prefix matching to include label fields with pv_all.
   Delay resuming new preloaded mirror devices with core logs in deptree code.
   Accept new kernel version 3 uname formats in initialisation.
diff --git a/lib/mm/memlock.c b/lib/mm/memlock.c
index 201eff2f..bdda2ca5 100644
--- a/lib/mm/memlock.c
+++ b/lib/mm/memlock.c
@@ -75,7 +75,7 @@ static size_t _size_malloc = 2000000;
 
 static void *_malloc_mem = NULL;
 static int _mem_locked = 0;
-static int _critical_section_count = 0;
+static int _critical_section = 0;
 static int _memlock_count_daemon = 0;
 static int _priority;
 static int _default_priority;
@@ -374,10 +374,10 @@ static void _unlock_mem(struct cmd_context *cmd)
 
 static void _lock_mem_if_needed(struct cmd_context *cmd)
 {
-	log_debug("Lock: Memlock counters: locked:%d critical:%d daemon:%d",
-		  _mem_locked, _critical_section_count, _memlock_count_daemon);
+	log_debug("Lock:   Memlock counters: locked:%d critical:%d daemon:%d suspended:%d",
+		  _mem_locked, _critical_section, _memlock_count_daemon, dm_get_suspended_counter());
 	if (!_mem_locked &&
-	    ((_critical_section_count + _memlock_count_daemon) == 1)) {
+	    ((_critical_section + _memlock_count_daemon) == 1)) {
 		_mem_locked = 1;
 		_lock_mem(cmd);
 	}
@@ -385,10 +385,10 @@ static void _lock_mem_if_needed(struct cmd_context *cmd)
 
 static void _unlock_mem_if_possible(struct cmd_context *cmd)
 {
-	log_debug("Unlock: Memlock counters: locked:%d critical:%d daemon:%d",
-		  _mem_locked, _critical_section_count, _memlock_count_daemon);
+	log_debug("Unlock: Memlock counters: locked:%d critical:%d daemon:%d suspended:%d",
+		  _mem_locked, _critical_section, _memlock_count_daemon, dm_get_suspended_counter());
 	if (_mem_locked &&
-	    !_critical_section_count &&
+	    !_critical_section &&
 	    !_memlock_count_daemon) {
 		_unlock_mem(cmd);
 		_mem_locked = 0;
@@ -397,25 +397,25 @@ static void _unlock_mem_if_possible(struct cmd_context *cmd)
 
 void critical_section_inc(struct cmd_context *cmd, const char *reason)
 {
-	++_critical_section_count;
-	log_debug("critical_section_inc to %d (%s).", _critical_section_count,
-		  reason);
+	if (!_critical_section) {
+		_critical_section = 1;
+		log_debug("Entering critical section (%s).", reason);
+	}
+
 	_lock_mem_if_needed(cmd);
 }
 
 void critical_section_dec(struct cmd_context *cmd, const char *reason)
 {
-	/* FIXME Maintain accurate suspended device counter in libdevmapper */
-	if (!_critical_section_count)
-		log_debug("_critical_section has dropped below 0.");
-	--_critical_section_count;
-	log_debug("critical_section_dec to %d (%s).", _critical_section_count,
-		  reason);
+	if (_critical_section && !dm_get_suspended_counter()) {
+		_critical_section = 0;
+		log_debug("Leaving critical section (%s).", reason);
+	}
 }
 
 int critical_section(void)
 {
-	return _critical_section_count;
+	return _critical_section;
 }
 
 /*
@@ -428,7 +428,7 @@ int critical_section(void)
 void memlock_inc_daemon(struct cmd_context *cmd)
 {
 	++_memlock_count_daemon;
-	if (_memlock_count_daemon == 1 && _critical_section_count > 0)
+	if (_memlock_count_daemon == 1 && _critical_section > 0)
                 log_error(INTERNAL_ERROR "_memlock_inc_daemon used in critical section.");
 	log_debug("memlock_count_daemon inc to %d", _memlock_count_daemon);
 	_lock_mem_if_needed(cmd);
@@ -460,7 +460,7 @@ void memlock_reset(void)
 {
 	log_debug("memlock reset.");
 	_mem_locked = 0;
-	_critical_section_count = 0;
+	_critical_section = 0;
 	_memlock_count_daemon = 0;
 }
 
diff --git a/libdm/ioctl/libdm-iface.c b/libdm/ioctl/libdm-iface.c
index ad343447..fb0d43ad 100644
--- a/libdm/ioctl/libdm-iface.c
+++ b/libdm/ioctl/libdm-iface.c
@@ -2026,6 +2026,7 @@ int dm_task_run(struct dm_task *dmt)
 	unsigned command;
 	int check_udev;
 	int udev_only;
+	int suspended_counter;
 
 #ifdef DM_COMPAT
 	if (_dm_version == 1)
@@ -2057,6 +2058,22 @@ int dm_task_run(struct dm_task *dmt)
 		return 0;
 	}
 
+	if ((suspended_counter = dm_get_suspended_counter()) &&
+	    dmt->type == DM_DEVICE_RELOAD)
+		log_error("Performing unsafe table load while %d device(s) "
+			  "are known to be suspended: "
+			  "%s%s %s %s%.0d%s%.0d%s%s",
+			  suspended_counter,
+	                  dmt->new_uuid ? "UUID " : "",
+	                  dmi->name,
+			  dmi->uuid,
+	                  dmt->major > 0 ? "(" : "",
+	                  dmt->major > 0 ? dmt->major : 0,
+	                  dmt->major > 0 ? ":" : "",
+	                  dmt->minor > 0 ? dmt->minor : 0,
+	                  dmt->major > 0 && dmt->minor == 0 ? "0" : "",
+	                  dmt->major > 0 ? ") " : "");
+
 	/* FIXME Detect and warn if cookie set but should not be. */
 repeat_ioctl:
 	if (!(dmi = _do_dm_ioctl(dmt, command, _ioctl_buffer_double_factor))) {
diff --git a/libdm/libdevmapper.h b/libdm/libdevmapper.h
index 88bddb10..a3ae8807 100644
--- a/libdm/libdevmapper.h
+++ b/libdm/libdevmapper.h
@@ -74,6 +74,11 @@ void dm_log_init(dm_log_fn fn);
  */
 int dm_log_is_non_default(void);
 
+/*
+ * Number of devices currently in suspended state (via the library).
+ */
+int dm_get_suspended_counter(void);
+
 enum {
 	DM_DEVICE_CREATE,
 	DM_DEVICE_RELOAD,
diff --git a/libdm/libdm-common.c b/libdm/libdm-common.c
index 43d78588..b2b34b81 100644
--- a/libdm/libdm-common.c
+++ b/libdm/libdm-common.c
@@ -60,6 +60,7 @@ union semun
 static char _dm_dir[PATH_MAX] = DEV_DIR DM_DIR;
 
 static int _verbose = 0;
+static int _suspended_dev_counter = 0;
 
 #ifdef HAVE_SELINUX_LABEL_H
 static struct selabel_handle *_selabel_handle = NULL;
@@ -173,6 +174,28 @@ int dm_get_library_version(char *version, size_t size)
 	return 1;
 }
 
+void inc_suspended(void)
+{
+	_suspended_dev_counter++;
+	log_debug("Suspended device counter increased to %d", _suspended_dev_counter);
+}
+
+void dec_suspended(void)
+{
+	if (!_suspended_dev_counter) {
+		log_error("Attempted to decrement suspended device counter below zero.");
+		return;
+	}
+
+	_suspended_dev_counter--;
+	log_debug("Suspended device counter reduced to %d", _suspended_dev_counter);
+}
+
+int dm_get_suspended_counter(void)
+{
+	return _suspended_dev_counter;
+}
+
 struct dm_task *dm_task_create(int type)
 {
 	struct dm_task *dmt = dm_zalloc(sizeof(*dmt));
diff --git a/libdm/libdm-common.h b/libdm/libdm-common.h
index 3267cfc4..2435f274 100644
--- a/libdm/libdm-common.h
+++ b/libdm/libdm-common.h
@@ -33,4 +33,7 @@ int set_dev_node_read_ahead(const char *dev_name, uint32_t read_ahead,
 void update_devs(void);
 void selinux_release(void);
 
+void inc_suspended(void);
+void dec_suspended(void);
+
 #endif
diff --git a/libdm/libdm-deptree.c b/libdm/libdm-deptree.c
index 4860ec27..044b7ffe 100644
--- a/libdm/libdm-deptree.c
+++ b/libdm/libdm-deptree.c
@@ -1034,7 +1034,7 @@ out:
 static int _resume_node(const char *name, uint32_t major, uint32_t minor,
 			uint32_t read_ahead, uint32_t read_ahead_flags,
 			struct dm_info *newinfo, uint32_t *cookie,
-			uint16_t udev_flags)
+			uint16_t udev_flags, int already_suspended)
 {
 	struct dm_task *dmt;
 	int r = 0;
@@ -1066,8 +1066,11 @@ static int _resume_node(const char *name, uint32_t major, uint32_t minor,
 	if (!dm_task_set_cookie(dmt, cookie, udev_flags))
 		goto out;
 
-	if ((r = dm_task_run(dmt)))
+	if ((r = dm_task_run(dmt))) {
+		if (already_suspended)
+			dec_suspended();
 		r = dm_task_get_info(dmt, newinfo);
+	}
 
 out:
 	dm_task_destroy(dmt);
@@ -1106,8 +1109,10 @@ static int _suspend_node(const char *name, uint32_t major, uint32_t minor,
 	if (no_flush && !dm_task_no_flush(dmt))
 		log_error("Failed to set no_flush flag.");
 
-	if ((r = dm_task_run(dmt)))
+	if ((r = dm_task_run(dmt))) {
+		inc_suspended();
 		r = dm_task_get_info(dmt, newinfo);
+	}
 
 	dm_task_destroy(dmt);
 
@@ -1352,7 +1357,7 @@ int dm_tree_activate_children(struct dm_tree_node *dnode,
 
 			if (!_resume_node(child->name, child->info.major, child->info.minor,
 					  child->props.read_ahead, child->props.read_ahead_flags,
-					  &newinfo, &child->dtree->cookie, child->udev_flags)) {
+					  &newinfo, &child->dtree->cookie, child->udev_flags, child->info.suspended)) {
 				log_error("Unable to resume %s (%" PRIu32
 					  ":%" PRIu32 ")", child->name, child->info.major,
 					  child->info.minor);
@@ -1919,7 +1924,8 @@ int dm_tree_preload_children(struct dm_tree_node *dnode,
 
 		if (!_resume_node(child->name, child->info.major, child->info.minor,
 				  child->props.read_ahead, child->props.read_ahead_flags,
-				  &newinfo, &child->dtree->cookie, child->udev_flags)) {
+				  &newinfo, &child->dtree->cookie, child->udev_flags,
+				  child->info.suspended)) {
 			log_error("Unable to resume %s (%" PRIu32
 				  ":%" PRIu32 ")", child->name, child->info.major,
 				  child->info.minor);