Merge pull request #5676 from kolyshkin/volume-flags-alt

Fix/improve pkg/storage.InitFSMounts

libpod/options.go

@@ -1334,7 +1334,7 @@ func WithNamedVolumes(volumes []*ContainerNamedVolume) CtrCreateOption {
 			}
 			destinations[vol.Dest] = true
 
-			mountOpts, err := util.ProcessOptions(vol.Options, false, nil)
+			mountOpts, err := util.ProcessOptions(vol.Options, false, "")
 			if err != nil {
 				return errors.Wrapf(err, "error processing options for named volume %q mounted at %q", vol.Name, vol.Dest)
 			}

pkg/spec/spec.go

@@ -381,11 +381,9 @@ func (config *CreateConfig) createConfigToOCISpec(runtime *libpod.Runtime, userM
 	// BIND MOUNTS
 	configSpec.Mounts = SupercedeUserMounts(userMounts, configSpec.Mounts)
 	// Process mounts to ensure correct options
-	finalMounts, err := InitFSMounts(configSpec.Mounts)
+	if err := InitFSMounts(configSpec.Mounts); err != nil {
-	if err != nil {
 		return nil, err
 	}
-	configSpec.Mounts = finalMounts
 
 	// BLOCK IO
 	blkio, err := config.CreateBlockIO()

pkg/spec/storage.go

@@ -10,7 +10,6 @@ import (
 	"github.com/containers/buildah/pkg/parse"
 	"github.com/containers/libpod/libpod"
 	"github.com/containers/libpod/pkg/util"
-	pmount "github.com/containers/storage/pkg/mount"
 	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -855,75 +854,22 @@ func SupercedeUserMounts(mounts []spec.Mount, configMount []spec.Mount) []spec.M
 }
 
 // Ensure mount options on all mounts are correct
-func InitFSMounts(inputMounts []spec.Mount) ([]spec.Mount, error) {
+func InitFSMounts(mounts []spec.Mount) error {
-	// We need to look up mounts so we can figure out the proper mount flags
+	for i, m := range mounts {
-	// to apply.
+		switch {
-	systemMounts, err := pmount.GetMounts()
+		case m.Type == TypeBind:
-	if err != nil {
+			opts, err := util.ProcessOptions(m.Options, false, m.Source)
-		return nil, errors.Wrapf(err, "error retrieving system mounts to look up mount options")
-	}
-
-	// TODO: We probably don't need to re-build the mounts array
-	var mounts []spec.Mount
-	for _, m := range inputMounts {
-		if m.Type == TypeBind {
-			baseMnt, err := findMount(m.Destination, systemMounts)
 			if err != nil {
-				return nil, errors.Wrapf(err, "error looking up mountpoint for mount %s", m.Destination)
+				return err
 			}
-			var noexec, nosuid, nodev bool
+			mounts[i].Options = opts
-			for _, baseOpt := range strings.Split(baseMnt.Opts, ",") {
+		case m.Type == TypeTmpfs && filepath.Clean(m.Destination) != "/dev":
-				switch baseOpt {
+			opts, err := util.ProcessOptions(m.Options, true, "")
-				case "noexec":
-					noexec = true
-				case "nosuid":
-					nosuid = true
-				case "nodev":
-					nodev = true
-				}
-			}
-
-			defaultMountOpts := new(util.DefaultMountOptions)
-			defaultMountOpts.Noexec = noexec
-			defaultMountOpts.Nosuid = nosuid
-			defaultMountOpts.Nodev = nodev
-
-			opts, err := util.ProcessOptions(m.Options, false, defaultMountOpts)
 			if err != nil {
-				return nil, err
+				return err
 			}
-			m.Options = opts
+			mounts[i].Options = opts
 		}
-		if m.Type == TypeTmpfs && filepath.Clean(m.Destination) != "/dev" {
-			opts, err := util.ProcessOptions(m.Options, true, nil)
-			if err != nil {
-				return nil, err
-			}
-			m.Options = opts
-		}
-
-		mounts = append(mounts, m)
 	}
-	return mounts, nil
+	return nil
-}
-
-// TODO: We could make this a bit faster by building a tree of the mountpoints
-// and traversing it to identify the correct mount.
-func findMount(target string, mounts []*pmount.Info) (*pmount.Info, error) {
-	var err error
-	target, err = filepath.Abs(target)
-	if err != nil {
-		return nil, errors.Wrapf(err, "cannot resolve %s", target)
-	}
-	var bestSoFar *pmount.Info
-	for _, i := range mounts {
-		if bestSoFar != nil && len(bestSoFar.Mountpoint) > len(i.Mountpoint) {
-			// Won't be better than what we have already found
-			continue
-		}
-		if strings.HasPrefix(target, i.Mountpoint) {
-			bestSoFar = i
-		}
-	}
-	return bestSoFar, nil
 }

pkg/specgen/oci.go

@@ -215,11 +215,9 @@ func (s *SpecGenerator) toOCISpec(rt *libpod.Runtime, newImage *image.Image) (*s
 	// BIND MOUNTS
 	configSpec.Mounts = createconfig.SupercedeUserMounts(s.Mounts, configSpec.Mounts)
 	// Process mounts to ensure correct options
-	finalMounts, err := createconfig.InitFSMounts(configSpec.Mounts)
+	if err := createconfig.InitFSMounts(configSpec.Mounts); err != nil {
-	if err != nil {
 		return nil, err
 	}
-	configSpec.Mounts = finalMounts
 
 	// Add annotations
 	if configSpec.Annotations == nil {

pkg/util/mountOpts.go

@@ -13,19 +13,17 @@ var (
 	ErrDupeMntOption = errors.Errorf("duplicate mount option passed")
 )
 
-// DefaultMountOptions sets default mount options for ProcessOptions.
+type defaultMountOptions struct {
-type DefaultMountOptions struct {
+	noexec bool
-	Noexec bool
+	nosuid bool
-	Nosuid bool
+	nodev  bool
-	Nodev  bool
 }
 
 // ProcessOptions parses the options for a bind or tmpfs mount and ensures that
 // they are sensible and follow convention. The isTmpfs variable controls
 // whether extra, tmpfs-specific options will be allowed.
-// The defaults variable controls default mount options that will be set. If it
+// The sourcePath variable, if not empty, contains a bind mount source.
-// is not included, they will be set unconditionally.
+func ProcessOptions(options []string, isTmpfs bool, sourcePath string) ([]string, error) {
-func ProcessOptions(options []string, isTmpfs bool, defaults *DefaultMountOptions) ([]string, error) {
 	var (
 		foundWrite, foundSize, foundProp, foundMode, foundExec, foundSuid, foundDev, foundCopyUp, foundBind, foundZ bool
 	)
@@ -122,13 +120,17 @@ func ProcessOptions(options []string, isTmpfs bool, defaults *DefaultMountOption
 	if !foundProp {
 		newOptions = append(newOptions, "rprivate")
 	}
-	if !foundExec && (defaults == nil || defaults.Noexec) {
+	defaults, err := getDefaultMountOptions(sourcePath)
+	if err != nil {
+		return nil, err
+	}
+	if !foundExec && defaults.noexec {
 		newOptions = append(newOptions, "noexec")
 	}
-	if !foundSuid && (defaults == nil || defaults.Nosuid) {
+	if !foundSuid && defaults.nosuid {
 		newOptions = append(newOptions, "nosuid")
 	}
-	if !foundDev && (defaults == nil || defaults.Nodev) {
+	if !foundDev && defaults.nodev {
 		newOptions = append(newOptions, "nodev")
 	}
 	if isTmpfs && !foundCopyUp {

pkg/util/mountOpts_linux.go

@@ -0,0 +1,23 @@
+package util
+
+import (
+	"os"
+
+	"golang.org/x/sys/unix"
+)
+
+func getDefaultMountOptions(path string) (defaultMountOptions, error) {
+	opts := defaultMountOptions{true, true, true}
+	if path == "" {
+		return opts, nil
+	}
+	var statfs unix.Statfs_t
+	if e := unix.Statfs(path, &statfs); e != nil {
+		return opts, &os.PathError{Op: "statfs", Path: path, Err: e}
+	}
+	opts.nodev = (statfs.Flags&unix.MS_NODEV == unix.MS_NODEV)
+	opts.noexec = (statfs.Flags&unix.MS_NOEXEC == unix.MS_NOEXEC)
+	opts.nosuid = (statfs.Flags&unix.MS_NOSUID == unix.MS_NOSUID)
+
+	return opts, nil
+}

pkg/util/mountOpts_other.go

@@ -0,0 +1,7 @@
+// +build !linux
+
+package util
+
+func getDefaultMountOptions(path string) (opts defaultMountOptions, err error) {
+	return
+}