pkg/mount: wrap mount/umount errors

kolyshkin · kolyshkin · commit 65331369617e · 2018-12-10T20:07:02.000-08:00
The errors returned from Mount and Unmount functions are raw syscall.Errno errors (like EPERM or EINVAL), which provides no context about what has happened and why. Similar to os.PathError type, introduce mount.Error type with some context. The error messages will now look like this: > mount /tmp/mount-tests/source:/tmp/mount-tests/target, flags: 0x1001: operation not permitted or > mount tmpfs:/tmp/mount-test-source-516297835: operation not permitted Before this patch, it was just > operation not permitted [v2: add Cause()] [v3: rename MountError to Error, document Cause()] [v4: fixes; audited all users] [v5: make Error type private; changes after @cpuguy83 reviews] Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
diff --git a/container/container_unix.go b/container/container_unix.go
@@ -17,7 +17,6 @@ import (
 	"github.com/docker/docker/volume"
 	volumemounts "github.com/docker/docker/volume/mounts"
 	"github.com/opencontainers/selinux/go-selinux/label"
-	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/sys/unix"
 )
@@ -190,7 +189,7 @@ func (container *Container) UnmountIpcMount() error {
 		return nil
 	}
 	if err = mount.Unmount(shmPath); err != nil && !os.IsNotExist(err) {
-		return errors.Wrapf(err, "umount %s", shmPath)
+		return err
 	}
 	return nil
 }
@@ -380,7 +379,8 @@ func (container *Container) DetachAndUnmount(volumeEventLog func(name, action st
 
 	for _, mountPath := range mountPaths {
 		if err := mount.Unmount(mountPath); err != nil {
-			logrus.Warnf("%s unmountVolumes: Failed to do lazy umount for volume '%s': %v", container.ID, mountPath, err)
+			logrus.WithError(err).WithField("container", container.ID).
+				Warn("Unable to unmount")
 		}
 	}
 	return container.UnmountVolumes(volumeEventLog)
diff --git a/daemon/graphdriver/btrfs/btrfs.go b/daemon/graphdriver/btrfs/btrfs.go
@@ -178,7 +178,7 @@ func (d *Driver) Cleanup() error {
 	}
 
 	if umountErr != nil {
-		return errors.Wrapf(umountErr, "error unmounting %s", d.home)
+		return umountErr
 	}
 
 	return nil
diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go
@@ -1200,7 +1200,7 @@ func (devices *DeviceSet) growFS(info *devInfo) error {
 	options = joinMountOptions(options, devices.mountOptions)
 
 	if err := mount.Mount(info.DevName(), fsMountPoint, devices.BaseDeviceFilesystem, options); err != nil {
-		return fmt.Errorf("Error mounting '%s' on '%s' (fstype='%s' options='%s'): %s\n%v", info.DevName(), fsMountPoint, devices.BaseDeviceFilesystem, options, err, string(dmesg.Dmesg(256)))
+		return errors.Wrapf(err, "Failed to mount; dmesg: %s", string(dmesg.Dmesg(256)))
 	}
 
 	defer unix.Unmount(fsMountPoint, unix.MNT_DETACH)
@@ -2381,7 +2381,7 @@ func (devices *DeviceSet) MountDevice(hash, path, mountLabel string) error {
 	options = joinMountOptions(options, label.FormatMountLabel("", mountLabel))
 
 	if err := mount.Mount(info.DevName(), path, fstype, options); err != nil {
-		return fmt.Errorf("devmapper: Error mounting '%s' on '%s' (fstype='%s' options='%s'): %s\n%v", info.DevName(), path, fstype, options, err, string(dmesg.Dmesg(256)))
+		return errors.Wrapf(err, "Failed to mount; dmesg: %s", string(dmesg.Dmesg(256)))
 	}
 
 	if fstype == "xfs" && devices.xfsNospaceRetries != "" {
diff --git a/daemon/graphdriver/devmapper/driver.go b/daemon/graphdriver/devmapper/driver.go
@@ -16,7 +16,6 @@ import (
 	"github.com/docker/docker/pkg/locker"
 	"github.com/docker/docker/pkg/mount"
 	"github.com/docker/go-units"
-	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/sys/unix"
 )
@@ -129,11 +128,7 @@ func (d *Driver) Cleanup() error {
 		return err
 	}
 
-	if umountErr != nil {
-		return errors.Wrapf(umountErr, "error unmounting %s", d.home)
-	}
-
-	return nil
+	return umountErr
 }
 
 // CreateReadWrite creates a layer that is writable for use as a container
diff --git a/daemon/graphdriver/zfs/zfs.go b/daemon/graphdriver/zfs/zfs.go
@@ -19,6 +19,7 @@ import (
 	"github.com/docker/docker/pkg/parsers"
 	"github.com/mistifyio/go-zfs"
 	"github.com/opencontainers/selinux/go-selinux/label"
+	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/sys/unix"
 )
@@ -390,7 +391,7 @@ func (d *Driver) Get(id, mountLabel string) (_ containerfs.ContainerFS, retErr e
 	}
 
 	if err := mount.Mount(filesystem, mountpoint, "zfs", options); err != nil {
-		return nil, fmt.Errorf("error creating zfs mount of %s to %s: %v", filesystem, mountpoint, err)
+		return nil, errors.Wrap(err, "error creating zfs mount")
 	}
 
 	// this could be our first mount after creation of the filesystem, and the root dir may still have root
diff --git a/pkg/mount/mount.go b/pkg/mount/mount.go
@@ -2,11 +2,46 @@ package mount // import "github.com/docker/docker/pkg/mount"
 
 import (
 	"sort"
+	"strconv"
 	"strings"
 
 	"github.com/sirupsen/logrus"
 )
 
+// mountError records an error from mount or unmount operation
+type mountError struct {
+	op             string
+	source, target string
+	flags          uintptr
+	data           string
+	err            error
+}
+
+func (e *mountError) Error() string {
+	out := e.op + " "
+
+	if e.source != "" {
+		out += e.source + ":" + e.target
+	} else {
+		out += e.target
+	}
+
+	if e.flags != uintptr(0) {
+		out += ", flags: 0x" + strconv.FormatUint(uint64(e.flags), 16)
+	}
+	if e.data != "" {
+		out += ", data: " + e.data
+	}
+
+	out += ": " + e.err.Error()
+	return out
+}
+
+// Cause returns the underlying cause of the error
+func (e *mountError) Cause() error {
+	return e.err
+}
+
 // FilterFunc is a type defining a callback function
 // to filter out unwanted entries. It takes a pointer
 // to an Info struct (not fully populated, currently
diff --git a/pkg/mount/mounter_freebsd.go b/pkg/mount/mounter_freebsd.go
@@ -11,8 +11,8 @@ package mount // import "github.com/docker/docker/pkg/mount"
 import "C"
 
 import (
-	"fmt"
 	"strings"
+	"syscall"
 	"unsafe"
 )
 
@@ -47,8 +47,13 @@ func mount(device, target, mType string, flag uintptr, data string) error {
 	}
 
 	if errno := C.nmount(&rawOptions[0], C.uint(len(options)), C.int(flag)); errno != 0 {
-		reason := C.GoString(C.strerror(*C.__error()))
-		return fmt.Errorf("Failed to call nmount: %s", reason)
+		return &mountError{
+			op:     "mount",
+			source: device,
+			target: target,
+			flags:  flag,
+			err:    syscall.Errno(errno),
+		}
 	}
 	return nil
 }
diff --git a/pkg/mount/mounter_linux.go b/pkg/mount/mounter_linux.go
@@ -33,20 +33,41 @@ func mount(device, target, mType string, flags uintptr, data string) error {
 		// Initial call applying all non-propagation flags for mount
 		// or remount with changed data
 		if err := unix.Mount(device, target, mType, oflags, data); err != nil {
-			return err
+			return &mountError{
+				op:     "mount",
+				source: device,
+				target: target,
+				flags:  oflags,
+				data:   data,
+				err:    err,
+			}
 		}
 	}
 
 	if flags&ptypes != 0 {
 		// Change the propagation type.
 		if err := unix.Mount("", target, "", flags&pflags, ""); err != nil {
+			return &mountError{
+				op:     "remount",
+				target: target,
+				flags:  flags & pflags,
+				err:    err,
+			}
 			return err
 		}
 	}
 
 	if oflags&broflags == broflags {
 		// Remount the bind to apply read only.
-		return unix.Mount("", target, "", oflags|unix.MS_REMOUNT, "")
+		if err := unix.Mount("", target, "", oflags|unix.MS_REMOUNT, ""); err != nil {
+			return &mountError{
+				op:     "remount-ro",
+				target: target,
+				flags:  oflags | unix.MS_REMOUNT,
+				err:    err,
+			}
+
+		}
 	}
 
 	return nil
diff --git a/pkg/mount/sharedsubtree_linux_test.go b/pkg/mount/sharedsubtree_linux_test.go
@@ -7,6 +7,7 @@ import (
 	"path"
 	"testing"
 
+	"github.com/pkg/errors"
 	"golang.org/x/sys/unix"
 )
 
@@ -326,7 +327,7 @@ func TestSubtreeUnbindable(t *testing.T) {
 	}()
 
 	// then attempt to mount it to target. It should fail
-	if err := Mount(sourceDir, targetDir, "none", "bind,rw"); err != nil && err != unix.EINVAL {
+	if err := Mount(sourceDir, targetDir, "none", "bind,rw"); err != nil && errors.Cause(err) != unix.EINVAL {
 		t.Fatal(err)
 	} else if err == nil {
 		t.Fatalf("%q should not have been bindable", sourceDir)
diff --git a/pkg/mount/unmount_unix.go b/pkg/mount/unmount_unix.go
@@ -6,12 +6,17 @@ import "golang.org/x/sys/unix"
 
 func unmount(target string, flags int) error {
 	err := unix.Unmount(target, flags)
-	if err == unix.EINVAL {
+	if err == nil || err == unix.EINVAL {
 		// Ignore "not mounted" error here. Note the same error
 		// can be returned if flags are invalid, so this code
 		// assumes that the flags value is always correct.
-		err = nil
+		return nil
 	}
 
-	return err
+	return &mountError{
+		op:     "umount",
+		target: target,
+		flags:  uintptr(flags),
+		err:    err,
+	}
 }
diff --git a/plugin/manager_linux.go b/plugin/manager_linux.go
@@ -61,7 +61,7 @@ func (pm *Manager) enable(p *v2.Plugin, c *controller, force bool) error {
 	if err := pm.executor.Create(p.GetID(), *spec, stdout, stderr); err != nil {
 		if p.PluginObj.Config.PropagatedMount != "" {
 			if err := mount.Unmount(propRoot); err != nil {
-				logrus.Warnf("Could not unmount %s: %v", propRoot, err)
+				logrus.WithField("plugin", p.Name()).WithError(err).Warn("Failed to unmount vplugin propagated mount root")
 			}
 		}
 		return errors.WithStack(err)
diff --git a/volume/local/local.go b/volume/local/local.go
@@ -344,7 +344,7 @@ func (v *localVolume) unmount() error {
 	if v.opts != nil {
 		if err := mount.Unmount(v.path); err != nil {
 			if mounted, mErr := mount.Mounted(v.path); mounted || mErr != nil {
-				return errdefs.System(errors.Wrapf(err, "error while unmounting volume path '%s'", v.path))
+				return errdefs.System(err)
 			}
 		}
 		v.active.mounted = false
diff --git a/volume/local/local_unix.go b/volume/local/local_unix.go
@@ -86,7 +86,7 @@ func (v *localVolume) mount() error {
 		}
 	}
 	err := mount.Mount(v.opts.MountDevice, v.path, v.opts.MountType, mountOpts)
-	return errors.Wrapf(err, "error while mounting volume with options: %s", v.opts)
+	return errors.Wrap(err, "failed to mount local volume")
 }
 
 func (v *localVolume) CreatedAt() (time.Time, error) {

Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,6 @@ import (`
`17`	`17`	`"github.com/docker/docker/volume"`
`18`	`18`	`volumemounts "github.com/docker/docker/volume/mounts"`
`19`	`19`	`"github.com/opencontainers/selinux/go-selinux/label"`
`20`		`- "github.com/pkg/errors"`
`21`	`20`	`"github.com/sirupsen/logrus"`
`22`	`21`	`"golang.org/x/sys/unix"`
`23`	`22`	`)`
`@@ -190,7 +189,7 @@ func (container *Container) UnmountIpcMount() error {`
`190`	`189`	`return nil`
`191`	`190`	`}`
`192`	`191`	`if err = mount.Unmount(shmPath); err != nil && !os.IsNotExist(err) {`
`193`		`- return errors.Wrapf(err, "umount %s", shmPath)`
	`192`	`+ return err`
`194`	`193`	`}`
`195`	`194`	`return nil`
`196`	`195`	`}`
`@@ -380,7 +379,8 @@ func (container *Container) DetachAndUnmount(volumeEventLog func(name, action st`
`380`	`379`
`381`	`380`	`for _, mountPath := range mountPaths {`
`382`	`381`	`if err := mount.Unmount(mountPath); err != nil {`
`383`		`- logrus.Warnf("%s unmountVolumes: Failed to do lazy umount for volume '%s': %v", container.ID, mountPath, err)`
	`382`	`+ logrus.WithError(err).WithField("container", container.ID).`
	`383`	`+ Warn("Unable to unmount")`
`384`	`384`	`}`
`385`	`385`	`}`
`386`	`386`	`return container.UnmountVolumes(volumeEventLog)`
Original file line number	Diff line number	Diff line change
`@@ -178,7 +178,7 @@ func (d *Driver) Cleanup() error {`
`178`	`178`	`}`
`179`	`179`
`180`	`180`	`if umountErr != nil {`
`181`		`- return errors.Wrapf(umountErr, "error unmounting %s", d.home)`
	`181`	`+ return umountErr`
`182`	`182`	`}`
`183`	`183`
`184`	`184`	`return nil`
Original file line number	Diff line number	Diff line change
`@@ -1200,7 +1200,7 @@ func (devices DeviceSet) growFS(info devInfo) error {`
`1200`	`1200`	`options = joinMountOptions(options, devices.mountOptions)`
`1201`	`1201`
`1202`	`1202`	`if err := mount.Mount(info.DevName(), fsMountPoint, devices.BaseDeviceFilesystem, options); err != nil {`
`1203`		`- return fmt.Errorf("Error mounting '%s' on '%s' (fstype='%s' options='%s'): %s\n%v", info.DevName(), fsMountPoint, devices.BaseDeviceFilesystem, options, err, string(dmesg.Dmesg(256)))`
	`1203`	`+ return errors.Wrapf(err, "Failed to mount; dmesg: %s", string(dmesg.Dmesg(256)))`
`1204`	`1204`	`}`
`1205`	`1205`
`1206`	`1206`	`defer unix.Unmount(fsMountPoint, unix.MNT_DETACH)`
`@@ -2381,7 +2381,7 @@ func (devices *DeviceSet) MountDevice(hash, path, mountLabel string) error {`
`2381`	`2381`	`options = joinMountOptions(options, label.FormatMountLabel("", mountLabel))`
`2382`	`2382`
`2383`	`2383`	`if err := mount.Mount(info.DevName(), path, fstype, options); err != nil {`
`2384`		`- return fmt.Errorf("devmapper: Error mounting '%s' on '%s' (fstype='%s' options='%s'): %s\n%v", info.DevName(), path, fstype, options, err, string(dmesg.Dmesg(256)))`
	`2384`	`+ return errors.Wrapf(err, "Failed to mount; dmesg: %s", string(dmesg.Dmesg(256)))`
`2385`	`2385`	`}`
`2386`	`2386`
`2387`	`2387`	`if fstype == "xfs" && devices.xfsNospaceRetries != "" {`
Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,6 @@ import (`
`16`	`16`	`"github.com/docker/docker/pkg/locker"`
`17`	`17`	`"github.com/docker/docker/pkg/mount"`
`18`	`18`	`"github.com/docker/go-units"`
`19`		`- "github.com/pkg/errors"`
`20`	`19`	`"github.com/sirupsen/logrus"`
`21`	`20`	`"golang.org/x/sys/unix"`
`22`	`21`	`)`
`@@ -129,11 +128,7 @@ func (d *Driver) Cleanup() error {`
`129`	`128`	`return err`
`130`	`129`	`}`
`131`	`130`
`132`		`- if umountErr != nil {`
`133`		`- return errors.Wrapf(umountErr, "error unmounting %s", d.home)`
`134`		`- }`
`135`		`-`
`136`		`- return nil`
	`131`	`+ return umountErr`
`137`	`132`	`}`
`138`	`133`
`139`	`134`	`// CreateReadWrite creates a layer that is writable for use as a container`
Original file line number	Diff line number	Diff line change
`@@ -19,6 +19,7 @@ import (`
`19`	`19`	`"github.com/docker/docker/pkg/parsers"`
`20`	`20`	`"github.com/mistifyio/go-zfs"`
`21`	`21`	`"github.com/opencontainers/selinux/go-selinux/label"`
	`22`	`+ "github.com/pkg/errors"`
`22`	`23`	`"github.com/sirupsen/logrus"`
`23`	`24`	`"golang.org/x/sys/unix"`
`24`	`25`	`)`
`@@ -390,7 +391,7 @@ func (d *Driver) Get(id, mountLabel string) (_ containerfs.ContainerFS, retErr e`
`390`	`391`	`}`
`391`	`392`
`392`	`393`	`if err := mount.Mount(filesystem, mountpoint, "zfs", options); err != nil {`
`393`		`- return nil, fmt.Errorf("error creating zfs mount of %s to %s: %v", filesystem, mountpoint, err)`
	`394`	`+ return nil, errors.Wrap(err, "error creating zfs mount")`
`394`	`395`	`}`
`395`	`396`
`396`	`397`	`// this could be our first mount after creation of the filesystem, and the root dir may still have root`
Original file line number	Diff line number	Diff line change
`@@ -11,8 +11,8 @@ package mount // import "github.com/docker/docker/pkg/mount"`
`11`	`11`	`import "C"`
`12`	`12`
`13`	`13`	`import (`
`14`		`- "fmt"`
`15`	`14`	`"strings"`
	`15`	`+ "syscall"`
`16`	`16`	`"unsafe"`
`17`	`17`	`)`
`18`	`18`
`@@ -47,8 +47,13 @@ func mount(device, target, mType string, flag uintptr, data string) error {`
`47`	`47`	`}`
`48`	`48`
`49`	`49`	`if errno := C.nmount(&rawOptions[0], C.uint(len(options)), C.int(flag)); errno != 0 {`
`50`		`- reason := C.GoString(C.strerror(*C.__error()))`
`51`		`- return fmt.Errorf("Failed to call nmount: %s", reason)`
	`50`	`+ return &mountError{`
	`51`	`+ op: "mount",`
	`52`	`+ source: device,`
	`53`	`+ target: target,`
	`54`	`+ flags: flag,`
	`55`	`+ err: syscall.Errno(errno),`
	`56`	`+ }`
`52`	`57`	`}`
`53`	`58`	`return nil`
`54`	`59`	`}`