libmultipath: fix leaked device-mapper suspend after a failed remove

libmultipath/devmapper.c

@@ -446,6 +446,35 @@ int dm_simplecmd_noflush (int task, const char *name, uint16_t udev_flags)
 	return dm_simplecmd(task, name, DMFL_NO_FLUSH|DMFL_NEED_SYNC, udev_flags);
 }
 
+/*
+ * A failed resume can leave the map suspended without the ioctl reporting
+ * failure. Mirror dm_addmap_reload(): re-issue DM_DEVICE_RESUME and confirm
+ * with dm_is_suspended() that the map is really active, rather than trusting
+ * the ioctl return value alone. Retry once to absorb a transient hiccup.
+ */
+static bool dm_resume_and_verify(const char *mapname, uint16_t udev_flags)
+{
+	int i;
+
+	for (i = 0; i < 2; i++) {
+		int r = dm_simplecmd_noflush(DM_DEVICE_RESUME, mapname, udev_flags);
+		int suspended = dm_is_suspended(mapname);
+
+		if (suspended == 0)
+			return true;
+		/*
+		 * The map may have been removed by another process after the
+		 * failed remove was observed; that is not a leaked suspend.
+		 */
+		if (suspended < 0 && dm_is_mpath(mapname) == DM_IS_MPATH_NO)
+			return true;
+		condlog(r ? 2 : 1, "%s: resume attempt %d did not activate the map",
+			mapname, i + 1);
+	}
+	condlog(0, "%s: map remains suspended after resume attempts", mapname);
+	return false;
+}
+
 static int
 dm_device_remove (const char *name, int flags) {
 	return dm_simplecmd(DM_DEVICE_REMOVE, name, flags, 0);
@@ -1042,26 +1071,51 @@ int dm_flush_map__ (const char *mapname, int flags, int retries)
 			}
 			condlog(4, "multipath map %s removed", mapname);
 			return DM_FLUSH_OK;
-		} else if (dm_is_mpath(mapname) != DM_IS_MPATH_YES) {
-			condlog(4, "multipath map %s removed externally",
-				mapname);
-			return DM_FLUSH_OK; /* raced. someone else removed it */
 		} else {
-			condlog(2, "failed to remove multipath map %s",
-				mapname);
-			if ((flags & DMFL_SUSPEND) && queue_if_no_path != -1) {
-				dm_simplecmd_noflush(DM_DEVICE_RESUME,
-						     mapname, udev_flags);
+			int is_mpath = dm_is_mpath(mapname);
+
+			if (is_mpath == DM_IS_MPATH_NO) {
+				condlog(4, "multipath map %s removed externally",
+					mapname);
+				return DM_FLUSH_OK; /* raced. someone else removed it */
+			}
+			/*
+			 * A query error (DM_IS_MPATH_ERR) must not be treated
+			 * as "removed externally": under an ioctl storm the
+			 * status query can transiently fail, and returning
+			 * DM_FLUSH_OK here would leave the map suspended.
+			 */
+			if (is_mpath == DM_IS_MPATH_ERR)
+				condlog(1, "%s: unable to verify map state after failed remove",
+					mapname);
+			else
+				condlog(2, "failed to remove multipath map %s",
+					mapname);
+			if ((flags & DMFL_SUSPEND) && queue_if_no_path != -1 &&
+			    !dm_resume_and_verify(mapname, udev_flags)) {
+				r = DM_FLUSH_FAIL;
+				goto out;
+			}
+			/*
+			 * The map state could not be verified, so do not keep
+			 * retrying as if the map were still present; report the
+			 * failure so callers/monitoring can act.
+			 */
+			if (is_mpath == DM_IS_MPATH_ERR) {
+				r = DM_FLUSH_FAIL;
+				goto out;
 			}
 		}
 		if (retries)
 			sleep(1);
 	} while (retries-- > 0);
 
+	r = DM_FLUSH_FAIL;
+out:
 	if (queue_if_no_path == 1 && _dm_queue_if_no_path(mapname, 1) != 0)
 		return DM_FLUSH_FAIL_CANT_RESTORE;
 
-	return DM_FLUSH_FAIL;
+	return r;
 }
 
 int