Commit d0c4469a authored by Kirill Smelkov's avatar Kirill Smelkov

wcfs: Server.stop: Don't report first unmount failure to outside

If first unmount fails, e.g. due to "device or resource is busy", we are
trying to unmount the filesystem the second time after force
kill/FUSE-abort (see 5f684a49 "wcfs: Server.stop: Make sure to remove
mount entry even if we had to use FUSE abort").

This way the caller of Server.stop should get an error only if that
second unmount fails, not on unmount-1 error, which should be considered
as internal to Server.stop implementation.

If we don't hide that unmount-1 error and raise it to the caller, from
outside it can confusingly look like "the server is successfully
stopped, but nevertheless we are raised with an error".
parent 49f826b1
...@@ -469,7 +469,7 @@ def __stop(wcsrv, ctx, _onstuck): ...@@ -469,7 +469,7 @@ def __stop(wcsrv, ctx, _onstuck):
try: try:
if _is_mountpoint(wcsrv.mountpoint): # could be unmounted from outside if _is_mountpoint(wcsrv.mountpoint): # could be unmounted from outside
_fuse_unmount(wcsrv.mountpoint) _fuse_unmount(wcsrv.mountpoint)
except: except Exception as e:
# if clean unmount failed -> kill -TERM wcfs and force abort of fuse connection. # if clean unmount failed -> kill -TERM wcfs and force abort of fuse connection.
# #
# aborting fuse connection is needed in case wcfs/kernel will be stuck # aborting fuse connection is needed in case wcfs/kernel will be stuck
...@@ -482,7 +482,11 @@ def __stop(wcsrv, ctx, _onstuck): ...@@ -482,7 +482,11 @@ def __stop(wcsrv, ctx, _onstuck):
wcsrv._fuseabort.write(b"1\n") wcsrv._fuseabort.write(b"1\n")
wcsrv._fuseabort.flush() wcsrv._fuseabort.flush()
defer(_) defer(_)
raise
# treat first unmount failure as temporary - e.g. due to "device or resource is busy".
# we'll be retrying to unmount the filesystem the second time after kill/fuse-abort.
if not isinstance(e, _FUSEUnmountError):
raise
# ---- misc ---- # ---- misc ----
...@@ -550,6 +554,8 @@ def _rmdir_ifexists(path): ...@@ -550,6 +554,8 @@ def _rmdir_ifexists(path):
# _fuse_unmount calls `fusermount -u` + logs details if unmount failed. # _fuse_unmount calls `fusermount -u` + logs details if unmount failed.
# #
# Additional options to fusermount can be passed via optv. # Additional options to fusermount can be passed via optv.
class _FUSEUnmountError(RuntimeError):
pass
@func @func
def _fuse_unmount(mntpt, *optv): def _fuse_unmount(mntpt, *optv):
ret, out = _sysproccallout(["fusermount", "-u"] + list(optv) + [mntpt]) ret, out = _sysproccallout(["fusermount", "-u"] + list(optv) + [mntpt])
...@@ -576,7 +582,7 @@ def _fuse_unmount(mntpt, *optv): ...@@ -576,7 +582,7 @@ def _fuse_unmount(mntpt, *optv):
opts += ' ' opts += ' '
emsg = "fuse_unmount %s%s: failed: %s" % (opts, mntpt, out) emsg = "fuse_unmount %s%s: failed: %s" % (opts, mntpt, out)
log.warn(emsg) log.warn(emsg)
raise RuntimeError("%s\n(more details logged)" % emsg) raise _FUSEUnmountError("%s\n(more details logged)" % emsg)
# _is_mountpoint returns whether path is a mountpoint # _is_mountpoint returns whether path is a mountpoint
def _is_mountpoint(path): # -> bool def _is_mountpoint(path): # -> bool
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment