Do not hardcode umount(8) path, emulate lazy umount

1) Use a system-dependent macro for umount(8) location instead of
relying on $PATH  to find it, for security and portability sake.

2) Introduce gf_umount_lazy() to replace umount -l (-l for lazy) invocations,
which is only supported on Linux; On Linux behavior in unchanged. On other
systems, we fork an external process (umountd) that will take care of
periodically attempt to unmount, and optionally rmdir.

BUG: 1129939
Change-Id: Ia91167c0652f8ddab85136324b08f87c5ac1e51d
Signed-off-by: Emmanuel Dreyfus <manu@netbsd.org>
Reviewed-on: http://review.gluster.org/8649
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Csaba Henk <csaba@redhat.com>
Reviewed-by: Vijay Bellur <vbellur@redhat.com>

5 files changed, 26 insertions, 74 deletions

diff --git a/xlators/mgmt/glusterd/src/glusterd-handler.c b/xlators/mgmt/glusterd/src/glusterd-handler.c
index 9ee26b52160..d97cc4bdf32 100644
--- a/xlators/mgmt/glusterd/src/glusterd-handler.c
+++ b/xlators/mgmt/glusterd/src/glusterd-handler.c
@@ -2824,12 +2824,16 @@ __glusterd_handle_umount (rpcsvc_request_t *req)
                 goto out;
         }
 
-        runinit (&runner);
-        runner_add_args (&runner, "umount", umnt_req.path, NULL);
-        if (umnt_req.lazy)
-                runner_add_arg (&runner, "-l");
         synclock_unlock (&priv->big_lock);
-        rsp.op_ret = runner_run (&runner);
+
+        if (umnt_req.lazy) {
+                rsp.op_ret = gf_umount_lazy (this->name, umnt_req.path, 0);
+        } else {
+                runinit (&runner);
+                runner_add_args (&runner, _PATH_UMOUNT, umnt_req.path, NULL);
+                rsp.op_ret = runner_run (&runner);
+        }
+
         synclock_lock (&priv->big_lock);
         if (rsp.op_ret == 0) {
                 if (realpath (umnt_req.path, mntp))
diff --git a/xlators/mgmt/glusterd/src/glusterd-quota.c b/xlators/mgmt/glusterd/src/glusterd-quota.c
index 7338e826ca9..f1e0417dbe5 100644
--- a/xlators/mgmt/glusterd/src/glusterd-quota.c
+++ b/xlators/mgmt/glusterd/src/glusterd-quota.c
@@ -256,11 +256,9 @@ glusterd_quota_initiate_fs_crawl (glusterd_conf_t *priv, char *volname,
 
 #ifndef GF_LINUX_HOST_OS
                 runner_end (&runner); /* blocks in waitpid */
-                runcmd ("umount", mountdir, NULL);
-#else
-                runcmd ("umount", "-l", mountdir, NULL);
 #endif
-                rmdir (mountdir);
+                gf_umount_lazy ("glusterd", mountdir, 1);
+
                 _exit (EXIT_SUCCESS);
         }
         ret = (waitpid (pid, &status, 0) == pid &&
diff --git a/xlators/mgmt/glusterd/src/glusterd-replace-brick.c b/xlators/mgmt/glusterd/src/glusterd-replace-brick.c
index 7c2b545905d..8bfa2d4bdd6 100644
--- a/xlators/mgmt/glusterd/src/glusterd-replace-brick.c
+++ b/xlators/mgmt/glusterd/src/glusterd-replace-brick.c
@@ -849,20 +849,6 @@ rb_spawn_glusterfs_client (glusterd_volinfo_t *volinfo,
         if (ret)
                 goto out;
 
-        runinit (&runner);
-        runner_add_args (&runner, "/bin/umount", "-l", mntpt, NULL);
-        ret = runner_run_reuse (&runner);
-        if (ret) {
-                runner_log (&runner, this->name, GF_LOG_DEBUG,
-                            "Lazy unmount failed on maintenance client");
-                runner_end (&runner);
-                goto out;
-        } else {
-                runner_log (&runner, this->name, GF_LOG_DEBUG,
-                            "Successfully unmounted  maintenance client");
-                runner_end (&runner);
-        }
-
 
 out:
 
@@ -1050,27 +1036,6 @@ out:
 }
 
 static int
-rb_mountpoint_rmdir (glusterd_volinfo_t *volinfo,
-                     glusterd_brickinfo_t *src_brickinfo)
-{
-        char             mntpt[PATH_MAX] = {0,};
-        int              ret                        = -1;
-
-        GLUSTERD_GET_RB_MNTPT (mntpt, sizeof (mntpt), volinfo);
-        ret = rmdir (mntpt);
-        if (ret) {
-                gf_log ("", GF_LOG_DEBUG, "rmdir failed, reason: %s",
-                        strerror (errno));
-                goto out;
-        }
-
-        ret = 0;
-
-out:
-        return ret;
-}
-
-static int
 rb_destroy_maintenance_client (glusterd_volinfo_t *volinfo,
                                glusterd_brickinfo_t *src_brickinfo)
 {
@@ -1079,6 +1044,7 @@ rb_destroy_maintenance_client (glusterd_volinfo_t *volinfo,
         char              volfile[PATH_MAX]           = {0,};
         int               ret                         = -1;
         int               mntfd                       = -1;
+        char              mntpt[PATH_MAX]             = {0,};
 
         this = THIS;
         priv = this->private;
@@ -1094,11 +1060,14 @@ rb_destroy_maintenance_client (glusterd_volinfo_t *volinfo,
                 goto out;
         }
 
-        ret = rb_mountpoint_rmdir (volinfo, src_brickinfo);
+        GLUSTERD_GET_RB_MNTPT (mntpt, sizeof (mntpt), volinfo);
+        ret = gf_umount_lazy (this->name, mntpt, 1);
         if (ret) {
-                gf_log (this->name, GF_LOG_DEBUG, "rmdir of mountpoint "
-                        "failed");
-                goto out;
+                gf_log (this->name, GF_LOG_WARNING,
+                        "Lazy unmount failed on maintenance client");
+        } else {
+                gf_log (this->name, GF_LOG_DEBUG,
+                        "Successfully unmounted  maintenance client");
         }
 
         snprintf (volfile, PATH_MAX, "%s/vols/%s/%s", priv->workdir,
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index 34d59e1d225..cae08ddf356 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -11987,7 +11987,6 @@ int
 glusterd_remove_auxiliary_mount (char *volname)
 {
         int       ret                = -1;
-        runner_t  runner             = {0,};
         char      mountdir[PATH_MAX] = {0,};
         char      pidfile[PATH_MAX]  = {0,};
         xlator_t *this               = NULL;
@@ -12004,20 +12003,11 @@ glusterd_remove_auxiliary_mount (char *volname)
         }
 
         GLUSTERD_GET_QUOTA_AUX_MOUNT_PATH (mountdir, volname, "/");
-        runinit (&runner);
-        runner_add_args (&runner, "umount",
-
-#if GF_LINUX_HOST_OS
-                        "-l",
-#endif
-                        mountdir, NULL);
-        ret = runner_run_reuse (&runner);
+        ret = gf_umount_lazy (this->name, mountdir, 1);
         if (ret)
                 gf_log (this->name, GF_LOG_ERROR, "umount on %s failed, "
                         "reason : %s", mountdir, strerror (errno));
-        runner_end (&runner);
 
-        rmdir (mountdir);
         return ret;
 }
 
@@ -12142,7 +12132,7 @@ glusterd_umount (const char *path)
 
         runinit (&runner);
         snprintf (msg, sizeof (msg), "umount path %s", path);
-        runner_add_args (&runner, "umount", "-f", path, NULL);
+        runner_add_args (&runner, _PATH_UMOUNT, "-f", path, NULL);
         runner_log (&runner, this->name, GF_LOG_DEBUG, msg);
         ret = runner_run (&runner);
         if (ret)
diff --git a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
index cde4bec844d..4685fa6a8e4 100644
--- a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
+++ b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
@@ -2241,7 +2241,6 @@ glusterd_stop_volume (glusterd_volinfo_t *volinfo)
         int                     ret                     = -1;
         glusterd_brickinfo_t    *brickinfo              = NULL;
         char                    mountdir[PATH_MAX]      = {0,};
-        runner_t                runner                  = {0,};
         char                    pidfile[PATH_MAX]       = {0,};
         xlator_t                *this                   = NULL;
 
@@ -2278,19 +2277,11 @@ glusterd_stop_volume (glusterd_volinfo_t *volinfo)
                 GLUSTERD_GET_QUOTA_AUX_MOUNT_PATH (mountdir, volinfo->volname,
                                                    "/");
 
-                runinit (&runner);
-                runner_add_args (&runner, "umount",
-
-                #if GF_LINUX_HOST_OS
-                                "-l",
-                #endif
-                                mountdir, NULL);
-                ret = runner_run_reuse (&runner);
+                ret = gf_umount_lazy (this->name, mountdir, 0);
                 if (ret)
-                        gf_log (this->name, GF_LOG_ERROR, "umount on %s failed, "
-                                "reason : %s", mountdir, strerror (errno));
-
-                runner_end (&runner);
+                        gf_log (this->name, GF_LOG_ERROR,
+                                "umount on %s failed, reason : %s",
+                                mountdir, strerror (errno));
         }
 
         ret = glusterd_handle_snapd_option (volinfo);
@@ -2496,7 +2487,7 @@ glusterd_clearlocks_unmount (glusterd_volinfo_t *volinfo, char *mntpt)
          * stat() on mount can be due to network failures.*/
 
         runinit (&runner);
-        runner_add_args (&runner, "/bin/umount", "-f", NULL);
+        runner_add_args (&runner, _PATH_UMOUNT, "-f", NULL);
         runner_argprintf (&runner, "%s", mntpt);
 
         synclock_unlock (&priv->big_lock);