summaryrefslogtreecommitdiffstats
path: root/xlators/features
diff options
context:
space:
mode:
authorCsaba Henk <csaba@redhat.com>2012-03-09 09:11:23 +0100
committerVijay Bellur <vijay@gluster.com>2012-06-10 07:47:50 -0700
commit6d602670ef63e184492e19ed2dea6095db3ec36e (patch)
tree15ec5a7066a47c4dc0b857310611a9a61f29ead1 /xlators/features
parent05ad7e460c9f7ab7759313dfcd07ab37f9acd2b2 (diff)
geo-rep / gsyncd: fix cleanup of temporary mounts
[This is a "forward port" of fafd5c17, http://review.gluster.com/2908] The "finally" clause that was meant to cleanup after the temp mount has not covered the case of getting signalled (eg. by monitor, upon worker timing out). So here we "outsource" the cleanup to an ephemeral child process. Child calls setsid(2) so it won't be bothered by internal process management. We use a pipe in between worker and the cleanup child; when child sees the worker end getting closed, it performs the cleanup. Worker end can get closed either because worker closes it (normal case), or because worker has terminated (faulty case) -- thus as bonus, we get a nice uniform handling with no need to differentiate between normal and faulty cases. The faulty case that was seen IRL -- ie., users of maintainance mounts hang in chdir(2) to mount point -- can be simulated for testing purposes by applying the following patch: diff --git a/xlators/mount/fuse/src/fuse-bridge.c b/xlators/mount/fuse/src/fuse-bridge.c index acd3c68..1ce5dc1 100644 --- a/xlators/mount/fuse/src/fuse-bridge.c +++ b/xlators/mount/fuse/src/fuse-bridge.c @@ -2918,7 +2918,7 @@ fuse_init (xlator_t *this, fuse_in_header_t *finh, void *msg) if (fini->minor < 9) *priv->msg0_len_p = sizeof(*finh) + FUSE_COMPAT_WRITE_IN_SIZE; #endif - ret = send_fuse_obj (this, finh, &fino); + ret = priv->client_pid_set ? 0 : send_fuse_obj (this, finh, &fino); if (ret == 0) gf_log ("glusterfs-fuse", GF_LOG_INFO, "FUSE inited with protocol versions:" Change-Id: I14bad56a60a7fa82d0104fa4b9a20f4e42a7186f BUG: 786291 Signed-off-by: Csaba Henk <csaba@redhat.com> Reviewed-on: http://review.gluster.com/3542 Tested-by: Gluster Build System <jenkins@build.gluster.com> Reviewed-by: Vijay Bellur <vijay@gluster.com>
Diffstat (limited to 'xlators/features')
-rw-r--r--xlators/features/marker/utils/syncdaemon/resource.py93
1 files changed, 76 insertions, 17 deletions
diff --git a/xlators/features/marker/utils/syncdaemon/resource.py b/xlators/features/marker/utils/syncdaemon/resource.py
index db6d5ce121c..de271bd3939 100644
--- a/xlators/features/marker/utils/syncdaemon/resource.py
+++ b/xlators/features/marker/utils/syncdaemon/resource.py
@@ -3,6 +3,7 @@ import os
import sys
import stat
import time
+import fcntl
import errno
import struct
import socket
@@ -149,6 +150,14 @@ class Popen(subprocess.Popen):
t.start()
cls.errhandler = t
+ @classmethod
+ def fork(cls):
+ """fork wrapper that restarts errhandler thread in child"""
+ pid = os.fork()
+ if not pid:
+ cls.init_errhandler()
+ return pid
+
def __init__(self, args, *a, **kw):
"""customizations for subprocess.Popen instantiation
@@ -172,8 +181,8 @@ class Popen(subprocess.Popen):
assert(getattr(self, 'errhandler', None))
self.errstore[self] = []
- def errfail(self):
- """fail nicely if child did not terminate with success"""
+ def errlog(self):
+ """make a log about child's failure event"""
filling = ""
if self.elines:
filling = ", saying:"
@@ -182,6 +191,10 @@ class Popen(subprocess.Popen):
for l in self.elines:
for ll in l.rstrip().split("\n"):
logging.error(self.args[0] + "> " + ll.rstrip())
+
+ def errfail(self):
+ """fail nicely if child did not terminate with success"""
+ self.errlog()
syncdutils.finalize(exval = 1)
def terminate_geterr(self, fail_on_err = True):
@@ -626,6 +639,7 @@ class GLUSTER(AbstractUrl, SlaveLocal, SlaveRemote):
def __init__(self, params):
self.params = params
+ self.mntpt = None
@classmethod
def get_glusterprog(cls):
@@ -644,7 +658,7 @@ class GLUSTER(AbstractUrl, SlaveLocal, SlaveRemote):
def make_mount_argv(self, *a):
raise NotImplementedError
- def cleanup_mntpt(self):
+ def cleanup_mntpt(self, *a):
pass
def handle_mounter(self, po):
@@ -657,24 +671,67 @@ class GLUSTER(AbstractUrl, SlaveLocal, SlaveRemote):
change into the mount, and lazy unmount the
filesystem.
"""
- mounted = False
- try:
- po = Popen(self.make_mount_argv(*a), **self.mountkw)
+
+ mpi, mpo = os.pipe()
+ mh = Popen.fork()
+ if mh:
+ os.close(mpi)
+ fcntl.fcntl(mpo, fcntl.F_SETFD, fcntl.FD_CLOEXEC)
+ d = None
+ margv = self.make_mount_argv(*a)
+ if self.mntpt:
+ # mntpt is determined pre-mount
+ d = self.mntpt
+ os.write(mpo, d + '\0')
+ po = Popen(margv, **self.mountkw)
self.handle_mounter(po)
po.terminate_geterr()
- d = self.mntpt
- mounted = True
logging.debug('auxiliary glusterfs mount in place')
+ if not d:
+ # mntpt is determined during mount
+ d = self.mntpt
+ os.write(mpo, d + '\0')
+ os.write(mpo, 'M')
os.chdir(d)
- self.umount_l(d).terminate_geterr()
- mounted = False
- finally:
+ os.close(mpo)
+ _, rv = syncdutils.waitpid(mh, 0)
+ if rv:
+ rv = (os.WIFEXITED(rv) and os.WEXITSTATUS(rv) or 0) - \
+ (os.WIFSIGNALED(rv) and os.WTERMSIG(rv) or 0)
+ logging.warn('stale mount possibly left behind on ' + d)
+ raise GsyncdError("cleaning up temp mountpoint %s failed with status %d" % \
+ (d, rv))
+ else:
+ rv = 0
try:
- if mounted:
- self.umount_l(d).terminate_geterr(fail_on_err = False)
- self.cleanup_mntpt()
+ os.setsid()
+ os.close(mpo)
+ mntdata = ''
+ while True:
+ c = os.read(mpi, 1)
+ if not c:
+ break
+ mntdata += c
+ if mntdata:
+ mounted = False
+ if mntdata[-1] == 'M':
+ mntdata = mntdata[:-1]
+ assert(mntdata)
+ mounted = True
+ assert(mntdata[-1] == '\0')
+ mntpt = mntdata[:-1]
+ assert(mntpt)
+ if mounted:
+ 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)
except:
- logging.warn('stale mount possibly left behind on ' + d)
+ logging.exception('mount cleanup failure:')
+ rv = 200
+ os._exit(rv)
logging.debug('auxiliary glusterfs mount prepared')
class DirectMounter(Mounter):
@@ -691,8 +748,10 @@ class GLUSTER(AbstractUrl, SlaveLocal, SlaveRemote):
self.mntpt = tempfile.mkdtemp(prefix = 'gsyncd-aux-mount-')
return [self.get_glusterprog()] + ['--' + p for p in self.params] + [self.mntpt]
- def cleanup_mntpt(self):
- os.rmdir(self.mntpt)
+ def cleanup_mntpt(self, mntpt = None):
+ if not mntpt:
+ mntpt = self.mntpt
+ os.rmdir(mntpt)
class MountbrokerMounter(Mounter):
"""mounter backend using the mountbroker gluster service"""