summaryrefslogtreecommitdiffstats
path: root/xlators
diff options
context:
space:
mode:
authorCsaba Henk <csaba@redhat.com>2012-03-09 09:11:23 +0100
committerAnand Avati <avati@redhat.com>2012-05-21 18:32:13 -0700
commitc7cc38b66f5e8581815d4d680190b96592313bc3 (patch)
tree548804a1ea9d6a78b48967e5c0bfaa408b3cc522 /xlators
parent04f6cd78fab5a2fa8a02da3be27b080a15aec203 (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/3259 Tested-by: Gluster Build System <jenkins@build.gluster.com> Reviewed-by: Jeff Darcy <jdarcy@redhat.com> Reviewed-by: Anand Avati <avati@redhat.com>
Diffstat (limited to 'xlators')
-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 923cf78d4cd..bb7dca61d47 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"""