Blob Blame History Raw
From fa2086eaa9545b472acd3fcc07be776d9f2cb38a Mon Sep 17 00:00:00 2001
From: Kotresh HR <khiremat@redhat.com>
Date: Thu, 15 Feb 2018 01:46:29 -0500
Subject: [PATCH 646/646] geo-rep: Remove lazy umount and use mount namespaces

Lazy umounting the master volume by worker causes
issues with rsync's usage of getcwd. Henc removing
the lazy umount and using private mount namespace
for the same. On the slave, the lazy umount is
retained as we can't use private namespace in non
root geo-rep setup because gsyncd is spawned as
non privileged user.

Backport of https://review.gluster.org/#/c/19544/

Change-Id: I851e8dc2b8523dc5668a97e87ef619ab70471dfd
BUG: 1547931
Signed-off-by: Kotresh HR <khiremat@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/131128
Tested-by: RHGS Build Bot <nigelb@redhat.com>
Reviewed-by: Aravinda Vishwanathapura Krishna Murthy <avishwan@redhat.com>
Reviewed-by: Atin Mukherjee <amukherj@redhat.com>
---
 geo-replication/syncdaemon/gconf.py      |  3 +++
 geo-replication/syncdaemon/gsyncd.py     | 14 +++++++-----
 geo-replication/syncdaemon/monitor.py    | 38 ++++++++++++++++++++------------
 geo-replication/syncdaemon/resource.py   | 16 ++++++++++++--
 geo-replication/syncdaemon/syncdutils.py | 18 ++++++++++++++-
 glusterfs.spec.in                        |  4 ++++
 6 files changed, 70 insertions(+), 23 deletions(-)

diff --git a/geo-replication/syncdaemon/gconf.py b/geo-replication/syncdaemon/gconf.py
index 97395b4..2280f44 100644
--- a/geo-replication/syncdaemon/gconf.py
+++ b/geo-replication/syncdaemon/gconf.py
@@ -28,5 +28,8 @@ class GConf(object):
     active_earlier = False
     passive_earlier = False
     mgmt_lock_fd = None
+    mountbroker = False
+    mount_point = None
+    mbr_umount_cmd = []
 
 gconf = GConf()
diff --git a/geo-replication/syncdaemon/gsyncd.py b/geo-replication/syncdaemon/gsyncd.py
index f9471e4..96256cf 100644
--- a/geo-replication/syncdaemon/gsyncd.py
+++ b/geo-replication/syncdaemon/gsyncd.py
@@ -269,6 +269,8 @@ def main_i():
                   type=str, action='callback', callback=store_abs)
     op.add_option('--georep-session-working-dir', metavar='STATF',
                   type=str, action='callback', callback=store_abs)
+    op.add_option('--access-mount', default=False, action='store_true')
+    op.add_option('--slave-access-mount', default=False, action='store_true')
     op.add_option('--ignore-deletes', default=False, action='store_true')
     op.add_option('--isolated-slave', default=False, action='store_true')
     op.add_option('--use-rsync-xattrs', default=False, action='store_true')
@@ -414,7 +416,7 @@ def main_i():
                     o.get_opt_string() not in ('--version', '--help'))]
     remote_tunables = ['listen', 'go_daemon', 'timeout',
                        'session_owner', 'config_file', 'use_rsync_xattrs',
-                       'local_id', 'local_node']
+                       'local_id', 'local_node', 'slave_access_mount']
     rq_remote_tunables = {'listen': True}
 
     # precedence for sources of values: 1) commandline, 2) cfg file, 3)
@@ -748,15 +750,15 @@ def main_i():
     else:
         log_file = gconf.log_file
     if be_monitor:
-        label = 'monitor'
+        gconf.label = 'monitor'
     elif be_agent:
-        label = gconf.local_path
+        gconf.label = gconf.local_path
     elif remote:
         # master
-        label = gconf.local_path
+        gconf.label = gconf.local_path
     else:
-        label = 'slave'
-    startup(go_daemon=go_daemon, log_file=log_file, label=label)
+        gconf.label = 'slave'
+    startup(go_daemon=go_daemon, log_file=log_file, label=gconf.label)
     resource.Popen.init_errhandler()
 
     if be_agent:
diff --git a/geo-replication/syncdaemon/monitor.py b/geo-replication/syncdaemon/monitor.py
index dc0211e..087a202 100644
--- a/geo-replication/syncdaemon/monitor.py
+++ b/geo-replication/syncdaemon/monitor.py
@@ -24,7 +24,7 @@ import random
 from gconf import gconf
 from syncdutils import select, waitpid, errno_wrap
 from syncdutils import set_term_handler, is_host_local, GsyncdError
-from syncdutils import escape, Thread, finalize, memoize
+from syncdutils import escape, Thread, finalize, memoize, boolify
 from syncdutils import gf_event, EVENT_GEOREP_FAULTY
 
 from gsyncdstatus import GeorepStatus, set_monitor_status
@@ -301,19 +301,29 @@ class Monitor(object):
                 os.close(pr)
                 os.close(ra)
                 os.close(wa)
-                os.execv(sys.executable, argv + ['--feedback-fd', str(pw),
-                                                 '--local-path', w[0]['dir'],
-                                                 '--local-node', w[0]['host'],
-                                                 '--local-node-id',
-                                                 w[0]['uuid'],
-                                                 '--local-id',
-                                                 '.' + escape(w[0]['dir']),
-                                                 '--rpc-fd',
-                                                 ','.join([str(rw), str(ww),
-                                                           str(ra), str(wa)]),
-                                                 '--subvol-num', str(w[2])] +
-                         (['--is-hottier'] if w[3] else []) +
-                         ['--resource-remote', remote_host])
+                args_to_worker = argv + ['--feedback-fd', str(pw),
+                                         '--local-path', w[0]['dir'],
+                                         '--local-node', w[0]['host'],
+                                         '--local-node-id',
+                                         w[0]['uuid'],
+                                         '--local-id',
+                                         '.' + escape(w[0]['dir']),
+                                         '--rpc-fd',
+                                         ','.join([str(rw), str(ww),
+                                         str(ra), str(wa)]),
+                                         '--subvol-num', str(w[2])]
+
+                if w[3]:
+                    args_to_worker.append('--is-hottier')
+                args_to_worker += ['--resource-remote', remote_host]
+
+                access_mount = boolify(gconf.access_mount)
+                if access_mount:
+                    os.execv(sys.executable, args_to_worker)
+                else:
+                    unshare_cmd = ['unshare', '-m', '--propagation', 'private']
+                    cmd = unshare_cmd + args_to_worker
+                    os.execvp("unshare", cmd)
 
             cpids.add(cpid)
             agents.add(apid)
diff --git a/geo-replication/syncdaemon/resource.py b/geo-replication/syncdaemon/resource.py
index 943e3ec..39d537b 100644
--- a/geo-replication/syncdaemon/resource.py
+++ b/geo-replication/syncdaemon/resource.py
@@ -989,6 +989,8 @@ class SlaveRemote(object):
             extra_opts += ['--local-node', ln]
         if boolify(gconf.use_rsync_xattrs):
             extra_opts.append('--use-rsync-xattrs')
+        if boolify(gconf.slave_access_mount):
+            extra_opts.append('--slave-access-mount')
         po = Popen(rargs + gconf.remote_gsyncd.split() + extra_opts +
                    ['-N', '--listen', '--timeout', str(gconf.timeout),
                     slave],
@@ -1258,6 +1260,7 @@ class GLUSTER(AbstractUrl, SlaveLocal, SlaveRemote):
         def __init__(self, params):
             self.params = params
             self.mntpt = None
+            self.umount_cmd = []
 
         @classmethod
         def get_glusterprog(cls):
@@ -1348,13 +1351,16 @@ class GLUSTER(AbstractUrl, SlaveLocal, SlaveRemote):
                         assert(mntdata[-1] == '\0')
                         mntpt = mntdata[:-1]
                         assert(mntpt)
-                        if mounted:
+                        if mounted and gconf.label == 'slave' \
+                           and not boolify(gconf.slave_access_mount):
                             po = self.umount_l(mntpt)
                             po.terminate_geterr(fail_on_err=False)
                             if po.returncode != 0:
                                 po.errlog()
                                 rv = po.returncode
-                        self.cleanup_mntpt(mntpt)
+                        if gconf.label == 'slave' \
+                           and not boolify(gconf.slave_access_mount):
+                            self.cleanup_mntpt(mntpt)
                 except:
                     logging.exception('mount cleanup failure:')
                     rv = 200
@@ -1374,6 +1380,7 @@ class GLUSTER(AbstractUrl, SlaveLocal, SlaveRemote):
 
         def make_mount_argv(self):
             self.mntpt = tempfile.mkdtemp(prefix='gsyncd-aux-mount-')
+            gconf.mount_point = self.mntpt
             return [self.get_glusterprog()] + \
                 ['--' + p for p in self.params] + [self.mntpt]
 
@@ -1405,6 +1412,11 @@ class GLUSTER(AbstractUrl, SlaveLocal, SlaveRemote):
 
         def handle_mounter(self, po):
             self.mntpt = po.stdout.readline()[:-1]
+            gconf.mount_point = self.mntpt
+            gconf.mountbroker = True
+            self.umount_cmd = self.make_cli_argv() + ['umount']
+            gconf.mbr_umount_cmd = self.umount_cmd
+
             po.stdout.close()
             sup(self, po)
             if po.returncode != 0:
diff --git a/geo-replication/syncdaemon/syncdutils.py b/geo-replication/syncdaemon/syncdutils.py
index a22289e..8dc6c96 100644
--- a/geo-replication/syncdaemon/syncdutils.py
+++ b/geo-replication/syncdaemon/syncdutils.py
@@ -16,6 +16,7 @@ import fcntl
 import shutil
 import logging
 import socket
+import subprocess
 from threading import Lock, Thread as baseThread
 from errno import EACCES, EAGAIN, EPIPE, ENOTCONN, ECONNABORTED
 from errno import EINTR, ENOENT, EPERM, ESTALE, EBUSY, errorcode
@@ -188,7 +189,6 @@ def grabpidfile(fname=None, setpid=True):
 
 final_lock = Lock()
 
-
 def finalize(*a, **kw):
     """all those messy final steps we go trough upon termination
 
@@ -233,6 +233,22 @@ def finalize(*a, **kw):
             if sys.exc_info()[0] == OSError:
                 pass
 
+    """ Unmount if not done """
+    if gconf.mount_point:
+        if gconf.mountbroker:
+            umount_cmd = gconf.mbr_umount_cmd + [gconf.mount_point, 'lazy']
+        else:
+            umount_cmd = ['umount', '-l', gconf.mount_point]
+        p0 = subprocess.Popen(umount_cmd, stderr=subprocess.PIPE)
+        _, errdata = p0.communicate()
+        if p0.returncode == 0:
+            try:
+                os.rmdir(gconf.mount_point)
+            except OSError:
+                pass
+        else:
+            pass
+
     if gconf.log_exit:
         logging.info("exiting.")
     sys.stdout.flush()
diff --git a/glusterfs.spec.in b/glusterfs.spec.in
index fc9125b..b7bfcac 100644
--- a/glusterfs.spec.in
+++ b/glusterfs.spec.in
@@ -439,6 +439,7 @@ Requires:         %{name}%{?_isa} = %{version}-%{release}
 Requires:         %{name}-server%{?_isa} = %{version}-%{release}
 Requires:         python python-ctypes
 Requires:         rsync
+Requires:         util-linux
 
 %description geo-replication
 GlusterFS is a distributed file-system capable of scaling to several
@@ -2067,6 +2068,9 @@ fi
 %endif
 
 %changelog
+* Thu Feb 22 2018 Kotresh HR <khiremat@redhat.com>
+- Added util-linux as dependency to georeplication rpm (#1544382)
+
 * Wed Dec 20 2017 Milind Changire <mchangir@redhat.com>
 - Remove ExclusiveArch directive to help building on non-x86_64 arches (#1527772)
 
-- 
1.8.3.1