Browse code

Optimizations for recurrsive unmount

When a recursive unmount fails, don't bother parsing the mount table to check
if what we expected to be a mountpoint is still mounted. `EINVAL` is
returned when you try to unmount something that is not a mountpoint, the
other cases of `EINVAL` would not apply here unless everything is just
wrong. Parsing the mount table over and over is relatively expensive,
especially in the code path that it's in.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

Brian Goff authored on 2017/08/03 10:29:43
Showing 1 changed files
... ...
@@ -4,6 +4,8 @@ import (
4 4
 	"sort"
5 5
 	"strings"
6 6
 
7
+	"syscall"
8
+
7 9
 	"github.com/sirupsen/logrus"
8 10
 )
9 11
 
... ...
@@ -77,18 +79,30 @@ func RecursiveUnmount(target string) error {
77 77
 			continue
78 78
 		}
79 79
 		logrus.Debugf("Trying to unmount %s", m.Mountpoint)
80
-		err = Unmount(m.Mountpoint)
81
-		if err != nil && i == len(mounts)-1 {
82
-			if mounted, err := Mounted(m.Mountpoint); err != nil || mounted {
83
-				return err
80
+		err = unmount(m.Mountpoint, mntDetach)
81
+		if err != nil {
82
+			// If the error is EINVAL either this whole package is wrong (invalid flags passed to unmount(2)) or this is
83
+			// not a mountpoint (which is ok in this case).
84
+			// Meanwhile calling `Mounted()` is very expensive.
85
+			//
86
+			// We've purposefully used `syscall.EINVAL` here instead of `unix.EINVAL` to avoid platform branching
87
+			// Since `EINVAL` is defined for both Windows and Linux in the `syscall` package (and other platforms),
88
+			//   this is nicer than defining a custom value that we can refer to in each platform file.
89
+			if err == syscall.EINVAL {
90
+				continue
91
+			}
92
+			if i == len(mounts)-1 {
93
+				if mounted, e := Mounted(m.Mountpoint); e != nil || mounted {
94
+					return err
95
+				}
96
+				continue
84 97
 			}
85
-			// Ignore errors for submounts and continue trying to unmount others
86
-			// The final unmount should fail if there ane any submounts remaining
87
-		} else if err != nil {
88
-			logrus.Errorf("Failed to unmount %s: %v", m.Mountpoint, err)
89
-		} else if err == nil {
90
-			logrus.Debugf("Unmounted %s", m.Mountpoint)
98
+			// This is some submount, we can ignore this error for now, the final unmount will fail if this is a real problem
99
+			logrus.WithError(err).Warnf("Failed to unmount submount %s", m.Mountpoint)
100
+			continue
91 101
 		}
102
+
103
+		logrus.Debugf("Unmounted %s", m.Mountpoint)
92 104
 	}
93 105
 	return nil
94 106
 }