Manage without dm_udev_cleanup?

lib/activate/dev_manager.c

@@ -1114,6 +1114,7 @@ static int _clean_tree(struct dev_manager *dm, struct dm_tree_node *root)
 	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)))
@@ -1132,12 +1133,12 @@ static int _clean_tree(struct dev_manager *dm, struct dm_tree_node *root)
 			continue;
 
 		dm_tree_set_cookie(root, 0);
-		if (!dm_tree_deactivate_children(root, uuid, strlen(uuid))) {
+		r = dm_tree_deactivate_children(root, uuid, strlen(uuid));
-			(void) dm_udev_cleanup(dm_tree_get_cookie(root));
-			return_0;
-		}
 		if (!dm_udev_wait(dm_tree_get_cookie(root)))
 			stack;
+
+		if (!r)
+			return_0;
 	}
 
 	return 1;
@@ -1171,12 +1172,11 @@ static int _tree_action(struct dev_manager *dm, struct logical_volume *lv, actio
 	case DEACTIVATE:
  		/* Deactivate LV and all devices it references that nothing else has open. */
 		dm_tree_set_cookie(root, 0);
-		if (!dm_tree_deactivate_children(root, dlid, ID_LEN + sizeof(UUID_PREFIX) - 1)) {
+		r = dm_tree_deactivate_children(root, dlid, ID_LEN + sizeof(UUID_PREFIX) - 1);
-			(void) dm_udev_cleanup(dm_tree_get_cookie(root));
-			goto_out;
-		}
 		if (!dm_udev_wait(dm_tree_get_cookie(root)))
 			stack;
+		if (!r)
+			goto_out;
 		if (!_remove_lv_symlinks(dm, root))
 			log_error("Failed to remove all device symlinks associated with %s.", lv->name);
 		break;
@@ -1196,24 +1196,22 @@ static int _tree_action(struct dev_manager *dm, struct logical_volume *lv, actio
 
 		/* Preload any devices required before any suspensions */
 		dm_tree_set_cookie(root, 0);
-		if (!dm_tree_preload_children(root, dlid, ID_LEN + sizeof(UUID_PREFIX) - 1)) {
+		r = dm_tree_preload_children(root, dlid, ID_LEN + sizeof(UUID_PREFIX) - 1);
-			(void) dm_udev_cleanup(dm_tree_get_cookie(root));
-			goto_out;
-		}
 		if (!dm_udev_wait(dm_tree_get_cookie(root)))
 			stack;
+		if (!r)
+			goto_out;
 
 		if (dm_tree_node_size_changed(root))
 			dm->flush_required = 1;
 
 		if (action == ACTIVATE) {
 			dm_tree_set_cookie(root, 0);
-			if (!dm_tree_activate_children(root, dlid, ID_LEN + sizeof(UUID_PREFIX) - 1)) {
+			r = dm_tree_activate_children(root, dlid, ID_LEN + sizeof(UUID_PREFIX) - 1);
-				(void) dm_udev_cleanup(dm_tree_get_cookie(root));
-				goto_out;
-			}
 			if (!dm_udev_wait(dm_tree_get_cookie(root)))
 				stack;
+			if (!r)
+				goto_out;
 		}
 
 		if (!_create_lv_symlinks(dm, root)) {

libdm/.exported_symbols

@@ -161,4 +161,3 @@ dm_udev_set_sync_support
 dm_udev_get_sync_support
 dm_udev_complete
 dm_udev_wait
-dm_udev_cleanup

libdm/ioctl/libdm-iface.c

@@ -1742,9 +1742,18 @@ int dm_task_run(struct dm_task *dmt)
 	if (!_open_control())
 		return 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)))
+	if (!(dmi = _do_dm_ioctl(dmt, command, _ioctl_buffer_double_factor))) {
+		/*
+		 * If an operation that uses a cookie fails, decrement the 
+		 * semaphore instead of udev.
+		 * FIXME Review error paths: found one where uevent fired too.
+		 */
+		if (dmt->cookie_set)
+			dm_udev_complete(dmt->event_nr);
 		return 0;
+	}
 
 	if (dmi->flags & DM_BUFFER_FULL_FLAG) {
 		switch (dmt->type) {

libdm/ioctl/libdm-targets.h

@@ -60,6 +60,7 @@ struct dm_task {
 	int skip_lockfs;
 	int suppress_identical_reload;
 	uint64_t existing_table_size;
+	int cookie_set;
 
 	char *uuid;
 };

libdm/libdevmapper.h

@@ -1023,6 +1023,5 @@ void dm_udev_set_sync_support(int sync_with_udev);
 int dm_udev_get_sync_support(void);
 int dm_udev_complete(uint32_t cookie);
 int dm_udev_wait(uint32_t cookie);
-int dm_udev_cleanup(uint32_t cookie);
 
 #endif				/* LIB_DEVICE_MAPPER_H */

libdm/libdm-common.c

@@ -174,6 +174,7 @@ struct dm_task *dm_task_create(int type)
 	dmt->no_open_count = 0;
 	dmt->read_ahead = DM_READ_AHEAD_AUTO;
 	dmt->read_ahead_flags = 0;
+	dmt->cookie_set = 0;
 
 	return dmt;
 }
@@ -799,11 +800,6 @@ int dm_udev_wait(uint32_t cookie)
 	return 1;
 }
 
-int dm_udev_cleanup(uint32_t cookie)
-{
-	return 1;
-}
-
 #else		/* UDEV_SYNC_SUPPORT */
 
 void dm_udev_set_sync_support(int sync_with_udev)
@@ -821,7 +817,8 @@ static int _get_cookie_sem(uint32_t cookie, int *semid)
 	if (!(cookie >> 16 & COOKIE_MAGIC)) {
 		log_error("Could not continue to access notification "
 			  "semaphore identified by cookie value %"
-			  PRIu32 " (0x%x). Incorrect cookie prefix.");
+			  PRIu32 " (0x%x). Incorrect cookie prefix.",
+			  cookie, cookie);
 		return 0;
 	}
 
@@ -952,7 +949,7 @@ static int _udev_notify_sem_create(uint32_t *cookie, int *semid)
 		log_error("semid %d: semctl failed: %s", gen_semid, strerror(errno));
 		/* We have to destroy just created semaphore
 		 * so it won't stay in the system. */
-		_udev_notify_sem_destroy(gen_semid, gen_cookie);
+		(void) _udev_notify_sem_destroy(gen_semid, gen_cookie);
 		goto bad;
 	}
 
@@ -996,6 +993,7 @@ int dm_task_set_cookie(struct dm_task *dmt, uint32_t *cookie)
 	}
 
 	dmt->event_nr = *cookie;
+	dmt->cookie_set = 1;
 	return 1;
 
 bad:
@@ -1039,7 +1037,7 @@ int dm_udev_wait(uint32_t cookie)
 			  "semaphore identified by cookie value %" PRIu32 " (0x%x) "
 			  "to initialize waiting for incoming notifications.",
 			  cookie, cookie);
-		_udev_notify_sem_destroy(semid, cookie);
+		(void) _udev_notify_sem_destroy(semid, cookie);
 		return 0;
 	}
 
@@ -1050,23 +1048,11 @@ repeat_wait:
 		log_error("Could not set wait state for notification semaphore "
 			  "identified by cookie value %" PRIu32 " (0x%x): %s",
 			  cookie, cookie, strerror(errno));
-		_udev_notify_sem_destroy(semid, cookie);
+		(void) _udev_notify_sem_destroy(semid, cookie);
 		return 0;
 	}
 
 	return _udev_notify_sem_destroy(semid, cookie);
 }
 
-int dm_udev_cleanup(uint32_t cookie)
-{
-	int semid;
-
-	if (!cookie || !dm_udev_get_sync_support() || !dm_cookie_supported())
-		return 1;
-
-	if (!_get_cookie_sem(cookie, &semid))
-		return_0;
-
-	return _udev_notify_sem_destroy(semid, cookie);
-}
 #endif		/* UDEV_SYNC_SUPPORT */

libdm/libdm-deptree.c

@@ -845,9 +845,6 @@ static int _deactivate_node(const char *name, uint32_t major, uint32_t minor, ui
 
 	r = dm_task_run(dmt);
 
-	if (!r)
-		(void) dm_udev_complete(*cookie);
-
 	/* FIXME Until kernel returns actual name so dm-ioctl.c can handle it */
 	rm_dev_node(name);
 
@@ -888,9 +885,6 @@ static int _rename_node(const char *old_name, const char *new_name, uint32_t maj
 
 	r = dm_task_run(dmt);
 
-	if (!r)
-		(void) dm_udev_complete(*cookie);
-
 out:
 	dm_task_destroy(dmt);
 
@@ -934,8 +928,6 @@ static int _resume_node(const char *name, uint32_t major, uint32_t minor,
 
 	if ((r = dm_task_run(dmt)))
 		r = dm_task_get_info(dmt, newinfo);
-	else
-		(void) dm_udev_complete(*cookie);
 
 out:
 	dm_task_destroy(dmt);

tools/dmsetup.c

@@ -588,12 +588,8 @@ static int _create(int argc, char **argv, void *data __attribute((unused)))
 		dm_udev_set_sync_support(0);
 
 	if (!dm_task_set_cookie(dmt, &cookie) ||
-	    !dm_task_run(dmt)) {
+	    !dm_task_run(dmt))
-		(void) dm_udev_cleanup(cookie);
 		goto out;
-	}
-
-	(void) dm_udev_wait(cookie);
 
 	r = 1;
 
@@ -601,6 +597,7 @@ static int _create(int argc, char **argv, void *data __attribute((unused)))
 		r = _display_info(dmt);
 
       out:
+	(void) dm_udev_wait(cookie);
 	dm_task_destroy(dmt);
 
 	return r;
@@ -626,16 +623,13 @@ static int _rename(int argc, char **argv, void *data __attribute((unused)))
 		goto out;
 
 	if (!dm_task_set_cookie(dmt, &cookie) ||
-	    !dm_task_run(dmt)) {
+	    !dm_task_run(dmt))
-		(void) dm_udev_cleanup(cookie);
 		goto out;
-	}
-
-	(void) dm_udev_wait(cookie);
 
 	r = 1;
 
       out:
+	(void) dm_udev_wait(cookie);
 	dm_task_destroy(dmt);
 
 	return r;
@@ -816,24 +810,18 @@ static int _simple(int task, const char *name, uint32_t event_nr, int display)
 				    _read_ahead_flags))
 		goto out;
 
-	if (udev_wait_flag && !dm_task_set_cookie(dmt, &cookie)) {
+	if (udev_wait_flag && !dm_task_set_cookie(dmt, &cookie))
-		(void) dm_udev_cleanup(cookie);
 		goto out;
-	}
 
 	r = dm_task_run(dmt);
 
-	if (udev_wait_flag) {
-		if (r)
-			(void) dm_udev_wait(cookie);
-		else
-			(void) dm_udev_cleanup(cookie);
-	}
-
 	if (r && display && _switches[VERBOSE_ARG])
 		r = _display_info(dmt);
 
       out:
+	if (udev_wait_flag)
+		(void) dm_udev_wait(cookie);
+
 	dm_task_destroy(dmt);
 	return r;
 }