From: NeilBrown <neilb@suse.de>
Date: Mon, 24 Sep 2012 02:26:03 +0000 (+1000)
Subject: Manage: fix checks for removal from a container.
X-Git-Tag: mdadm-3.3-rc1~219
X-Git-Url: http://git.ipfire.org/?p=thirdparty%2Fmdadm.git;a=commitdiff_plain;h=aab15415eddbfa800031cb9f6273ca8fcf59efb7

Manage: fix checks for removal from a container.

We must only remove from a container if the device isn't a
member of any member array.
To check we look at the 'holders' directory in sysfs.

We currently skip that check if ->devname is "detached", however
that can never be true since the change that introduced
add_detached().

Also sysfs_unique_holder returns status in 'errno' which isn't
entirely safe as e.g. closedir() is probably allowed to clear it.

So make sysfs_unique_holder return an unambigious value, and us
it to decide what to report.

Signed-off-by: NeilBrown <neilb@suse.de>
---

diff --git a/Manage.c b/Manage.c
index 7f27f741..9e622531 100644
--- a/Manage.c
+++ b/Manage.c
@@ -857,6 +857,7 @@ int Manage_remove(struct supertype *tst, int fd, struct mddev_dev *dv,
 		 * hot spare while we are checking, we
 		 * get an O_EXCL open on the container
 		 */
+		int ret;
 		int dnum = fd2devnum(fd);
 		lfd = open_dev_excl(dnum);
 		if (lfd < 0) {
@@ -864,19 +865,26 @@ int Manage_remove(struct supertype *tst, int fd, struct mddev_dev *dv,
 			       " to container - odd\n");
 			return -1;
 		}
-		/* In the detached case it is not possible to
-		 * check if we are the unique holder, so just
-		 * rely on the 'detached' checks
+		/* We may not be able to check on holders in
+		 * sysfs, either because we don't have the dev num
+		 * (rdev == 0) or because the device has been detached
+		 * and the 'holders' directory no longer exists
+		 * (ret == -1).  In that case, assume it is OK to
+		 * remove.
 		 */
-		if (strcmp(dv->devname, "detached") == 0 ||
-		    sysfd >= 0 ||
-		    sysfs_unique_holder(dnum, rdev))
-			/* pass */;
-		else {
-			pr_err("%s is %s, cannot remove.\n",
-			       dv->devname,
-			       errno == EEXIST ? "still in use":
-			       "not a member");
+		if (rdev == 0)
+			ret = -1;
+		else
+			ret = sysfs_unique_holder(dnum, rdev);
+		if (ret == 0) {
+			pr_err("%s is not a member, cannot remove.\n",
+			       dv->devname);
+			close(lfd);
+			return -1;
+		}
+		if (ret >= 2) {
+			pr_err("%s is still in use, cannot remove.\n",
+			       dv->devname);
 			close(lfd);
 			return -1;
 		}
@@ -1030,7 +1038,7 @@ int Manage_subdevs(char *devname, int fd,
 			struct mddev_dev **dp;
 			if (dv->disposition != 'A') {
 				pr_err("'missing' only meaningful "
-					"with --re-add\n");
+				       "with --re-add\n");
 				goto abort;
 			}
 			add_devlist = conf_get_devs();
@@ -1047,8 +1055,8 @@ int Manage_subdevs(char *devname, int fd,
 		}
 
 		if (strchr(dv->devname, '/') == NULL &&
-			   strchr(dv->devname, ':') == NULL &&
-			   strlen(dv->devname) < 50) {
+		    strchr(dv->devname, ':') == NULL &&
+		    strlen(dv->devname) < 50) {
 			/* Assume this is a kernel-internal name like 'sda1' */
 			int found = 0;
 			char dname[55];
diff --git a/sysfs.c b/sysfs.c
index 0043d272..47294556 100644
--- a/sysfs.c
+++ b/sysfs.c
@@ -778,18 +778,22 @@ int sysfs_unique_holder(int devnum, long rdev)
 	 * and is the only holder.
 	 * we should be locked against races by
 	 * an O_EXCL on devnum
+	 * Return values:
+	 *  0 - not unique, not even a holder
+	 *  1 - unique, this is the only holder.
+	 *  2/3 - not unique, there is another holder
+	 * -1 - error, cannot find the holders
 	 */
 	DIR *dir;
 	struct dirent *de;
 	char dirname[100];
 	char l;
-	int found = 0;
+	int ret = 0;
 	sprintf(dirname, "/sys/dev/block/%d:%d/holders",
 		major(rdev), minor(rdev));
 	dir = opendir(dirname);
-	errno = ENOENT;
 	if (!dir)
-		return 0;
+		return -1;
 	l = strlen(dirname);
 	while ((de = readdir(dir)) != NULL) {
 		char buf[10];
@@ -807,8 +811,8 @@ int sysfs_unique_holder(int devnum, long rdev)
 		strcat(dirname+l, "/dev");
 		fd = open(dirname, O_RDONLY);
 		if (fd < 0) {
-			errno = ENOENT;
-			break;
+			/* Probably a race, just ignore this */
+			continue;
 		}
 		n = read(fd, buf, sizeof(buf)-1);
 		close(fd);
@@ -816,24 +820,18 @@ int sysfs_unique_holder(int devnum, long rdev)
 			continue;
 		buf[n] = 0;
 		if (sscanf(buf, "%d:%d%c", &mj, &mn, &c) != 3 ||
-		    c != '\n') {
-			errno = ENOENT;
-			break;
-		}
+		    c != '\n')
+			continue;
 		if (mj != MD_MAJOR)
 			mn = -1-(mn>>6);
 
-		if (devnum != mn) {
-			errno = EEXIST;
-			break;
-		}
-		found = 1;
+		if (devnum == mn)
+			ret |= 1;
+		else
+			ret |= 2;
 	}
 	closedir(dir);
-	if (de)
-		return 0;
-	else
-		return found;
+	return ret;
 }
 
 int sysfs_freeze_array(struct mdinfo *sra)