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 7f27f7413691cef4473efd0b36518513e849fc79..9e6225315a139300a32e86c82d1554f85b2a0452 100644 (file)
--- 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 0043d27256aeed292f10f3d40e84cb4b1eb0d534..47294556eef5957047e2aac076408fd3d9f57091 100644 (file)
--- 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)