Merge pull request #38068 from kolyshkin/err

More context for errors

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"
 )
@@ -174,8 +173,8 @@ func (container *Container) HasMountFor(path string) bool {
 	return false
 }
 
-// UnmountIpcMount uses the provided unmount function to unmount shm if it was mounted
-func (container *Container) UnmountIpcMount(unmount func(pth string) error) error {
+// UnmountIpcMount unmounts shm if it was mounted
+func (container *Container) UnmountIpcMount() error {
 	if container.HasMountFor("/dev/shm") {
 		return nil
 	}
@@ -189,10 +188,8 @@ func (container *Container) UnmountIpcMount(unmount func(pth string) error) erro
 	if shmPath == "" {
 		return nil
 	}
-	if err = unmount(shmPath); err != nil && !os.IsNotExist(err) {
-		if mounted, mErr := mount.Mounted(shmPath); mounted || mErr != nil {
-			return errors.Wrapf(err, "umount %s", shmPath)
-		}
+	if err = mount.Unmount(shmPath); err != nil && !os.IsNotExist(err) {
+		return err
 	}
 	return nil
 }
@@ -382,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)

container/container_windows.go

@@ -22,7 +22,7 @@ const (
 
 // UnmountIpcMount unmounts Ipc related mounts.
 // This is a NOOP on windows.
-func (container *Container) UnmountIpcMount(unmount func(pth string) error) error {
+func (container *Container) UnmountIpcMount() error {
 	return nil
 }
 

daemon/container_operations_unix.go

@@ -351,10 +351,6 @@ func killProcessDirectly(cntr *container.Container) error {
 	return nil
 }
 
-func detachMounted(path string) error {
-	return unix.Unmount(path, unix.MNT_DETACH)
-}
-
 func isLinkable(child *container.Container) bool {
 	// A container is linkable only if it belongs to the default network
 	_, ok := child.NetworkSettings.Networks[runconfig.DefaultDaemonNetworkMode().NetworkName()]

daemon/container_operations_windows.go

@@ -78,10 +78,6 @@ func (daemon *Daemon) mountVolumes(container *container.Container) error {
 	return nil
 }
 
-func detachMounted(path string) error {
-	return nil
-}
-
 func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) {
 	if len(c.SecretReferences) == 0 {
 		return nil

daemon/graphdriver/aufs/aufs.go

@@ -43,7 +43,7 @@ import (
 	"github.com/docker/docker/pkg/directory"
 	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/locker"
-	mountpk "github.com/docker/docker/pkg/mount"
+	"github.com/docker/docker/pkg/mount"
 	"github.com/docker/docker/pkg/system"
 	rsystem "github.com/opencontainers/runc/libcontainer/system"
 	"github.com/opencontainers/selinux/go-selinux/label"
@@ -598,7 +598,7 @@ func (a *Driver) Cleanup() error {
 			logger.Debugf("error unmounting %s: %s", m, err)
 		}
 	}
-	return mountpk.RecursiveUnmount(a.root)
+	return mount.RecursiveUnmount(a.root)
 }
 
 func (a *Driver) aufsMount(ro []string, rw, target, mountLabel string) (err error) {
@@ -632,14 +632,14 @@ func (a *Driver) aufsMount(ro []string, rw, target, mountLabel string) (err erro
 		opts += ",dirperm1"
 	}
 	data := label.FormatMountLabel(fmt.Sprintf("%s,%s", string(b[:bp]), opts), mountLabel)
-	if err = mount("none", target, "aufs", 0, data); err != nil {
+	if err = unix.Mount("none", target, "aufs", 0, data); err != nil {
 		return
 	}
 
 	for ; index < len(ro); index++ {
 		layer := fmt.Sprintf(":%s=ro+wh", ro[index])
 		data := label.FormatMountLabel(fmt.Sprintf("append%s", layer), mountLabel)
-		if err = mount("none", target, "aufs", unix.MS_REMOUNT, data); err != nil {
+		if err = unix.Mount("none", target, "aufs", unix.MS_REMOUNT, data); err != nil {
 			return
 		}
 	}
@@ -666,7 +666,7 @@ func useDirperm() bool {
 		defer os.RemoveAll(union)
 
 		opts := fmt.Sprintf("br:%s,dirperm1,xino=/dev/shm/aufs.xino", base)
-		if err := mount("none", union, "aufs", 0, opts); err != nil {
+		if err := unix.Mount("none", union, "aufs", 0, opts); err != nil {
 			return
 		}
 		enableDirperm = true

daemon/graphdriver/aufs/mount_linux.go

@@ -1,7 +0,0 @@
-package aufs // import "github.com/docker/docker/daemon/graphdriver/aufs"
-
-import "golang.org/x/sys/unix"
-
-func mount(source string, target string, fstype string, flags uintptr, data string) error {
-	return unix.Mount(source, target, fstype, flags, data)
-}

daemon/graphdriver/aufs/mount_unsupported.go

@@ -1,12 +0,0 @@
-// +build !linux
-
-package aufs // import "github.com/docker/docker/daemon/graphdriver/aufs"
-
-import "errors"
-
-// MsRemount declared to specify a non-linux system mount.
-const MsRemount = 0
-
-func mount(source string, target string, fstype string, flags uintptr, data string) (err error) {
-	return errors.New("mount is not implemented on this platform")
-}

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

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 != "" {

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

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

daemon/start.go

@@ -216,7 +216,7 @@ func (daemon *Daemon) containerStart(container *container.Container, checkpoint
 func (daemon *Daemon) Cleanup(container *container.Container) {
 	daemon.releaseNetwork(container)
 
-	if err := container.UnmountIpcMount(detachMounted); err != nil {
+	if err := container.UnmountIpcMount(); err != nil {
 		logrus.Warnf("%s cleanup: failed to unmount IPC: %s", container.ID, err)
 	}
 

pkg/mount/mount.go

@@ -2,12 +2,46 @@ package mount // import "github.com/docker/docker/pkg/mount"
 
 import (
 	"sort"
+	"strconv"
 	"strings"
-	"syscall"
 
 	"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
@@ -89,12 +123,7 @@ func ForceMount(device, target, mType, options string) error {
 // Unmount lazily unmounts a filesystem on supported platforms, otherwise
 // does a normal unmount.
 func Unmount(target string) error {
-	err := unmount(target, mntDetach)
-	if err == syscall.EINVAL {
-		// ignore "not mounted" error
-		err = nil
-	}
-	return err
+	return unmount(target, mntDetach)
 }
 
 // RecursiveUnmount unmounts the target and all mounts underneath, starting with
@@ -114,25 +143,14 @@ func RecursiveUnmount(target string) error {
 		logrus.Debugf("Trying to unmount %s", m.Mountpoint)
 		err = unmount(m.Mountpoint, mntDetach)
 		if err != nil {
-			// If the error is EINVAL either this whole package is wrong (invalid flags passed to unmount(2)) or this is
-			// not a mountpoint (which is ok in this case).
-			// Meanwhile calling `Mounted()` is very expensive.
-			//
-			// We've purposefully used `syscall.EINVAL` here instead of `unix.EINVAL` to avoid platform branching
-			// Since `EINVAL` is defined for both Windows and Linux in the `syscall` package (and other platforms),
-			//   this is nicer than defining a custom value that we can refer to in each platform file.
-			if err == syscall.EINVAL {
-				continue
-			}
-			if i == len(mounts)-1 {
+			if i == len(mounts)-1 { // last mount
 				if mounted, e := Mounted(m.Mountpoint); e != nil || mounted {
 					return err
 				}
-				continue
+			} else {
+				// This is some submount, we can ignore this error for now, the final unmount will fail if this is a real problem
+				logrus.WithError(err).Warnf("Failed to unmount submount %s", m.Mountpoint)
 			}
-			// This is some submount, we can ignore this error for now, the final unmount will fail if this is a real problem
-			logrus.WithError(err).Warnf("Failed to unmount submount %s", m.Mountpoint)
-			continue
 		}
 
 		logrus.Debugf("Unmounted %s", m.Mountpoint)

pkg/mount/mounter_freebsd.go

@@ -11,11 +11,9 @@ package mount // import "github.com/docker/docker/pkg/mount"
 import "C"
 
 import (
-	"fmt"
 	"strings"
+	"syscall"
 	"unsafe"
-
-	"golang.org/x/sys/unix"
 )
 
 func allocateIOVecs(options []string) []C.struct_iovec {
@@ -49,12 +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
 }
-
-func unmount(target string, flag int) error {
-	return unix.Unmount(target, flag)
-}

pkg/mount/mounter_linux.go

@@ -33,25 +33,42 @@ 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
 }
-
-func unmount(target string, flag int) error {
-	return unix.Unmount(target, flag)
-}

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)

pkg/mount/unmount_unix.go

@@ -0,0 +1,22 @@
+// +build !windows
+
+package mount // import "github.com/docker/docker/pkg/mount"
+
+import "golang.org/x/sys/unix"
+
+func unmount(target string, flags int) error {
+	err := unix.Unmount(target, flags)
+	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.
+		return nil
+	}
+
+	return &mountError{
+		op:     "umount",
+		target: target,
+		flags:  uintptr(flags),
+		err:    err,
+	}
+}

pkg/system/lstat_unix.go

@@ -3,6 +3,7 @@
 package system // import "github.com/docker/docker/pkg/system"
 
 import (
+	"os"
 	"syscall"
 )
 
@@ -13,7 +14,7 @@ import (
 func Lstat(path string) (*StatT, error) {
 	s := &syscall.Stat_t{}
 	if err := syscall.Lstat(path, s); err != nil {
-		return nil, err
+		return nil, &os.PathError{"Lstat", path, err}
 	}
 	return fromStatT(s)
 }

pkg/system/stat_unix.go

@@ -3,6 +3,7 @@
 package system // import "github.com/docker/docker/pkg/system"
 
 import (
+	"os"
 	"syscall"
 )
 
@@ -59,7 +60,7 @@ func (s StatT) IsDir() bool {
 func Stat(path string) (*StatT, error) {
 	s := &syscall.Stat_t{}
 	if err := syscall.Stat(path, s); err != nil {
-		return nil, err
+		return nil, &os.PathError{"Stat", path, err}
 	}
 	return fromStatT(s)
 }

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)

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

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) {