11209: Restrict --unmount* operations to given --subtype.

Add warnings about affecting other fuse mounts with --unmount flags.

diff --git a/services/fuse/arvados_fuse/command.py b/services/fuse/arvados_fuse/command.py
index fea6048798317f904393e5e9b0511895b68338ee..ef3e78e8709c57c03b2fc63cd05259cdc3dc91a7 100644 (file)
--- a/services/fuse/arvados_fuse/command.py
+++ b/services/fuse/arvados_fuse/command.py
@@ -93,9 +93,9 @@ class ArgumentParser(argparse.ArgumentParser):
 
         unmount = self.add_mutually_exclusive_group()
         unmount.add_argument('--unmount', action='store_true', default=False,
-                             help="Forcefully unmount the specified mountpoint (if it's a fuse mount) and exit.")
+                             help="Forcefully unmount the specified mountpoint (if it's a fuse mount) and exit. If --subtype is given, unmount only if the mount has the specified subtype. WARNING: This command can affect any kind of fuse mount, not just arv-mount.")
         unmount.add_argument('--unmount-all', action='store_true', default=False,
-                             help="Forcefully unmount every fuse mount at or below the specified mountpoint and exit.")
+                             help="Forcefully unmount every fuse mount at or below the specified path and exit. If --subtype is given, unmount only mounts that have the specified subtype. Exit non-zero if any other types of mounts are found at or below the given path. WARNING: This command can affect any kind of fuse mount, not just arv-mount.")
         unmount.add_argument('--replace', action='store_true', default=False,
                              help="If a fuse mount is already present at mountpoint, forcefully unmount it before mounting")
         self.add_argument('--unmount-timeout',
@@ -159,6 +159,7 @@ class Mount(object):
     def run(self):
         if self.args.unmount or self.args.unmount_all:
             unmount(path=self.args.mountpoint,
+                    subtype=self.args.subtype,
                     timeout=self.args.unmount_timeout,
                     recursive=self.args.unmount_all)
         elif self.args.exec_args:

diff --git a/services/fuse/arvados_fuse/unmount.py b/services/fuse/arvados_fuse/unmount.py
index db78ddc738311b40914087c7b07fd0e27d6700f1..e213c733d1768c6c7cb37346d3840b0d73bdcb54 100644 (file)
--- a/services/fuse/arvados_fuse/unmount.py
+++ b/services/fuse/arvados_fuse/unmount.py
@@ -26,7 +26,7 @@ def mountinfo():
     return mi
 
 
-def unmount(path, timeout=10, recursive=False):
+def unmount(path, subtype=None, timeout=10, recursive=False):
     """Unmount the fuse mount at path.
 
     Unmounting is done by writing 1 to the "abort" control file in
@@ -43,15 +43,23 @@ def unmount(path, timeout=10, recursive=False):
 
     path = os.path.realpath(path)
 
+    if subtype is None:
+        mnttype = None
+    elif subtype == '':
+        mnttype = 'fuse'
+    else:
+        mnttype = 'fuse.' + subtype
+
     if recursive:
         paths = []
         for m in mountinfo():
             if m.path == path or m.path.startswith(path+"/"):
                 paths.append(m.path)
-                if not m.is_fuse:
+                if not (m.is_fuse and (mnttype is None or
+                                       mnttype == m.mnttype)):
                     raise Exception(
-                        "cannot unmount {}: non-fuse mountpoint {}".format(
-                            path, m))
+                        "cannot unmount {}: mount type is {}".format(
+                            path, m.mnttype))
         for path in sorted(paths, key=len, reverse=True):
             unmount(path, timeout=timeout, recursive=False)
         return len(paths) > 0
@@ -66,7 +74,7 @@ def unmount(path, timeout=10, recursive=False):
     while True:
         mounted = False
         for m in mountinfo():
-            if m.is_fuse:
+            if m.is_fuse and (mnttype is None or mnttype == m.mnttype):
                 try:
                     if os.path.realpath(m.path) == path:
                         was_mounted = True

diff --git a/services/fuse/tests/test_unmount.py b/services/fuse/tests/test_unmount.py
index 972edaadc8baaf2bdd7cc5c2312473bc6adf81fd..716a0e00d704d16b5edcb8436b3ca68de157024b 100644 (file)
--- a/services/fuse/tests/test_unmount.py
+++ b/services/fuse/tests/test_unmount.py
@@ -32,10 +32,41 @@ class UnmountTest(IntegrationTest):
             self.assertNotIn(' '+self.mnt+' ', m)
 
     def _mounted(self, mounts):
-        all_mounts = subprocess.check_output(['mount', '-t', 'fuse.test'])
+        all_mounts = subprocess.check_output(['mount'])
         return [m for m in mounts
                 if ' '+m+' ' in all_mounts]
 
+    def _wait_for_mounts(self, mounts):
+        deadline = time.time() + 10
+        while self._mounted(mounts) != mounts:
+            time.sleep(0.1)
+            self.assertLess(time.time(), deadline)
+
+    def test_unmount_subtype(self):
+        mounts = []
+        for d in ['foo', 'bar']:
+            mnt = self.tmp+'/'+d
+            os.mkdir(mnt)
+            self.to_delete.insert(0, mnt)
+            mounts.append(mnt)
+            subprocess.check_call(
+                ['./bin/arv-mount', '--subtype', d, mnt])
+
+        self._wait_for_mounts(mounts)
+        self.assertEqual(mounts, self._mounted(mounts))
+        subprocess.call(['./bin/arv-mount', '--subtype', 'baz', '--unmount-all', self.tmp])
+        self.assertEqual(mounts, self._mounted(mounts))
+        subprocess.call(['./bin/arv-mount', '--subtype', 'bar', '--unmount', mounts[0]])
+        self.assertEqual(mounts, self._mounted(mounts))
+        subprocess.call(['./bin/arv-mount', '--subtype', '', '--unmount', self.tmp])
+        self.assertEqual(mounts, self._mounted(mounts))
+        subprocess.check_call(['./bin/arv-mount', '--subtype', 'foo', '--unmount', mounts[0]])
+        self.assertEqual(mounts[1:], self._mounted(mounts))
+        subprocess.check_call(['./bin/arv-mount', '--subtype', '', '--unmount-all', mounts[0]])
+        self.assertEqual(mounts[1:], self._mounted(mounts))
+        subprocess.check_call(['./bin/arv-mount', '--subtype', 'bar', '--unmount-all', self.tmp])
+        self.assertEqual([], self._mounted(mounts))
+
     def test_unmount_children(self):
         for d in ['foo', 'foo/bar', 'bar']:
             mnt = self.tmp+'/'+d
@@ -48,12 +79,7 @@ class UnmountTest(IntegrationTest):
             subprocess.check_call(
                 ['./bin/arv-mount', '--subtype', 'test', mnt])
 
-        # Wait for mounts to attach
-        deadline = time.time() + 10
-        while self._mounted(mounts) != mounts:
-            time.sleep(0.1)
-            self.assertLess(time.time(), deadline)
-
+        self._wait_for_mounts(mounts)
         self.assertEqual(mounts, self._mounted(mounts))
         subprocess.check_call(['./bin/arv-mount', '--unmount', self.tmp])
         self.assertEqual(mounts, self._mounted(mounts))