Add dm_uuid_prefix/dm_set_uuid_prefix for non-lvm users to override hard-coded

LVM- prefix.

Try harder not to leave stray empty devices around (locally or remotely) when
reverting changes after failures while there are inactive tables.

WHATS_NEW_DM

@@ -1,11 +1,13 @@
 Version 1.02.68 -
 ==================================
+  Remove empty devices when clearing left-over inactive tables in deptree.
+  Add dm_uuid_prefix/dm_set_uuid_prefix to override hard-coded LVM- prefix.
   Improve dmsetup man page about readahead parameter.
   Use sysfs to set/get of read-ahead setting if possible.
   Fix lvm2-monitor init script to use normalized output when using vgs.
   Add test for max length (DM_MAX_TYPE_NAME) of target type name.
   Include a copy of kernel DM documentation in doc/kernel.
-  Improve man page style for dmsetup.
+  Improve man page style for dmsetup and mention more targets.
   Fix _get_proc_number to be tolerant of malformed /proc/misc entries.
   Add ExecReload to dm-event.service for systemd to reload dmeventd properly.
   Add dm_config_tree_find_str_allow_empty and dm_config_find_str_allow_empty.

lib/commands/toolcontext.c

@@ -246,6 +246,9 @@ static int _process_config(struct cmd_context *cmd)
 	}
 #ifdef DEVMAPPER_SUPPORT
 	dm_set_dev_dir(cmd->dev_dir);
+
+	if (!dm_set_uuid_prefix("LVM-"))
+		return_0;
 #endif
 
 	/* proc dir */

lib/locking/locking.c

@@ -515,7 +515,6 @@ int revert_lvs(struct cmd_context *cmd, struct dm_list *lvs)
 int suspend_lvs(struct cmd_context *cmd, struct dm_list *lvs,
 		struct volume_group *vg_to_revert)
 {
-	struct dm_list *lvh;
 	struct lv_list *lvl;
 
 	dm_list_iterate_items(lvl, lvs) {
@@ -523,11 +522,15 @@ int suspend_lvs(struct cmd_context *cmd, struct dm_list *lvs,
 			log_error("Failed to suspend %s", lvl->lv->name);
 			if (vg_to_revert)
 				vg_revert(vg_to_revert);
-			dm_list_uniterate(lvh, lvs, &lvl->list) {
+			/*
-				lvl = dm_list_item(lvh, struct lv_list);
+			 * FIXME Should be
+			 * 	dm_list_uniterate(lvh, lvs, &lvl->list) {
+			 *	lvl = dm_list_item(lvh, struct lv_list);
+			 * but revert would need fixing to use identical tree deps first.
+			 */
+			dm_list_iterate_items(lvl, lvs)
 				if (!revert_lv(cmd, lvl->lv))
 					stack;
-			}
 
 			return 0;
 		}

libdm/libdevmapper.h

@@ -284,6 +284,16 @@ const char *dm_dir(void);
 int dm_set_sysfs_dir(const char *dir);
 const char *dm_sysfs_dir(void);
 
+/*
+ * Configure default UUID prefix string.
+ * Conventionally this is a short capitalised prefix indicating the subsystem
+ * that is managing the devices, e.g. "LVM-" or "MPATH-".
+ * To support stacks of devices from different subsystems, recursive functions
+ * stop recursing if they reach a device with a different prefix.
+ */
+int dm_set_uuid_prefix(const char *uuid_prefix);
+const char *dm_uuid_prefix(void);
+
 /*
  * Determine whether a major number belongs to device-mapper or not.
  */

libdm/libdm-common.c

@@ -61,6 +61,9 @@ static char _dm_dir[PATH_MAX] = DEV_DIR DM_DIR;
 static char _sysfs_dir[PATH_MAX] = "/sys/";
 static char _path0[PATH_MAX];           /* path buffer, safe 4kB on stack */
 
+#define DM_MAX_UUID_PREFIX_LEN	15
+static char _default_uuid_prefix[DM_MAX_UUID_PREFIX_LEN + 1] = "LVM-";
+
 static int _verbose = 0;
 static int _suspended_dev_counter = 0;
 
@@ -1139,6 +1142,29 @@ const char *dm_sysfs_dir(void)
 	return _sysfs_dir;
 }
 
+/*
+ * Replace existing uuid_prefix provided it isn't too long.
+ */
+int dm_set_uuid_prefix(const char *uuid_prefix)
+{
+	if (!uuid_prefix)
+		return_0;
+
+	if (strlen(uuid_prefix) > DM_MAX_UUID_PREFIX_LEN) {
+		log_error("New uuid prefix %s too long.", uuid_prefix);
+		return 0;
+	}
+
+	strcpy(_default_uuid_prefix, uuid_prefix);
+
+	return 1;
+}
+
+const char *dm_uuid_prefix(void)
+{
+	return _default_uuid_prefix;
+}
+
 int dm_device_has_holders(uint32_t major, uint32_t minor)
 {
 	char sysfs_path[PATH_MAX];

libdm/libdm-deptree.c

@@ -24,9 +24,6 @@
 
 #define MAX_TARGET_PARAMSIZE 500000
 
-/* FIXME Fix interface so this is used only by LVM */
-#define UUID_PREFIX "LVM-"
-
 #define REPLICATOR_LOCAL_SITE 0
 
 /* Supported segment types */
@@ -488,25 +485,32 @@ static struct dm_tree_node *_find_dm_tree_node_by_uuid(struct dm_tree *dtree,
 						       const char *uuid)
 {
 	struct dm_tree_node *node;
+	const char *default_uuid_prefix;
+	size_t default_uuid_prefix_len;
 
 	if ((node = dm_hash_lookup(dtree->uuids, uuid)))
 		return node;
 
-	if (strncmp(uuid, UUID_PREFIX, sizeof(UUID_PREFIX) - 1))
+	default_uuid_prefix = dm_uuid_prefix();
+	default_uuid_prefix_len = strlen(default_uuid_prefix);
+
+	if (strncmp(uuid, default_uuid_prefix, default_uuid_prefix_len))
 		return NULL;
 
-	return dm_hash_lookup(dtree->uuids, uuid + sizeof(UUID_PREFIX) - 1);
+	return dm_hash_lookup(dtree->uuids, uuid + default_uuid_prefix_len);
 }
 
 static int _deps(struct dm_task **dmt, struct dm_pool *mem, uint32_t major, uint32_t minor,
-		 const char **name, const char **uuid,
+		 const char **name, const char **uuid, unsigned inactive_table,
 		 struct dm_info *info, struct dm_deps **deps)
 {
 	memset(info, 0, sizeof(*info));
 
 	if (!dm_is_dm_major(major)) {
-		*name = "";
+		if (name)
-		*uuid = "";
+			*name = "";
+		if (uuid)
+			*uuid = "";
 		*deps = NULL;
 		info->major = major;
 		info->minor = minor;
@@ -534,6 +538,12 @@ static int _deps(struct dm_task **dmt, struct dm_pool *mem, uint32_t major, uint
 		goto failed;
 	}
 
+	if (inactive_table && !dm_task_query_inactive_table(*dmt)) {
+		log_error("_deps: failed to set inactive table for (%" PRIu32 ":%" PRIu32 ")",
+			  major, minor);
+		goto failed;
+	}
+
 	if (!dm_task_run(*dmt)) {
 		log_error("_deps: task run failed for (%" PRIu32 ":%" PRIu32 ")",
 			  major, minor);
@@ -547,8 +557,10 @@ static int _deps(struct dm_task **dmt, struct dm_pool *mem, uint32_t major, uint
 	}
 
 	if (!info->exists) {
-		*name = "";
+		if (name)
-		*uuid = "";
+			*name = "";
+		if (uuid)
+			*uuid = "";
 		*deps = NULL;
 	} else {
 		if (info->major != major) {
@@ -561,11 +573,11 @@ static int _deps(struct dm_task **dmt, struct dm_pool *mem, uint32_t major, uint
 				  minor, info->minor);
 			goto failed;
 		}
-		if (!(*name = dm_pool_strdup(mem, dm_task_get_name(*dmt)))) {
+		if (name && !(*name = dm_pool_strdup(mem, dm_task_get_name(*dmt)))) {
 			log_error("name pool_strdup failed");
 			goto failed;
 		}
-		if (!(*uuid = dm_pool_strdup(mem, dm_task_get_uuid(*dmt)))) {
+		if (uuid && !(*uuid = dm_pool_strdup(mem, dm_task_get_uuid(*dmt)))) {
 			log_error("uuid pool_strdup failed");
 			goto failed;
 		}
@@ -595,7 +607,7 @@ static struct dm_tree_node *_add_dev(struct dm_tree *dtree,
 
 	/* Already in tree? */
 	if (!(node = _find_dm_tree_node(dtree, major, minor))) {
-		if (!_deps(&dmt, dtree->mem, major, minor, &name, &uuid, &info, &deps))
+		if (!_deps(&dmt, dtree->mem, major, minor, &name, &uuid, 0, &info, &deps))
 			return_NULL;
 
 		if (!(node = _create_dm_tree_node(dtree, name, uuid, &info,
@@ -637,12 +649,22 @@ out:
 	return node;
 }
 
-static int _node_clear_table(struct dm_tree_node *dnode)
+// FIXME Move fn group down.
+static int _info_by_dev(uint32_t major, uint32_t minor, int with_open_count,
+			struct dm_info *info, struct dm_pool *mem,
+			const char **name, const char **uuid);
+static int _deactivate_node(const char *name, uint32_t major, uint32_t minor,
+			    uint32_t *cookie, uint16_t udev_flags, int retry);
+static int _node_clear_table(struct dm_tree_node *dnode, uint16_t udev_flags)
 {
-	struct dm_task *dmt;
+	struct dm_task *dmt = NULL, *deps_dmt = NULL;
-	struct dm_info *info;
+	struct dm_info *info, deps_info;
-	const char *name;
+	struct dm_deps *deps = NULL;
-	int r;
+	const char *name, *uuid;
+	const char *default_uuid_prefix;
+	size_t default_uuid_prefix_len;
+	uint32_t i;
+	int r = 0;
 
 	if (!(info = &dnode->info)) {
 		log_error("_node_clear_table failed: missing info");
@@ -658,21 +680,24 @@ static int _node_clear_table(struct dm_tree_node *dnode)
 	if (!info->exists || !info->inactive_table)
 		return 1;
 
-// FIXME Get inactive deps.  If any dev referenced has 1 opener and no live table, remove it after the clear.
+	/* Get devices used by inactive table that's about to be deleted. */
+	if (!_deps(&deps_dmt, dnode->dtree->mem, info->major, info->minor, NULL, NULL, 1, info, &deps)) {
+		log_error("Failed to obtain dependencies for %s before clearing table.", name);
+		return 0;
+	}
 
 	log_verbose("Clearing inactive table %s (%" PRIu32 ":%" PRIu32 ")",
 		    name, info->major, info->minor);
 
 	if (!(dmt = dm_task_create(DM_DEVICE_CLEAR))) {
 		log_error("Table clear dm_task creation failed for %s", name);
-		return 0;
+		goto_out;
 	}
 
 	if (!dm_task_set_major(dmt, info->major) ||
 	    !dm_task_set_minor(dmt, info->minor)) {
 		log_error("Failed to set device number for %s table clear", name);
-		dm_task_destroy(dmt);
+		goto_out;
-		return 0;
 	}
 
 	r = dm_task_run(dmt);
@@ -682,18 +707,60 @@ static int _node_clear_table(struct dm_tree_node *dnode)
 		r = 0;
 	}
 
-	dm_task_destroy(dmt);
+	if (!r || !deps)
+		goto_out;
+
+	/*
+	 * Remove (incomplete) devices that the inactive table referred to but
+	 * which are not in the tree, no longer referenced and don't have a live
+	 * table.
+	 */
+	default_uuid_prefix = dm_uuid_prefix();
+	default_uuid_prefix_len = strlen(default_uuid_prefix);
+
+	for (i = 0; i < deps->count; i++) {
+		/* If already in tree, assume it's under control */
+		if (_find_dm_tree_node(dnode->dtree, MAJOR(deps->device[i]), MINOR(deps->device[i])))
+			continue;
+
+		if (!_info_by_dev(MAJOR(deps->device[i]), MINOR(deps->device[i]), 1,
+				  &deps_info, dnode->dtree->mem, &name, &uuid))
+			continue;
+
+		/* Proceed if device is an 'orphan' - unreferenced and without a live table. */
+		if (!deps_info.exists || deps_info.live_table || deps_info.open_count)
+			continue;
+
+		if (strncmp(uuid, default_uuid_prefix, default_uuid_prefix_len))
+			continue;
+
+		/* Remove device. */
+		if (!_deactivate_node(name, deps_info.major, deps_info.minor, &dnode->dtree->cookie, udev_flags, 0)) {
+			log_error("Failed to deactivate no-longer-used device %s (%"
+				  PRIu32 ":%" PRIu32 ")", name, deps_info.major, deps_info.minor);
+		} else if (deps_info.suspended)
+			dec_suspended();
+	}
+
+out:
+	if (dmt)
+		dm_task_destroy(dmt);
+
+	if (deps_dmt)
+		dm_task_destroy(deps_dmt);
 
 	return r;
 }
 
-struct dm_tree_node *dm_tree_add_new_dev(struct dm_tree *dtree,
+struct dm_tree_node *dm_tree_add_new_dev_with_udev_flags(struct dm_tree *dtree,
-					    const char *name,
+							 const char *name,
-					    const char *uuid,
+							 const char *uuid,
-					    uint32_t major, uint32_t minor,
+							 uint32_t major,
-					    int read_only,
+							 uint32_t minor,
-					    int clear_inactive,
+							 int read_only,
-					    void *context)
+							 int clear_inactive,
+							 void *context,
+							 uint16_t udev_flags)
 {
 	struct dm_tree_node *dnode;
 	struct dm_info info;
@@ -734,7 +801,7 @@ struct dm_tree_node *dm_tree_add_new_dev(struct dm_tree *dtree,
 		/* Do we need to rename node? */
 		if (!(dnode->props.new_name = dm_pool_strdup(dtree->mem, name))) {
 			log_error("name pool_strdup failed");
-			return 0;
+			return NULL;
 		}
 	}
 
@@ -742,32 +809,21 @@ struct dm_tree_node *dm_tree_add_new_dev(struct dm_tree *dtree,
 	dnode->props.read_ahead = DM_READ_AHEAD_AUTO;
 	dnode->props.read_ahead_flags = 0;
 
-	if (clear_inactive && !_node_clear_table(dnode))
+	if (clear_inactive && !_node_clear_table(dnode, udev_flags))
 		return_NULL;
 
 	dnode->context = context;
-	dnode->udev_flags = 0;
+	dnode->udev_flags = udev_flags;
 
 	return dnode;
 }
 
-struct dm_tree_node *dm_tree_add_new_dev_with_udev_flags(struct dm_tree *dtree,
+struct dm_tree_node *dm_tree_add_new_dev(struct dm_tree *dtree, const char *name,
-							 const char *name,
+					 const char *uuid, uint32_t major, uint32_t minor,
-							 const char *uuid,
+					 int read_only, int clear_inactive, void *context)
-							 uint32_t major,
-							 uint32_t minor,
-							 int read_only,
-							 int clear_inactive,
-							 void *context,
-							 uint16_t udev_flags)
 {
-	struct dm_tree_node *node;
+	return dm_tree_add_new_dev_with_udev_flags(dtree, name, uuid, major, minor,
-
+						   read_only, clear_inactive, context, 0);
-	if ((node = dm_tree_add_new_dev(dtree, name, uuid, major, minor, read_only,
-				       clear_inactive, context)))
-		node->udev_flags = udev_flags;
-
-	return node;
 }
 
 void dm_tree_node_set_udev_flags(struct dm_tree_node *dnode, uint16_t udev_flags)
@@ -852,6 +908,9 @@ int dm_tree_node_num_children(const struct dm_tree_node *node, uint32_t inverted
  */
 static int _uuid_prefix_matches(const char *uuid, const char *uuid_prefix, size_t uuid_prefix_len)
 {
+	const char *default_uuid_prefix = dm_uuid_prefix();
+	size_t default_uuid_prefix_len = strlen(default_uuid_prefix);
+
 	if (!uuid_prefix)
 		return 1;
 
@@ -862,13 +921,13 @@ static int _uuid_prefix_matches(const char *uuid, const char *uuid_prefix, size_
 	if (uuid_prefix_len <= 4)
 		return 0;
 
-	if (!strncmp(uuid, UUID_PREFIX, sizeof(UUID_PREFIX) - 1))
+	if (!strncmp(uuid, default_uuid_prefix, default_uuid_prefix_len))
 		return 0;
 
-	if (strncmp(uuid_prefix, UUID_PREFIX, sizeof(UUID_PREFIX) - 1))
+	if (strncmp(uuid_prefix, default_uuid_prefix, default_uuid_prefix_len))
 		return 0;
 
-	if (!strncmp(uuid, uuid_prefix + sizeof(UUID_PREFIX) - 1, uuid_prefix_len - (sizeof(UUID_PREFIX) - 1)))
+	if (!strncmp(uuid, uuid_prefix + default_uuid_prefix_len, uuid_prefix_len - default_uuid_prefix_len))
 		return 1;
 
 	return 0;
@@ -976,7 +1035,8 @@ struct dm_tree_node *dm_tree_next_child(void **handle,
  * Deactivate a device with its dependencies if the uuid prefix matches.
  */
 static int _info_by_dev(uint32_t major, uint32_t minor, int with_open_count,
-			struct dm_info *info)
+			struct dm_info *info, struct dm_pool *mem,
+			const char **name, const char **uuid)
 {
 	struct dm_task *dmt;
 	int r;
@@ -995,9 +1055,25 @@ static int _info_by_dev(uint32_t major, uint32_t minor, int with_open_count,
 	if (!with_open_count && !dm_task_no_open_count(dmt))
 		log_error("Failed to disable open_count");
 
-	if ((r = dm_task_run(dmt)))
+	if (!(r = dm_task_run(dmt)))
-		r = dm_task_get_info(dmt, info);
+		goto_out;
 
+	if (!(r = dm_task_get_info(dmt, info)))
+		goto_out;
+
+	if (name && !(*name = dm_pool_strdup(mem, dm_task_get_name(dmt)))) {
+		log_error("name pool_strdup failed");
+		r = 0;
+		goto_out;
+	}
+
+	if (uuid && !(*uuid = dm_pool_strdup(mem, dm_task_get_uuid(dmt)))) {
+		log_error("uuid pool_strdup failed");
+		r = 0;
+		goto_out;
+	}
+
+out:
 	dm_task_destroy(dmt);
 
 	return r;
@@ -1061,7 +1137,7 @@ static int _node_has_closed_parents(struct dm_tree_node *node,
 		}
 
 		/* Refresh open_count */
-		if (!_info_by_dev(dinfo->major, dinfo->minor, 1, &info) ||
+		if (!_info_by_dev(dinfo->major, dinfo->minor, 1, &info, NULL, NULL, NULL) ||
 		    !info.exists)
 			continue;
 
@@ -1096,9 +1172,9 @@ static int _deactivate_node(const char *name, uint32_t major, uint32_t minor,
 	if (!dm_task_no_open_count(dmt))
 		log_error("Failed to disable open_count");
 
-	if (!dm_task_set_cookie(dmt, cookie, udev_flags))
+	if (cookie)
-		goto out;
+		if (!dm_task_set_cookie(dmt, cookie, udev_flags))
-
+			goto out;
 
 	if (retry)
 		dm_task_retry_remove(dmt);
@@ -1107,7 +1183,7 @@ static int _deactivate_node(const char *name, uint32_t major, uint32_t minor,
 
 	/* FIXME Until kernel returns actual name so dm-iface.c can handle it */
 	rm_dev_node(name, dmt->cookie_set && !(udev_flags & DM_UDEV_DISABLE_DM_RULES_FLAG),
-			  dmt->cookie_set && (udev_flags & DM_UDEV_DISABLE_LIBRARY_FALLBACK));
+		    dmt->cookie_set && (udev_flags & DM_UDEV_DISABLE_LIBRARY_FALLBACK));
 
 	/* FIXME Remove node from tree or mark invalid? */
 
@@ -1440,7 +1516,7 @@ static int _dm_tree_deactivate_children(struct dm_tree_node *dnode,
 			continue;
 
 		/* Refresh open_count */
-		if (!_info_by_dev(dinfo->major, dinfo->minor, 1, &info) ||
+		if (!_info_by_dev(dinfo->major, dinfo->minor, 1, &info, NULL, NULL, NULL) ||
 		    !info.exists)
 			continue;
 
@@ -1564,7 +1640,7 @@ int dm_tree_suspend_children(struct dm_tree_node *dnode,
 		if (!_children_suspended(child, 1, uuid_prefix, uuid_prefix_len))
 			continue;
 
-		if (!_info_by_dev(dinfo->major, dinfo->minor, 0, &info) ||
+		if (!_info_by_dev(dinfo->major, dinfo->minor, 0, &info, NULL, NULL, NULL) ||
 		    !info.exists || info.suspended)
 			continue;