1. docker
  2. Commit 90be078fe59a8cfeff2bcc5dc2f34a00309837b6
Browse code

pkg/mount: refactor Unmount()

It has been pointed out that we're ignoring EINVAL from umount(2)
everywhere, so let's move it to a lower-level function. Also, its
implementation should be the same for any UNIX incarnation, so
let's consolidate it.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

Kir Kolyshkin authored on 2018/10/26 02:44:28
Showing 4 changed files
pkg/mount/mount.go
History View file @ 90be078
... ... 
@@ -3,7 +3,6 @@ package mount // import "github.com/docker/docker/pkg/mount"
3 3 
 import (
4 4 
 	"sort"
5 5 
 	"strings"
6 
-	"syscall"
7 6
8 7 
 	"github.com/sirupsen/logrus"
9 8 
 )
... ... 
@@ -89,12 +88,7 @@ func ForceMount(device, target, mType, options string) error {
89 89 
 // Unmount lazily unmounts a filesystem on supported platforms, otherwise
90 90 
 // does a normal unmount.
91 91 
 func Unmount(target string) error {
92 
-	err := unmount(target, mntDetach)
93 
-	if err == syscall.EINVAL {
94 
-		// ignore "not mounted" error
95 
-		err = nil
96 
-	}
97 
-	return err
92 
+	return unmount(target, mntDetach)
98 93 
 }
99 94
100 95 
 // RecursiveUnmount unmounts the target and all mounts underneath, starting with
... ... 
@@ -114,25 +108,14 @@ func RecursiveUnmount(target string) error {
114 114 
 		logrus.Debugf("Trying to unmount %s", m.Mountpoint)
115 115 
 		err = unmount(m.Mountpoint, mntDetach)
116 116 
 		if err != nil {
117 
-			// If the error is EINVAL either this whole package is wrong (invalid flags passed to unmount(2)) or this is
118 
-			// not a mountpoint (which is ok in this case).
119 
-			// Meanwhile calling `Mounted()` is very expensive.
120 
-			//
121 
-			// We've purposefully used `syscall.EINVAL` here instead of `unix.EINVAL` to avoid platform branching
122 
-			// Since `EINVAL` is defined for both Windows and Linux in the `syscall` package (and other platforms),
123 
-			//   this is nicer than defining a custom value that we can refer to in each platform file.
124 
-			if err == syscall.EINVAL {
125 
-				continue
126 
-			}
127 
-			if i == len(mounts)-1 {
117 
+			if i == len(mounts)-1 { // last mount
128 118 
 				if mounted, e := Mounted(m.Mountpoint); e != nil || mounted {
129 119 
 					return err
130 120 
 				}
131 
-				continue
121 
+			} else {
122 
+				// This is some submount, we can ignore this error for now, the final unmount will fail if this is a real problem
123 
+				logrus.WithError(err).Warnf("Failed to unmount submount %s", m.Mountpoint)
132 124 
 			}
133 
-			// This is some submount, we can ignore this error for now, the final unmount will fail if this is a real problem
134 
-			logrus.WithError(err).Warnf("Failed to unmount submount %s", m.Mountpoint)
135 
-			continue
136 125 
 		}
137 126
138 127 
 		logrus.Debugf("Unmounted %s", m.Mountpoint)
pkg/mount/mounter_freebsd.go
History View file @ 90be078
... ... 
@@ -14,8 +14,6 @@ import (
14 14 
 	"fmt"
15 15 
 	"strings"
16 16 
 	"unsafe"
17 
-
18 
-	"golang.org/x/sys/unix"
19 17 
 )
20 18
21 19 
 func allocateIOVecs(options []string) []C.struct_iovec {
... ... 
@@ -54,7 +52,3 @@ func mount(device, target, mType string, flag uintptr, data string) error {
54 54 
 	}
55 55 
 	return nil
56 56 
 }
57 
-
58 
-func unmount(target string, flag int) error {
59 
-	return unix.Unmount(target, flag)
60 
-}
pkg/mount/mounter_linux.go
History View file @ 90be078
... ... 
@@ -51,7 +51,3 @@ func mount(device, target, mType string, flags uintptr, data string) error {
51 51
52 52 
 	return nil
53 53 
 }
54 
-
55 
-func unmount(target string, flag int) error {
56 
-	return unix.Unmount(target, flag)
57 
-}
pkg/mount/unmount_unix.go
History View file @ 90be078
58 54 
new file mode 100644
... ... 
@@ -0,0 +1,17 @@
0 
+// +build !windows
1 
+
2 
+package mount // import "github.com/docker/docker/pkg/mount"
3 
+
4 
+import "golang.org/x/sys/unix"
5 
+
6 
+func unmount(target string, flags int) error {
7 
+	err := unix.Unmount(target, flags)
8 
+	if err == unix.EINVAL {
9 
+		// Ignore "not mounted" error here. Note the same error
10 
+		// can be returned if flags are invalid, so this code
11 
+		// assumes that the flags value is always correct.
12 
+		err = nil
13 
+	}
14 
+
15 
+	return err
16 
+}