Browse Source

Add new `HostConfig` field, `Mounts`.

`Mounts` allows users to specify in a much safer way the volumes they
want to use in the container.
This replaces `Binds` and `Volumes`, which both still exist, but
`Mounts` and `Binds`/`Volumes` are exclussive.
The CLI will continue to use `Binds` and `Volumes` due to concerns with
parsing the volume specs on the client side and cross-platform support
(for now).

The new API follows exactly the services mount API.

Example usage of `Mounts`:

```
$ curl -XPOST localhost:2375/containers/create -d '{
  "Image": "alpine:latest",
  "HostConfig": {
    "Mounts": [{
      "Type": "Volume",
      "Target": "/foo"
      },{
      "Type": "bind",
      "Source": "/var/run/docker.sock",
      "Target": "/var/run/docker.sock",
      },{
      "Type": "volume",
      "Name": "important_data",
      "Target": "/var/data",
      "ReadOnly": true,
      "VolumeOptions": {
	"DriverConfig": {
	  Name: "awesomeStorage",
	  Options: {"size": "10m"},
	  Labels: {"some":"label"}
	}
      }]
    }
}'
```

There are currently 2 types of mounts:

  - **bind**: Paths on the host that get mounted into the
    container. Paths must exist prior to creating the container.
  - **volume**: Volumes that persist after the
    container is removed.

Not all fields are available in each type, and validation is done to
ensure these fields aren't mixed up between types.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Brian Goff 9 years ago
parent
commit
fc7b904dce

+ 2 - 0
container/container.go

@@ -17,6 +17,7 @@ import (
 
 
 	"github.com/Sirupsen/logrus"
 	"github.com/Sirupsen/logrus"
 	containertypes "github.com/docker/docker/api/types/container"
 	containertypes "github.com/docker/docker/api/types/container"
+	mounttypes "github.com/docker/docker/api/types/mount"
 	networktypes "github.com/docker/docker/api/types/network"
 	networktypes "github.com/docker/docker/api/types/network"
 	"github.com/docker/docker/daemon/exec"
 	"github.com/docker/docker/daemon/exec"
 	"github.com/docker/docker/daemon/logger"
 	"github.com/docker/docker/daemon/logger"
@@ -551,6 +552,7 @@ func (container *Container) ShouldRestart() bool {
 // AddMountPointWithVolume adds a new mount point configured with a volume to the container.
 // AddMountPointWithVolume adds a new mount point configured with a volume to the container.
 func (container *Container) AddMountPointWithVolume(destination string, vol volume.Volume, rw bool) {
 func (container *Container) AddMountPointWithVolume(destination string, vol volume.Volume, rw bool) {
 	container.MountPoints[destination] = &volume.MountPoint{
 	container.MountPoints[destination] = &volume.MountPoint{
+		Type:        mounttypes.TypeVolume,
 		Name:        vol.Name(),
 		Name:        vol.Name(),
 		Driver:      vol.DriverName(),
 		Driver:      vol.DriverName(),
 		Destination: destination,
 		Destination: destination,

+ 6 - 1
daemon/create_unix.go

@@ -9,6 +9,7 @@ import (
 
 
 	"github.com/Sirupsen/logrus"
 	"github.com/Sirupsen/logrus"
 	containertypes "github.com/docker/docker/api/types/container"
 	containertypes "github.com/docker/docker/api/types/container"
+	mounttypes "github.com/docker/docker/api/types/mount"
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/pkg/stringid"
 	"github.com/docker/docker/pkg/stringid"
 	"github.com/opencontainers/runc/libcontainer/label"
 	"github.com/opencontainers/runc/libcontainer/label"
@@ -63,7 +64,11 @@ func (daemon *Daemon) createContainerPlatformSpecificSettings(container *contain
 // this is only called when the container is created.
 // this is only called when the container is created.
 func (daemon *Daemon) populateVolumes(c *container.Container) error {
 func (daemon *Daemon) populateVolumes(c *container.Container) error {
 	for _, mnt := range c.MountPoints {
 	for _, mnt := range c.MountPoints {
-		if !mnt.CopyData || mnt.Volume == nil {
+		if mnt.Volume == nil {
+			continue
+		}
+
+		if mnt.Type != mounttypes.TypeVolume || !mnt.CopyData {
 			continue
 			continue
 		}
 		}
 
 

+ 1 - 1
daemon/create_windows.go

@@ -18,7 +18,7 @@ func (daemon *Daemon) createContainerPlatformSpecificSettings(container *contain
 
 
 	for spec := range config.Volumes {
 	for spec := range config.Volumes {
 
 
-		mp, err := volume.ParseMountSpec(spec, hostConfig.VolumeDriver)
+		mp, err := volume.ParseMountRaw(spec, hostConfig.VolumeDriver)
 		if err != nil {
 		if err != nil {
 			return fmt.Errorf("Unrecognised volume spec: %v", err)
 			return fmt.Errorf("Unrecognised volume spec: %v", err)
 		}
 		}

+ 1 - 0
daemon/inspect_unix.go

@@ -68,6 +68,7 @@ func addMountPoints(container *container.Container) []types.MountPoint {
 	mountPoints := make([]types.MountPoint, 0, len(container.MountPoints))
 	mountPoints := make([]types.MountPoint, 0, len(container.MountPoints))
 	for _, m := range container.MountPoints {
 	for _, m := range container.MountPoints {
 		mountPoints = append(mountPoints, types.MountPoint{
 		mountPoints = append(mountPoints, types.MountPoint{
+			Type:        m.Type,
 			Name:        m.Name,
 			Name:        m.Name,
 			Source:      m.Path(),
 			Source:      m.Path(),
 			Destination: m.Destination,
 			Destination: m.Destination,

+ 1 - 0
daemon/inspect_windows.go

@@ -16,6 +16,7 @@ func addMountPoints(container *container.Container) []types.MountPoint {
 	mountPoints := make([]types.MountPoint, 0, len(container.MountPoints))
 	mountPoints := make([]types.MountPoint, 0, len(container.MountPoints))
 	for _, m := range container.MountPoints {
 	for _, m := range container.MountPoints {
 		mountPoints = append(mountPoints, types.MountPoint{
 		mountPoints = append(mountPoints, types.MountPoint{
+			Type:        m.Type,
 			Name:        m.Name,
 			Name:        m.Name,
 			Source:      m.Path(),
 			Source:      m.Path(),
 			Destination: m.Destination,
 			Destination: m.Destination,

+ 1 - 1
daemon/mounts.go

@@ -27,7 +27,7 @@ func (daemon *Daemon) removeMountPoints(container *container.Container, rm bool)
 		if rm {
 		if rm {
 			// Do not remove named mountpoints
 			// Do not remove named mountpoints
 			// these are mountpoints specified like `docker run -v <name>:/foo`
 			// these are mountpoints specified like `docker run -v <name>:/foo`
-			if m.Named {
+			if m.Spec.Source != "" {
 				continue
 				continue
 			}
 			}
 			err := daemon.volumes.Remove(m.Volume)
 			err := daemon.volumes.Remove(m.Volume)

+ 54 - 7
daemon/volumes.go

@@ -9,8 +9,11 @@ import (
 
 
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types"
 	containertypes "github.com/docker/docker/api/types/container"
 	containertypes "github.com/docker/docker/api/types/container"
+	mounttypes "github.com/docker/docker/api/types/mount"
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/container"
+	dockererrors "github.com/docker/docker/errors"
 	"github.com/docker/docker/volume"
 	"github.com/docker/docker/volume"
+	"github.com/opencontainers/runc/libcontainer/label"
 )
 )
 
 
 var (
 var (
@@ -106,7 +109,8 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo
 				Driver:      m.Driver,
 				Driver:      m.Driver,
 				Destination: m.Destination,
 				Destination: m.Destination,
 				Propagation: m.Propagation,
 				Propagation: m.Propagation,
-				Named:       m.Named,
+				Spec:        m.Spec,
+				CopyData:    false,
 			}
 			}
 
 
 			if len(cp.Source) == 0 {
 			if len(cp.Source) == 0 {
@@ -123,18 +127,18 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo
 
 
 	// 3. Read bind mounts
 	// 3. Read bind mounts
 	for _, b := range hostConfig.Binds {
 	for _, b := range hostConfig.Binds {
-		// #10618
-		bind, err := volume.ParseMountSpec(b, hostConfig.VolumeDriver)
+		bind, err := volume.ParseMountRaw(b, hostConfig.VolumeDriver)
 		if err != nil {
 		if err != nil {
 			return err
 			return err
 		}
 		}
 
 
+		// #10618
 		_, tmpfsExists := hostConfig.Tmpfs[bind.Destination]
 		_, tmpfsExists := hostConfig.Tmpfs[bind.Destination]
 		if binds[bind.Destination] || tmpfsExists {
 		if binds[bind.Destination] || tmpfsExists {
 			return fmt.Errorf("Duplicate mount point '%s'", bind.Destination)
 			return fmt.Errorf("Duplicate mount point '%s'", bind.Destination)
 		}
 		}
 
 
-		if len(bind.Name) > 0 {
+		if bind.Type == mounttypes.TypeVolume {
 			// create the volume
 			// create the volume
 			v, err := daemon.volumes.CreateWithRef(bind.Name, bind.Driver, container.ID, nil, nil)
 			v, err := daemon.volumes.CreateWithRef(bind.Name, bind.Driver, container.ID, nil, nil)
 			if err != nil {
 			if err != nil {
@@ -144,9 +148,8 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo
 			bind.Source = v.Path()
 			bind.Source = v.Path()
 			// bind.Name is an already existing volume, we need to use that here
 			// bind.Name is an already existing volume, we need to use that here
 			bind.Driver = v.DriverName()
 			bind.Driver = v.DriverName()
-			bind.Named = true
-			if bind.Driver == "local" {
-				bind = setBindModeIfNull(bind)
+			if bind.Driver == volume.DefaultDriverName {
+				setBindModeIfNull(bind)
 			}
 			}
 		}
 		}
 
 
@@ -154,6 +157,50 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo
 		mountPoints[bind.Destination] = bind
 		mountPoints[bind.Destination] = bind
 	}
 	}
 
 
+	for _, cfg := range hostConfig.Mounts {
+		mp, err := volume.ParseMountSpec(cfg)
+		if err != nil {
+			return dockererrors.NewBadRequestError(err)
+		}
+
+		if binds[mp.Destination] {
+			return fmt.Errorf("Duplicate mount point '%s'", cfg.Target)
+		}
+
+		if mp.Type == mounttypes.TypeVolume {
+			var v volume.Volume
+			if cfg.VolumeOptions != nil {
+				var driverOpts map[string]string
+				if cfg.VolumeOptions.DriverConfig != nil {
+					driverOpts = cfg.VolumeOptions.DriverConfig.Options
+				}
+				v, err = daemon.volumes.CreateWithRef(mp.Name, mp.Driver, container.ID, driverOpts, cfg.VolumeOptions.Labels)
+			} else {
+				v, err = daemon.volumes.CreateWithRef(mp.Name, mp.Driver, container.ID, nil, nil)
+			}
+			if err != nil {
+				return err
+			}
+
+			if err := label.Relabel(mp.Source, container.MountLabel, false); err != nil {
+				return err
+			}
+			mp.Volume = v
+			mp.Name = v.Name()
+			mp.Driver = v.DriverName()
+
+			// only use the cached path here since getting the path is not neccessary right now and calling `Path()` may be slow
+			if cv, ok := v.(interface {
+				CachedPath() string
+			}); ok {
+				mp.Source = cv.CachedPath()
+			}
+		}
+
+		binds[mp.Destination] = true
+		mountPoints[mp.Destination] = mp
+	}
+
 	container.Lock()
 	container.Lock()
 
 
 	// 4. Cleanup old volumes that are about to be reassigned.
 	// 4. Cleanup old volumes that are about to be reassigned.

+ 1 - 2
daemon/volumes_unix.go

@@ -85,11 +85,10 @@ func sortMounts(m []container.Mount) []container.Mount {
 // setBindModeIfNull is platform specific processing to ensure the
 // setBindModeIfNull is platform specific processing to ensure the
 // shared mode is set to 'z' if it is null. This is called in the case
 // shared mode is set to 'z' if it is null. This is called in the case
 // of processing a named volume and not a typical bind.
 // of processing a named volume and not a typical bind.
-func setBindModeIfNull(bind *volume.MountPoint) *volume.MountPoint {
+func setBindModeIfNull(bind *volume.MountPoint) {
 	if bind.Mode == "" {
 	if bind.Mode == "" {
 		bind.Mode = "z"
 		bind.Mode = "z"
 	}
 	}
-	return bind
 }
 }
 
 
 // migrateVolume links the contents of a volume created pre Docker 1.7
 // migrateVolume links the contents of a volume created pre Docker 1.7

+ 2 - 2
daemon/volumes_windows.go

@@ -46,6 +46,6 @@ func (daemon *Daemon) setupMounts(c *container.Container) ([]container.Mount, er
 
 
 // setBindModeIfNull is platform specific processing which is a no-op on
 // setBindModeIfNull is platform specific processing which is a no-op on
 // Windows.
 // Windows.
-func setBindModeIfNull(bind *volume.MountPoint) *volume.MountPoint {
-	return bind
+func setBindModeIfNull(bind *volume.MountPoint) {
+	return
 }
 }

+ 1 - 0
docs/reference/api/docker_remote_api.md

@@ -122,6 +122,7 @@ This section lists each version from latest to oldest.  Each listing includes a
 * `DELETE /volumes/(name)` now accepts a `force` query parameter to force removal of volumes that were already removed out of band by the volume driver plugin.
 * `DELETE /volumes/(name)` now accepts a `force` query parameter to force removal of volumes that were already removed out of band by the volume driver plugin.
 * `POST /containers/create/` and `POST /containers/(name)/update` now validates restart policies.
 * `POST /containers/create/` and `POST /containers/(name)/update` now validates restart policies.
 * `POST /containers/create` now validates IPAMConfig in NetworkingConfig, and returns error for invalid IPv4 and IPv6 addresses (`--ip` and `--ip6` in `docker create/run`).
 * `POST /containers/create` now validates IPAMConfig in NetworkingConfig, and returns error for invalid IPv4 and IPv6 addresses (`--ip` and `--ip6` in `docker create/run`).
+* `POST /containers/create` now takes a `Mounts` field in `HostConfig` which replaces `Binds` and `Volumes`. *note*: `Binds` and `Volumes` are still available but are exclusive with `Mounts`
 
 
 ### v1.24 API changes
 ### v1.24 API changes
 
 

+ 4 - 2
docs/reference/api/docker_remote_api_v1.24.md

@@ -334,7 +334,8 @@ Create a container
              "StorageOpt": {},
              "StorageOpt": {},
              "CgroupParent": "",
              "CgroupParent": "",
              "VolumeDriver": "",
              "VolumeDriver": "",
-             "ShmSize": 67108864
+             "ShmSize": 67108864,
+             "Mounts": []
           },
           },
           "NetworkingConfig": {
           "NetworkingConfig": {
               "EndpointsConfig": {
               "EndpointsConfig": {
@@ -610,7 +611,8 @@ Return low-level information on the container `id`
 			"VolumesFrom": null,
 			"VolumesFrom": null,
 			"Ulimits": [{}],
 			"Ulimits": [{}],
 			"VolumeDriver": "",
 			"VolumeDriver": "",
-			"ShmSize": 67108864
+			"ShmSize": 67108864,
+			"Mounts": []
 		},
 		},
 		"HostnamePath": "/var/lib/docker/containers/ba033ac4401106a3b513bc9d639eee123ad78ca3616b921167cd74b20e25ed39/hostname",
 		"HostnamePath": "/var/lib/docker/containers/ba033ac4401106a3b513bc9d639eee123ad78ca3616b921167cd74b20e25ed39/hostname",
 		"HostsPath": "/var/lib/docker/containers/ba033ac4401106a3b513bc9d639eee123ad78ca3616b921167cd74b20e25ed39/hosts",
 		"HostsPath": "/var/lib/docker/containers/ba033ac4401106a3b513bc9d639eee123ad78ca3616b921167cd74b20e25ed39/hosts",

+ 18 - 0
docs/reference/api/docker_remote_api_v1.25.md

@@ -486,6 +486,24 @@ Create a container
     -   **CgroupParent** - Path to `cgroups` under which the container's `cgroup` is created. If the path is not absolute, the path is considered to be relative to the `cgroups` path of the init process. Cgroups are created if they do not already exist.
     -   **CgroupParent** - Path to `cgroups` under which the container's `cgroup` is created. If the path is not absolute, the path is considered to be relative to the `cgroups` path of the init process. Cgroups are created if they do not already exist.
     -   **VolumeDriver** - Driver that this container users to mount volumes.
     -   **VolumeDriver** - Driver that this container users to mount volumes.
     -   **ShmSize** - Size of `/dev/shm` in bytes. The size must be greater than 0.  If omitted the system uses 64MB.
     -   **ShmSize** - Size of `/dev/shm` in bytes. The size must be greater than 0.  If omitted the system uses 64MB.
+    -   **Mounts** – Specification for mounts to be added to the container.
+        - **Target** – Container path.
+        - **Source** – Mount source (e.g. a volume name, a host path).
+        - **Type** – The mount type (`bind`, or `volume`).
+          Available types (for the `Type` field):
+          - **bind** - Mounts a file or directory from the host into the container. Must exist prior to creating the container.
+          - **volume** - Creates a volume with the given name and options (or uses a pre-existing volume with the same name and options). These are **not** removed when the container is removed.
+        - **ReadOnly** – A boolean indicating whether the mount should be read-only.
+        - **BindOptions** - Optional configuration for the `bind` type.
+          - **Propagation** – A propagation mode with the value `[r]private`, `[r]shared`, or `[r]slave`.
+        - **VolumeOptions** – Optional configuration for the `volume` type.
+            - **NoCopy** – A boolean indicating if volume should be
+              populated with the data from the target. (Default false)
+            - **Labels** – User-defined name and labels for the volume as key/value pairs: `{"name": "value"}`
+            - **DriverConfig** – Map of driver-specific options.
+              - **Name** - Name of the driver to use to create the volume.
+              - **Options** - key/value map of driver specific options.
+
 
 
 **Query parameters**:
 **Query parameters**:
 
 

+ 215 - 0
integration-cli/docker_api_containers_test.go

@@ -6,10 +6,12 @@ import (
 	"encoding/json"
 	"encoding/json"
 	"fmt"
 	"fmt"
 	"io"
 	"io"
+	"io/ioutil"
 	"net/http"
 	"net/http"
 	"net/http/httputil"
 	"net/http/httputil"
 	"net/url"
 	"net/url"
 	"os"
 	"os"
+	"path/filepath"
 	"regexp"
 	"regexp"
 	"strconv"
 	"strconv"
 	"strings"
 	"strings"
@@ -17,10 +19,14 @@ import (
 
 
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types"
 	containertypes "github.com/docker/docker/api/types/container"
 	containertypes "github.com/docker/docker/api/types/container"
+	mounttypes "github.com/docker/docker/api/types/mount"
 	networktypes "github.com/docker/docker/api/types/network"
 	networktypes "github.com/docker/docker/api/types/network"
 	"github.com/docker/docker/pkg/integration"
 	"github.com/docker/docker/pkg/integration"
 	"github.com/docker/docker/pkg/integration/checker"
 	"github.com/docker/docker/pkg/integration/checker"
+	"github.com/docker/docker/pkg/ioutils"
+	"github.com/docker/docker/pkg/mount"
 	"github.com/docker/docker/pkg/stringid"
 	"github.com/docker/docker/pkg/stringid"
+	"github.com/docker/docker/volume"
 	"github.com/go-check/check"
 	"github.com/go-check/check"
 )
 )
 
 
@@ -1525,3 +1531,212 @@ func (s *DockerSuite) TestContainerApiStatsWithNetworkDisabled(c *check.C) {
 		c.Assert(dec.Decode(&s), checker.IsNil)
 		c.Assert(dec.Decode(&s), checker.IsNil)
 	}
 	}
 }
 }
+
+func (s *DockerSuite) TestContainersApiCreateMountsValidation(c *check.C) {
+	type m mounttypes.Mount
+	type hc struct{ Mounts []m }
+	type cfg struct {
+		Image      string
+		HostConfig hc
+	}
+	type testCase struct {
+		config cfg
+		status int
+		msg    string
+	}
+
+	prefix, slash := getPrefixAndSlashFromDaemonPlatform()
+	destPath := prefix + slash + "foo"
+	notExistPath := prefix + slash + "notexist"
+
+	cases := []testCase{
+		{cfg{Image: "busybox", HostConfig: hc{Mounts: []m{{Type: "notreal", Target: destPath}}}}, http.StatusBadRequest, "mount type unknown"},
+		{cfg{Image: "busybox", HostConfig: hc{Mounts: []m{{Type: "bind"}}}}, http.StatusBadRequest, "Target must not be empty"},
+		{cfg{Image: "busybox", HostConfig: hc{Mounts: []m{{Type: "bind", Target: destPath}}}}, http.StatusBadRequest, "Source must not be empty"},
+		{cfg{Image: "busybox", HostConfig: hc{Mounts: []m{{Type: "bind", Source: notExistPath, Target: destPath}}}}, http.StatusBadRequest, "bind source path does not exist"},
+		{cfg{Image: "busybox", HostConfig: hc{Mounts: []m{{Type: "volume"}}}}, http.StatusBadRequest, "Target must not be empty"},
+		{cfg{Image: "busybox", HostConfig: hc{Mounts: []m{{Type: "volume", Source: "hello", Target: destPath}}}}, http.StatusCreated, ""},
+		{cfg{Image: "busybox", HostConfig: hc{Mounts: []m{{Type: "volume", Source: "hello2", Target: destPath, VolumeOptions: &mounttypes.VolumeOptions{DriverConfig: &mounttypes.Driver{Name: "local"}}}}}}, http.StatusCreated, ""},
+	}
+
+	if SameHostDaemon.Condition() {
+		tmpDir, err := ioutils.TempDir("", "test-mounts-api")
+		c.Assert(err, checker.IsNil)
+		defer os.RemoveAll(tmpDir)
+		cases = append(cases, []testCase{
+			{cfg{Image: "busybox", HostConfig: hc{Mounts: []m{{Type: "bind", Source: tmpDir, Target: destPath}}}}, http.StatusCreated, ""},
+			{cfg{Image: "busybox", HostConfig: hc{Mounts: []m{{Type: "bind", Source: tmpDir, Target: destPath, VolumeOptions: &mounttypes.VolumeOptions{}}}}}, http.StatusBadRequest, "VolumeOptions must not be specified"},
+		}...)
+	}
+
+	if DaemonIsLinux.Condition() {
+		cases = append(cases, []testCase{
+			{cfg{Image: "busybox", HostConfig: hc{Mounts: []m{{Type: "volume", Source: "hello3", Target: destPath, VolumeOptions: &mounttypes.VolumeOptions{DriverConfig: &mounttypes.Driver{Name: "local", Options: map[string]string{"o": "size=1"}}}}}}}, http.StatusCreated, ""},
+		}...)
+
+	}
+
+	for i, x := range cases {
+		c.Logf("case %d", i)
+		status, b, err := sockRequest("POST", "/containers/create", x.config)
+		c.Assert(err, checker.IsNil)
+		c.Assert(status, checker.Equals, x.status, check.Commentf("%s\n%v", string(b), cases[i].config))
+		if len(x.msg) > 0 {
+			c.Assert(string(b), checker.Contains, x.msg, check.Commentf("%v", cases[i].config))
+		}
+	}
+}
+
+func (s *DockerSuite) TestContainerApiCreateMountsBindRead(c *check.C) {
+	testRequires(c, NotUserNamespace, SameHostDaemon)
+	// also with data in the host side
+	prefix, slash := getPrefixAndSlashFromDaemonPlatform()
+	destPath := prefix + slash + "foo"
+	tmpDir, err := ioutil.TempDir("", "test-mounts-api-bind")
+	c.Assert(err, checker.IsNil)
+	defer os.RemoveAll(tmpDir)
+	err = ioutil.WriteFile(filepath.Join(tmpDir, "bar"), []byte("hello"), 666)
+	c.Assert(err, checker.IsNil)
+
+	data := map[string]interface{}{
+		"Image":      "busybox",
+		"Cmd":        []string{"/bin/sh", "-c", "cat /foo/bar"},
+		"HostConfig": map[string]interface{}{"Mounts": []map[string]interface{}{{"Type": "bind", "Source": tmpDir, "Target": destPath}}},
+	}
+	status, resp, err := sockRequest("POST", "/containers/create?name=test", data)
+	c.Assert(err, checker.IsNil, check.Commentf(string(resp)))
+	c.Assert(status, checker.Equals, http.StatusCreated, check.Commentf(string(resp)))
+
+	out, _ := dockerCmd(c, "start", "-a", "test")
+	c.Assert(out, checker.Equals, "hello")
+}
+
+// Test Mounts comes out as expected for the MountPoint
+func (s *DockerSuite) TestContainersApiCreateMountsCreate(c *check.C) {
+	prefix, slash := getPrefixAndSlashFromDaemonPlatform()
+	destPath := prefix + slash + "foo"
+
+	var (
+		err     error
+		testImg string
+	)
+	if daemonPlatform != "windows" {
+		testImg, err = buildImage("test-mount-config", `
+	FROM busybox
+	RUN mkdir `+destPath+` && touch `+destPath+slash+`bar
+	CMD cat `+destPath+slash+`bar
+	`, true)
+	} else {
+		testImg = "busybox"
+	}
+	c.Assert(err, checker.IsNil)
+
+	type testCase struct {
+		cfg      mounttypes.Mount
+		expected types.MountPoint
+	}
+
+	cases := []testCase{
+		// use literal strings here for `Type` instead of the defined constants in the volume package to keep this honest
+		// Validation of the actual `Mount` struct is done in another test is not needed here
+		{mounttypes.Mount{Type: "volume", Target: destPath}, types.MountPoint{Driver: volume.DefaultDriverName, Type: "volume", RW: true, Destination: destPath}},
+		{mounttypes.Mount{Type: "volume", Target: destPath + slash}, types.MountPoint{Driver: volume.DefaultDriverName, Type: "volume", RW: true, Destination: destPath}},
+		{mounttypes.Mount{Type: "volume", Target: destPath, Source: "test1"}, types.MountPoint{Type: "volume", Name: "test1", RW: true, Destination: destPath}},
+		{mounttypes.Mount{Type: "volume", Target: destPath, ReadOnly: true, Source: "test2"}, types.MountPoint{Type: "volume", Name: "test2", RW: false, Destination: destPath}},
+		{mounttypes.Mount{Type: "volume", Target: destPath, Source: "test3", VolumeOptions: &mounttypes.VolumeOptions{DriverConfig: &mounttypes.Driver{Name: volume.DefaultDriverName}}}, types.MountPoint{Driver: volume.DefaultDriverName, Type: "volume", Name: "test3", RW: true, Destination: destPath}},
+	}
+
+	if SameHostDaemon.Condition() {
+		// setup temp dir for testing binds
+		tmpDir1, err := ioutil.TempDir("", "test-mounts-api-1")
+		c.Assert(err, checker.IsNil)
+		defer os.RemoveAll(tmpDir1)
+		cases = append(cases, []testCase{
+			{mounttypes.Mount{Type: "bind", Source: tmpDir1, Target: destPath}, types.MountPoint{Type: "bind", RW: true, Destination: destPath, Source: tmpDir1}},
+			{mounttypes.Mount{Type: "bind", Source: tmpDir1, Target: destPath, ReadOnly: true}, types.MountPoint{Type: "bind", RW: false, Destination: destPath, Source: tmpDir1}},
+		}...)
+
+		// for modes only supported on Linux
+		if DaemonIsLinux.Condition() {
+			tmpDir3, err := ioutils.TempDir("", "test-mounts-api-3")
+			c.Assert(err, checker.IsNil)
+			defer os.RemoveAll(tmpDir3)
+
+			c.Assert(mount.Mount(tmpDir3, tmpDir3, "none", "bind,rw"), checker.IsNil)
+			c.Assert(mount.ForceMount("", tmpDir3, "none", "shared"), checker.IsNil)
+
+			cases = append(cases, []testCase{
+				{mounttypes.Mount{Type: "bind", Source: tmpDir3, Target: destPath}, types.MountPoint{Type: "bind", RW: true, Destination: destPath, Source: tmpDir3}},
+				{mounttypes.Mount{Type: "bind", Source: tmpDir3, Target: destPath, ReadOnly: true}, types.MountPoint{Type: "bind", RW: false, Destination: destPath, Source: tmpDir3}},
+				{mounttypes.Mount{Type: "bind", Source: tmpDir3, Target: destPath, ReadOnly: true, BindOptions: &mounttypes.BindOptions{Propagation: "shared"}}, types.MountPoint{Type: "bind", RW: false, Destination: destPath, Source: tmpDir3, Propagation: "shared"}},
+			}...)
+		}
+	}
+
+	if daemonPlatform != "windows" { // Windows does not support volume populate
+		cases = append(cases, []testCase{
+			{mounttypes.Mount{Type: "volume", Target: destPath, VolumeOptions: &mounttypes.VolumeOptions{NoCopy: true}}, types.MountPoint{Driver: volume.DefaultDriverName, Type: "volume", RW: true, Destination: destPath}},
+			{mounttypes.Mount{Type: "volume", Target: destPath + slash, VolumeOptions: &mounttypes.VolumeOptions{NoCopy: true}}, types.MountPoint{Driver: volume.DefaultDriverName, Type: "volume", RW: true, Destination: destPath}},
+			{mounttypes.Mount{Type: "volume", Target: destPath, Source: "test4", VolumeOptions: &mounttypes.VolumeOptions{NoCopy: true}}, types.MountPoint{Type: "volume", Name: "test4", RW: true, Destination: destPath}},
+			{mounttypes.Mount{Type: "volume", Target: destPath, Source: "test5", ReadOnly: true, VolumeOptions: &mounttypes.VolumeOptions{NoCopy: true}}, types.MountPoint{Type: "volume", Name: "test5", RW: false, Destination: destPath}},
+		}...)
+	}
+
+	type wrapper struct {
+		containertypes.Config
+		HostConfig containertypes.HostConfig
+	}
+	type createResp struct {
+		ID string `json:"Id"`
+	}
+	for i, x := range cases {
+		c.Logf("case %d - config: %v", i, x.cfg)
+		status, data, err := sockRequest("POST", "/containers/create", wrapper{containertypes.Config{Image: testImg}, containertypes.HostConfig{Mounts: []mounttypes.Mount{x.cfg}}})
+		c.Assert(err, checker.IsNil, check.Commentf(string(data)))
+		c.Assert(status, checker.Equals, http.StatusCreated, check.Commentf(string(data)))
+
+		var resp createResp
+		err = json.Unmarshal(data, &resp)
+		c.Assert(err, checker.IsNil, check.Commentf(string(data)))
+		id := resp.ID
+
+		var mps []types.MountPoint
+		err = json.NewDecoder(strings.NewReader(inspectFieldJSON(c, id, "Mounts"))).Decode(&mps)
+		c.Assert(err, checker.IsNil)
+		c.Assert(mps, checker.HasLen, 1)
+		c.Assert(mps[0].Destination, checker.Equals, x.expected.Destination)
+
+		if len(x.expected.Source) > 0 {
+			c.Assert(mps[0].Source, checker.Equals, x.expected.Source)
+		}
+		if len(x.expected.Name) > 0 {
+			c.Assert(mps[0].Name, checker.Equals, x.expected.Name)
+		}
+		if len(x.expected.Driver) > 0 {
+			c.Assert(mps[0].Driver, checker.Equals, x.expected.Driver)
+		}
+		c.Assert(mps[0].RW, checker.Equals, x.expected.RW)
+		c.Assert(mps[0].Type, checker.Equals, x.expected.Type)
+		c.Assert(mps[0].Mode, checker.Equals, x.expected.Mode)
+		if len(x.expected.Propagation) > 0 {
+			c.Assert(mps[0].Propagation, checker.Equals, x.expected.Propagation)
+		}
+
+		out, _, err := dockerCmdWithError("start", "-a", id)
+		if (x.cfg.Type != "volume" || (x.cfg.VolumeOptions != nil && x.cfg.VolumeOptions.NoCopy)) && daemonPlatform != "windows" {
+			c.Assert(err, checker.NotNil, check.Commentf("%s\n%v", out, mps[0]))
+		} else {
+			c.Assert(err, checker.IsNil, check.Commentf("%s\n%v", out, mps[0]))
+		}
+
+		dockerCmd(c, "rm", "-fv", id)
+		if x.cfg.Type == "volume" && len(x.cfg.Source) > 0 {
+			// This should still exist even though we removed the container
+			dockerCmd(c, "volume", "inspect", mps[0].Name)
+		} else {
+			// This should be removed automatically when we removed the container
+			out, _, err := dockerCmdWithError("volume", "inspect", mps[0].Name)
+			c.Assert(err, checker.NotNil, check.Commentf(out))
+		}
+	}
+}

+ 3 - 9
integration-cli/docker_cli_run_test.go

@@ -4272,15 +4272,9 @@ func (s *DockerSuite) TestRunVolumesMountedAsSlave(c *check.C) {
 
 
 func (s *DockerSuite) TestRunNamedVolumesMountedAsShared(c *check.C) {
 func (s *DockerSuite) TestRunNamedVolumesMountedAsShared(c *check.C) {
 	testRequires(c, DaemonIsLinux, NotUserNamespace)
 	testRequires(c, DaemonIsLinux, NotUserNamespace)
-	out, exitcode, _ := dockerCmdWithError("run", "-v", "foo:/test:shared", "busybox", "touch", "/test/somefile")
-
-	if exitcode == 0 {
-		c.Fatalf("expected non-zero exit code; received %d", exitcode)
-	}
-
-	if expected := "Invalid volume specification"; !strings.Contains(out, expected) {
-		c.Fatalf(`Expected %q in output; got: %s`, expected, out)
-	}
+	out, exitCode, _ := dockerCmdWithError("run", "-v", "foo:/test:shared", "busybox", "touch", "/test/somefile")
+	c.Assert(exitCode, checker.Not(checker.Equals), 0)
+	c.Assert(out, checker.Contains, "invalid mount config")
 }
 }
 
 
 func (s *DockerSuite) TestRunNamedVolumeCopyImageData(c *check.C) {
 func (s *DockerSuite) TestRunNamedVolumeCopyImageData(c *check.C) {

+ 17 - 4
runconfig/config.go

@@ -73,16 +73,29 @@ func DecodeContainerConfig(src io.Reader) (*container.Config, *container.HostCon
 // validateVolumesAndBindSettings validates each of the volumes and bind settings
 // validateVolumesAndBindSettings validates each of the volumes and bind settings
 // passed by the caller to ensure they are valid.
 // passed by the caller to ensure they are valid.
 func validateVolumesAndBindSettings(c *container.Config, hc *container.HostConfig) error {
 func validateVolumesAndBindSettings(c *container.Config, hc *container.HostConfig) error {
+	if len(hc.Mounts) > 0 {
+		if len(hc.Binds) > 0 {
+			return conflictError(fmt.Errorf("must not specify both Binds and Mounts"))
+		}
+
+		if len(c.Volumes) > 0 {
+			return conflictError(fmt.Errorf("must not specify both Volumes and Mounts"))
+		}
+
+		if len(hc.VolumeDriver) > 0 {
+			return conflictError(fmt.Errorf("must not specify both VolumeDriver and Mounts"))
+		}
+	}
 
 
 	// Ensure all volumes and binds are valid.
 	// Ensure all volumes and binds are valid.
 	for spec := range c.Volumes {
 	for spec := range c.Volumes {
-		if _, err := volume.ParseMountSpec(spec, hc.VolumeDriver); err != nil {
-			return fmt.Errorf("Invalid volume spec %q: %v", spec, err)
+		if _, err := volume.ParseMountRaw(spec, hc.VolumeDriver); err != nil {
+			return fmt.Errorf("invalid volume spec %q: %v", spec, err)
 		}
 		}
 	}
 	}
 	for _, spec := range hc.Binds {
 	for _, spec := range hc.Binds {
-		if _, err := volume.ParseMountSpec(spec, hc.VolumeDriver); err != nil {
-			return fmt.Errorf("Invalid bind mount spec %q: %v", spec, err)
+		if _, err := volume.ParseMountRaw(spec, hc.VolumeDriver); err != nil {
+			return fmt.Errorf("invalid bind mount spec %q: %v", spec, err)
 		}
 		}
 	}
 	}
 
 

+ 6 - 0
runconfig/errors.go

@@ -2,6 +2,8 @@ package runconfig
 
 
 import (
 import (
 	"fmt"
 	"fmt"
+
+	"github.com/docker/docker/errors"
 )
 )
 
 
 var (
 var (
@@ -38,3 +40,7 @@ var (
 	// ErrConflictUTSHostname conflict between the hostname and the UTS mode
 	// ErrConflictUTSHostname conflict between the hostname and the UTS mode
 	ErrConflictUTSHostname = fmt.Errorf("Conflicting options: hostname and the UTS mode")
 	ErrConflictUTSHostname = fmt.Errorf("Conflicting options: hostname and the UTS mode")
 )
 )
+
+func conflictError(err error) error {
+	return errors.NewRequestConflictError(err)
+}

+ 118 - 0
volume/validate.go

@@ -0,0 +1,118 @@
+package volume
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"path/filepath"
+
+	"github.com/docker/docker/api/types/mount"
+)
+
+var errBindNotExist = errors.New("bind source path does not exist")
+
+type validateOpts struct {
+	skipBindSourceCheck   bool
+	skipAbsolutePathCheck bool
+}
+
+func validateMountConfig(mnt *mount.Mount, options ...func(*validateOpts)) error {
+	opts := validateOpts{}
+	for _, o := range options {
+		o(&opts)
+	}
+
+	if len(mnt.Target) == 0 {
+		return &errMountConfig{mnt, errMissingField("Target")}
+	}
+
+	if err := validateNotRoot(mnt.Target); err != nil {
+		return &errMountConfig{mnt, err}
+	}
+
+	if !opts.skipAbsolutePathCheck {
+		if err := validateAbsolute(mnt.Target); err != nil {
+			return &errMountConfig{mnt, err}
+		}
+	}
+
+	switch mnt.Type {
+	case mount.TypeBind:
+		if len(mnt.Source) == 0 {
+			return &errMountConfig{mnt, errMissingField("Source")}
+		}
+		// Don't error out just because the propagation mode is not supported on the platform
+		if opts := mnt.BindOptions; opts != nil {
+			if len(opts.Propagation) > 0 && len(propagationModes) > 0 {
+				if _, ok := propagationModes[opts.Propagation]; !ok {
+					return &errMountConfig{mnt, fmt.Errorf("invalid propagation mode: %s", opts.Propagation)}
+				}
+			}
+		}
+		if mnt.VolumeOptions != nil {
+			return &errMountConfig{mnt, errExtraField("VolumeOptions")}
+		}
+
+		if err := validateAbsolute(mnt.Source); err != nil {
+			return &errMountConfig{mnt, err}
+		}
+
+		// Do not allow binding to non-existent path
+		if !opts.skipBindSourceCheck {
+			fi, err := os.Stat(mnt.Source)
+			if err != nil {
+				if !os.IsNotExist(err) {
+					return &errMountConfig{mnt, err}
+				}
+				return &errMountConfig{mnt, errBindNotExist}
+			}
+			if err := validateStat(fi); err != nil {
+				return &errMountConfig{mnt, err}
+			}
+		}
+	case mount.TypeVolume:
+		if mnt.BindOptions != nil {
+			return &errMountConfig{mnt, errExtraField("BindOptions")}
+		}
+
+		if len(mnt.Source) == 0 && mnt.ReadOnly {
+			return &errMountConfig{mnt, fmt.Errorf("must not set ReadOnly mode when using anonymous volumes")}
+		}
+
+		if len(mnt.Source) != 0 {
+			if valid, err := IsVolumeNameValid(mnt.Source); !valid {
+				if err == nil {
+					err = errors.New("invalid volume name")
+				}
+				return &errMountConfig{mnt, err}
+			}
+		}
+	default:
+		return &errMountConfig{mnt, errors.New("mount type unknown")}
+	}
+	return nil
+}
+
+type errMountConfig struct {
+	mount *mount.Mount
+	err   error
+}
+
+func (e *errMountConfig) Error() string {
+	return fmt.Sprintf("invalid mount config for type %q: %v", e.mount.Type, e.err.Error())
+}
+
+func errExtraField(name string) error {
+	return fmt.Errorf("field %s must not be specified", name)
+}
+func errMissingField(name string) error {
+	return fmt.Errorf("field %s must not be empty", name)
+}
+
+func validateAbsolute(p string) error {
+	p = convertSlash(p)
+	if filepath.IsAbs(p) {
+		return nil
+	}
+	return fmt.Errorf("invalid mount path: '%s' mount path must be absolute", p)
+}

+ 43 - 0
volume/validate_test.go

@@ -0,0 +1,43 @@
+package volume
+
+import (
+	"errors"
+	"io/ioutil"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/docker/docker/api/types/mount"
+)
+
+func TestValidateMount(t *testing.T) {
+	testDir, err := ioutil.TempDir("", "test-validate-mount")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(testDir)
+
+	cases := []struct {
+		input    mount.Mount
+		expected error
+	}{
+		{mount.Mount{Type: mount.TypeVolume}, errMissingField("Target")},
+		{mount.Mount{Type: mount.TypeVolume, Target: testDestinationPath, Source: "hello"}, nil},
+		{mount.Mount{Type: mount.TypeVolume, Target: testDestinationPath}, nil},
+		{mount.Mount{Type: mount.TypeBind}, errMissingField("Target")},
+		{mount.Mount{Type: mount.TypeBind, Target: testDestinationPath}, errMissingField("Source")},
+		{mount.Mount{Type: mount.TypeBind, Target: testDestinationPath, Source: testSourcePath, VolumeOptions: &mount.VolumeOptions{}}, errExtraField("VolumeOptions")},
+		{mount.Mount{Type: mount.TypeBind, Source: testSourcePath, Target: testDestinationPath}, errBindNotExist},
+		{mount.Mount{Type: mount.TypeBind, Source: testDir, Target: testDestinationPath}, nil},
+		{mount.Mount{Type: "invalid", Target: testDestinationPath}, errors.New("mount type unknown")},
+	}
+	for i, x := range cases {
+		err := validateMountConfig(&x.input)
+		if err == nil && x.expected == nil {
+			continue
+		}
+		if (err == nil && x.expected != nil) || (x.expected == nil && err != nil) || !strings.Contains(err.Error(), x.expected.Error()) {
+			t.Fatalf("expected %q, got %q, case: %d", x.expected, err, i)
+		}
+	}
+}

+ 8 - 0
volume/validate_test_unix.go

@@ -0,0 +1,8 @@
+// +build !windows
+
+package volume
+
+var (
+	testDestinationPath = "/foo"
+	testSourcePath      = "/foo"
+)

+ 6 - 0
volume/validate_test_windows.go

@@ -0,0 +1,6 @@
+package volume
+
+var (
+	testDestinationPath = `c:\foo`
+	testSourcePath      = `c:\foo`
+)

+ 137 - 28
volume/volume.go

@@ -3,6 +3,7 @@ package volume
 import (
 import (
 	"fmt"
 	"fmt"
 	"os"
 	"os"
+	"path/filepath"
 	"strings"
 	"strings"
 	"syscall"
 	"syscall"
 
 
@@ -82,19 +83,19 @@ type ScopedVolume interface {
 // specifies which volume is to be used and where inside a container it should
 // specifies which volume is to be used and where inside a container it should
 // be mounted.
 // be mounted.
 type MountPoint struct {
 type MountPoint struct {
-	Source      string // Container host directory
-	Destination string // Inside the container
-	RW          bool   // True if writable
-	Name        string // Name set by user
-	Driver      string // Volume driver to use
-	Volume      Volume `json:"-"`
+	Source      string          // Container host directory
+	Destination string          // Inside the container
+	RW          bool            // True if writable
+	Name        string          // Name set by user
+	Driver      string          // Volume driver to use
+	Type        mounttypes.Type `json:",omitempty"` // Type of mount to use, see `Type<foo>` definitions
+	Volume      Volume          `json:"-"`
 
 
 	// Note Mode is not used on Windows
 	// Note Mode is not used on Windows
-	Mode string `json:"Relabel"` // Originally field was `Relabel`"
+	Mode string `json:"Relabel,omitempty"` // Originally field was `Relabel`"
 
 
 	// Note Propagation is not used on Windows
 	// Note Propagation is not used on Windows
-	Propagation mounttypes.Propagation // Mount propagation string
-	Named       bool                   // specifies if the mountpoint was specified by name
+	Propagation mounttypes.Propagation `json:",omitempty"` // Mount propagation string
 
 
 	// Specifies if data should be copied from the container before the first mount
 	// Specifies if data should be copied from the container before the first mount
 	// Use a pointer here so we can tell if the user set this value explicitly
 	// Use a pointer here so we can tell if the user set this value explicitly
@@ -102,7 +103,8 @@ type MountPoint struct {
 	CopyData bool `json:"-"`
 	CopyData bool `json:"-"`
 	// ID is the opaque ID used to pass to the volume driver.
 	// ID is the opaque ID used to pass to the volume driver.
 	// This should be set by calls to `Mount` and unset by calls to `Unmount`
 	// This should be set by calls to `Mount` and unset by calls to `Unmount`
-	ID string
+	ID   string `json:",omitempty"`
+	Spec mounttypes.Mount
 }
 }
 
 
 // Setup sets up a mount point by either mounting the volume if it is
 // Setup sets up a mount point by either mounting the volume if it is
@@ -117,12 +119,15 @@ func (m *MountPoint) Setup(mountLabel string, rootUID, rootGID int) (string, err
 	if len(m.Source) == 0 {
 	if len(m.Source) == 0 {
 		return "", fmt.Errorf("Unable to setup mount point, neither source nor volume defined")
 		return "", fmt.Errorf("Unable to setup mount point, neither source nor volume defined")
 	}
 	}
-	// idtools.MkdirAllNewAs() produces an error if m.Source exists and is a file (not a directory)
-	// also, makes sure that if the directory is created, the correct remapped rootUID/rootGID will own it
-	if err := idtools.MkdirAllNewAs(m.Source, 0755, rootUID, rootGID); err != nil {
-		if perr, ok := err.(*os.PathError); ok {
-			if perr.Err != syscall.ENOTDIR {
-				return "", err
+	// system.MkdirAll() produces an error if m.Source exists and is a file (not a directory),
+	if m.Type == mounttypes.TypeBind {
+		// idtools.MkdirAllNewAs() produces an error if m.Source exists and is a file (not a directory)
+		// also, makes sure that if the directory is created, the correct remapped rootUID/rootGID will own it
+		if err := idtools.MkdirAllNewAs(m.Source, 0755, rootUID, rootGID); err != nil {
+			if perr, ok := err.(*os.PathError); ok {
+				if perr.Err != syscall.ENOTDIR {
+					return "", err
+				}
 			}
 			}
 		}
 		}
 	}
 	}
@@ -142,17 +147,6 @@ func (m *MountPoint) Path() string {
 	return m.Source
 	return m.Source
 }
 }
 
 
-// Type returns the type of mount point
-func (m *MountPoint) Type() string {
-	if m.Name != "" {
-		return "volume"
-	}
-	if m.Source != "" {
-		return "bind"
-	}
-	return "ephemeral"
-}
-
 // ParseVolumesFrom ensures that the supplied volumes-from is valid.
 // ParseVolumesFrom ensures that the supplied volumes-from is valid.
 func ParseVolumesFrom(spec string) (string, string, error) {
 func ParseVolumesFrom(spec string) (string, string, error) {
 	if len(spec) == 0 {
 	if len(spec) == 0 {
@@ -183,10 +177,125 @@ func ParseVolumesFrom(spec string) (string, string, error) {
 	return id, mode, nil
 	return id, mode, nil
 }
 }
 
 
+// ParseMountRaw parses a raw volume spec (e.g. `-v /foo:/bar:shared`) into a
+// structured spec. Once the raw spec is parsed it relies on `ParseMountSpec` to
+// validate the spec and create a MountPoint
+func ParseMountRaw(raw, volumeDriver string) (*MountPoint, error) {
+	arr, err := splitRawSpec(convertSlash(raw))
+	if err != nil {
+		return nil, err
+	}
+
+	var spec mounttypes.Mount
+	var mode string
+	switch len(arr) {
+	case 1:
+		// Just a destination path in the container
+		spec.Target = arr[0]
+	case 2:
+		if ValidMountMode(arr[1]) {
+			// Destination + Mode is not a valid volume - volumes
+			// cannot include a mode. eg /foo:rw
+			return nil, errInvalidSpec(raw)
+		}
+		// Host Source Path or Name + Destination
+		spec.Source = arr[0]
+		spec.Target = arr[1]
+	case 3:
+		// HostSourcePath+DestinationPath+Mode
+		spec.Source = arr[0]
+		spec.Target = arr[1]
+		mode = arr[2]
+	default:
+		return nil, errInvalidSpec(raw)
+	}
+
+	if !ValidMountMode(mode) {
+		return nil, errInvalidMode(mode)
+	}
+
+	if filepath.IsAbs(spec.Source) {
+		spec.Type = mounttypes.TypeBind
+	} else {
+		spec.Type = mounttypes.TypeVolume
+	}
+
+	spec.ReadOnly = !ReadWrite(mode)
+
+	// cannot assume that if a volume driver is passed in that we should set it
+	if volumeDriver != "" && spec.Type == mounttypes.TypeVolume {
+		spec.VolumeOptions = &mounttypes.VolumeOptions{
+			DriverConfig: &mounttypes.Driver{Name: volumeDriver},
+		}
+	}
+
+	if copyData, isSet := getCopyMode(mode); isSet {
+		if spec.VolumeOptions == nil {
+			spec.VolumeOptions = &mounttypes.VolumeOptions{}
+		}
+		spec.VolumeOptions.NoCopy = !copyData
+	}
+	if HasPropagation(mode) {
+		spec.BindOptions = &mounttypes.BindOptions{
+			Propagation: GetPropagation(mode),
+		}
+	}
+
+	mp, err := ParseMountSpec(spec, platformRawValidationOpts...)
+	if mp != nil {
+		mp.Mode = mode
+	}
+	if err != nil {
+		err = fmt.Errorf("%v: %v", errInvalidSpec(raw), err)
+	}
+	return mp, err
+}
+
+// ParseMountSpec reads a mount config, validates it, and configures a mountpoint from it.
+func ParseMountSpec(cfg mounttypes.Mount, options ...func(*validateOpts)) (*MountPoint, error) {
+	if err := validateMountConfig(&cfg, options...); err != nil {
+		return nil, err
+	}
+	mp := &MountPoint{
+		RW:          !cfg.ReadOnly,
+		Destination: clean(convertSlash(cfg.Target)),
+		Type:        cfg.Type,
+		Spec:        cfg,
+	}
+
+	switch cfg.Type {
+	case mounttypes.TypeVolume:
+		if cfg.Source == "" {
+			mp.Name = stringid.GenerateNonCryptoID()
+		} else {
+			mp.Name = cfg.Source
+		}
+		mp.CopyData = DefaultCopyMode
+
+		mp.Driver = DefaultDriverName
+		if cfg.VolumeOptions != nil {
+			if cfg.VolumeOptions.DriverConfig != nil {
+				mp.Driver = cfg.VolumeOptions.DriverConfig.Name
+			}
+			if cfg.VolumeOptions.NoCopy {
+				mp.CopyData = false
+			}
+		}
+	case mounttypes.TypeBind:
+		mp.Source = clean(convertSlash(cfg.Source))
+		if cfg.BindOptions != nil {
+			if len(cfg.BindOptions.Propagation) > 0 {
+				mp.Propagation = cfg.BindOptions.Propagation
+			}
+		}
+	}
+	return mp, nil
+}
+
 func errInvalidMode(mode string) error {
 func errInvalidMode(mode string) error {
 	return fmt.Errorf("invalid mode: %v", mode)
 	return fmt.Errorf("invalid mode: %v", mode)
 }
 }
 
 
 func errInvalidSpec(spec string) error {
 func errInvalidSpec(spec string) error {
-	return fmt.Errorf("Invalid volume specification: '%s'", spec)
+	return fmt.Errorf("invalid volume specification: '%s'", spec)
 }
 }

+ 0 - 5
volume/volume_copy.go

@@ -2,11 +2,6 @@ package volume
 
 
 import "strings"
 import "strings"
 
 
-const (
-	// DefaultCopyMode is the copy mode used by default for normal/named volumes
-	DefaultCopyMode = true
-)
-
 // {<copy mode>=isEnabled}
 // {<copy mode>=isEnabled}
 var copyModes = map[string]bool{
 var copyModes = map[string]bool{
 	"nocopy": false,
 	"nocopy": false,

+ 8 - 0
volume/volume_copy_unix.go

@@ -0,0 +1,8 @@
+// +build !windows
+
+package volume
+
+const (
+	// DefaultCopyMode is the copy mode used by default for normal/named volumes
+	DefaultCopyMode = true
+)

+ 6 - 0
volume/volume_copy_windows.go

@@ -0,0 +1,6 @@
+package volume
+
+const (
+	// DefaultCopyMode is the copy mode used by default for normal/named volumes
+	DefaultCopyMode = false
+)

+ 2 - 2
volume/volume_propagation_linux.go

@@ -10,9 +10,9 @@ import (
 
 
 // DefaultPropagationMode defines what propagation mode should be used by
 // DefaultPropagationMode defines what propagation mode should be used by
 // default if user has not specified one explicitly.
 // default if user has not specified one explicitly.
-const DefaultPropagationMode mounttypes.Propagation = "rprivate"
-
 // propagation modes
 // propagation modes
+const DefaultPropagationMode = mounttypes.PropagationRPrivate
+
 var propagationModes = map[mounttypes.Propagation]bool{
 var propagationModes = map[mounttypes.Propagation]bool{
 	mounttypes.PropagationPrivate:  true,
 	mounttypes.PropagationPrivate:  true,
 	mounttypes.PropagationRPrivate: true,
 	mounttypes.PropagationRPrivate: true,

+ 17 - 17
volume/volume_propagation_linux_test.go

@@ -7,7 +7,7 @@ import (
 	"testing"
 	"testing"
 )
 )
 
 
-func TestParseMountSpecPropagation(t *testing.T) {
+func TestParseMountRawPropagation(t *testing.T) {
 	var (
 	var (
 		valid   []string
 		valid   []string
 		invalid map[string]string
 		invalid map[string]string
@@ -34,31 +34,31 @@ func TestParseMountSpecPropagation(t *testing.T) {
 		"/hostPath:/containerPath:ro,Z,rprivate",
 		"/hostPath:/containerPath:ro,Z,rprivate",
 	}
 	}
 	invalid = map[string]string{
 	invalid = map[string]string{
-		"/path:/path:ro,rshared,rslave":   `invalid mode: ro,rshared,rslave`,
-		"/path:/path:ro,z,rshared,rslave": `invalid mode: ro,z,rshared,rslave`,
-		"/path:shared":                    "Invalid volume specification",
-		"/path:slave":                     "Invalid volume specification",
-		"/path:private":                   "Invalid volume specification",
-		"name:/absolute-path:shared":      "Invalid volume specification",
-		"name:/absolute-path:rshared":     "Invalid volume specification",
-		"name:/absolute-path:slave":       "Invalid volume specification",
-		"name:/absolute-path:rslave":      "Invalid volume specification",
-		"name:/absolute-path:private":     "Invalid volume specification",
-		"name:/absolute-path:rprivate":    "Invalid volume specification",
+		"/path:/path:ro,rshared,rslave":   `invalid mode`,
+		"/path:/path:ro,z,rshared,rslave": `invalid mode`,
+		"/path:shared":                    "invalid volume specification",
+		"/path:slave":                     "invalid volume specification",
+		"/path:private":                   "invalid volume specification",
+		"name:/absolute-path:shared":      "invalid volume specification",
+		"name:/absolute-path:rshared":     "invalid volume specification",
+		"name:/absolute-path:slave":       "invalid volume specification",
+		"name:/absolute-path:rslave":      "invalid volume specification",
+		"name:/absolute-path:private":     "invalid volume specification",
+		"name:/absolute-path:rprivate":    "invalid volume specification",
 	}
 	}
 
 
 	for _, path := range valid {
 	for _, path := range valid {
-		if _, err := ParseMountSpec(path, "local"); err != nil {
-			t.Fatalf("ParseMountSpec(`%q`) should succeed: error %q", path, err)
+		if _, err := ParseMountRaw(path, "local"); err != nil {
+			t.Fatalf("ParseMountRaw(`%q`) should succeed: error %q", path, err)
 		}
 		}
 	}
 	}
 
 
 	for path, expectedError := range invalid {
 	for path, expectedError := range invalid {
-		if _, err := ParseMountSpec(path, "local"); err == nil {
-			t.Fatalf("ParseMountSpec(`%q`) should have failed validation. Err %v", path, err)
+		if _, err := ParseMountRaw(path, "local"); err == nil {
+			t.Fatalf("ParseMountRaw(`%q`) should have failed validation. Err %v", path, err)
 		} else {
 		} else {
 			if !strings.Contains(err.Error(), expectedError) {
 			if !strings.Contains(err.Error(), expectedError) {
-				t.Fatalf("ParseMountSpec(`%q`) error should contain %q, got %v", path, expectedError, err.Error())
+				t.Fatalf("ParseMountRaw(`%q`) error should contain %q, got %v", path, expectedError, err.Error())
 			}
 			}
 		}
 		}
 	}
 	}

+ 1 - 1
volume/volume_propagation_unsupported.go

@@ -9,7 +9,7 @@ import mounttypes "github.com/docker/docker/api/types/mount"
 const DefaultPropagationMode mounttypes.Propagation = ""
 const DefaultPropagationMode mounttypes.Propagation = ""
 
 
 // propagation modes not supported on this platform.
 // propagation modes not supported on this platform.
-var propagationModes = map[string]bool{}
+var propagationModes = map[mounttypes.Propagation]bool{}
 
 
 // GetPropagation is not supported. Return empty string.
 // GetPropagation is not supported. Return empty string.
 func GetPropagation(mode string) mounttypes.Propagation {
 func GetPropagation(mode string) mounttypes.Propagation {

+ 122 - 66
volume/volume_test.go

@@ -1,12 +1,16 @@
 package volume
 package volume
 
 
 import (
 import (
+	"io/ioutil"
+	"os"
 	"runtime"
 	"runtime"
 	"strings"
 	"strings"
 	"testing"
 	"testing"
+
+	"github.com/docker/docker/api/types/mount"
 )
 )
 
 
-func TestParseMountSpec(t *testing.T) {
+func TestParseMountRaw(t *testing.T) {
 	var (
 	var (
 		valid   []string
 		valid   []string
 		invalid map[string]string
 		invalid map[string]string
@@ -36,25 +40,25 @@ func TestParseMountSpec(t *testing.T) {
 			`c:\Program Files (x86)`, // With capitals and brackets
 			`c:\Program Files (x86)`, // With capitals and brackets
 		}
 		}
 		invalid = map[string]string{
 		invalid = map[string]string{
-			``:                                 "Invalid volume specification: ",
-			`.`:                                "Invalid volume specification: ",
-			`..\`:                              "Invalid volume specification: ",
-			`c:\:..\`:                          "Invalid volume specification: ",
-			`c:\:d:\:xyzzy`:                    "Invalid volume specification: ",
-			`c:`:                               "cannot be c:",
-			`c:\`:                              `cannot be c:\`,
-			`c:\notexist:d:`:                   `The system cannot find the file specified`,
-			`c:\windows\system32\ntdll.dll:d:`: `Source 'c:\windows\system32\ntdll.dll' is not a directory`,
-			`name<:d:`:                         `Invalid volume specification`,
-			`name>:d:`:                         `Invalid volume specification`,
-			`name::d:`:                         `Invalid volume specification`,
-			`name":d:`:                         `Invalid volume specification`,
-			`name\:d:`:                         `Invalid volume specification`,
-			`name*:d:`:                         `Invalid volume specification`,
-			`name|:d:`:                         `Invalid volume specification`,
-			`name?:d:`:                         `Invalid volume specification`,
-			`name/:d:`:                         `Invalid volume specification`,
-			`d:\pathandmode:rw`:                `Invalid volume specification`,
+			``:                                 "invalid volume specification: ",
+			`.`:                                "invalid volume specification: ",
+			`..\`:                              "invalid volume specification: ",
+			`c:\:..\`:                          "invalid volume specification: ",
+			`c:\:d:\:xyzzy`:                    "invalid volume specification: ",
+			`c:`:                               "cannot be `c:`",
+			`c:\`:                              "cannot be `c:`",
+			`c:\notexist:d:`:                   `source path does not exist`,
+			`c:\windows\system32\ntdll.dll:d:`: `source path must be a directory`,
+			`name<:d:`:                         `invalid volume specification`,
+			`name>:d:`:                         `invalid volume specification`,
+			`name::d:`:                         `invalid volume specification`,
+			`name":d:`:                         `invalid volume specification`,
+			`name\:d:`:                         `invalid volume specification`,
+			`name*:d:`:                         `invalid volume specification`,
+			`name|:d:`:                         `invalid volume specification`,
+			`name?:d:`:                         `invalid volume specification`,
+			`name/:d:`:                         `invalid volume specification`,
+			`d:\pathandmode:rw`:                `invalid volume specification`,
 			`con:d:`:                           `cannot be a reserved word for Windows filenames`,
 			`con:d:`:                           `cannot be a reserved word for Windows filenames`,
 			`PRN:d:`:                           `cannot be a reserved word for Windows filenames`,
 			`PRN:d:`:                           `cannot be a reserved word for Windows filenames`,
 			`aUx:d:`:                           `cannot be a reserved word for Windows filenames`,
 			`aUx:d:`:                           `cannot be a reserved word for Windows filenames`,
@@ -93,50 +97,50 @@ func TestParseMountSpec(t *testing.T) {
 			"/rw:/ro",
 			"/rw:/ro",
 		}
 		}
 		invalid = map[string]string{
 		invalid = map[string]string{
-			"":                "Invalid volume specification",
-			"./":              "Invalid volume destination",
-			"../":             "Invalid volume destination",
-			"/:../":           "Invalid volume destination",
-			"/:path":          "Invalid volume destination",
-			":":               "Invalid volume specification",
-			"/tmp:":           "Invalid volume destination",
-			":test":           "Invalid volume specification",
-			":/test":          "Invalid volume specification",
-			"tmp:":            "Invalid volume destination",
-			":test:":          "Invalid volume specification",
-			"::":              "Invalid volume specification",
-			":::":             "Invalid volume specification",
-			"/tmp:::":         "Invalid volume specification",
-			":/tmp::":         "Invalid volume specification",
-			"/path:rw":        "Invalid volume specification",
-			"/path:ro":        "Invalid volume specification",
-			"/rw:rw":          "Invalid volume specification",
-			"path:ro":         "Invalid volume specification",
-			"/path:/path:sw":  `invalid mode: sw`,
-			"/path:/path:rwz": `invalid mode: rwz`,
+			"":                "invalid volume specification",
+			"./":              "mount path must be absolute",
+			"../":             "mount path must be absolute",
+			"/:../":           "mount path must be absolute",
+			"/:path":          "mount path must be absolute",
+			":":               "invalid volume specification",
+			"/tmp:":           "invalid volume specification",
+			":test":           "invalid volume specification",
+			":/test":          "invalid volume specification",
+			"tmp:":            "invalid volume specification",
+			":test:":          "invalid volume specification",
+			"::":              "invalid volume specification",
+			":::":             "invalid volume specification",
+			"/tmp:::":         "invalid volume specification",
+			":/tmp::":         "invalid volume specification",
+			"/path:rw":        "invalid volume specification",
+			"/path:ro":        "invalid volume specification",
+			"/rw:rw":          "invalid volume specification",
+			"path:ro":         "invalid volume specification",
+			"/path:/path:sw":  `invalid mode`,
+			"/path:/path:rwz": `invalid mode`,
 		}
 		}
 	}
 	}
 
 
 	for _, path := range valid {
 	for _, path := range valid {
-		if _, err := ParseMountSpec(path, "local"); err != nil {
-			t.Fatalf("ParseMountSpec(`%q`) should succeed: error %q", path, err)
+		if _, err := ParseMountRaw(path, "local"); err != nil {
+			t.Fatalf("ParseMountRaw(`%q`) should succeed: error %q", path, err)
 		}
 		}
 	}
 	}
 
 
 	for path, expectedError := range invalid {
 	for path, expectedError := range invalid {
-		if _, err := ParseMountSpec(path, "local"); err == nil {
-			t.Fatalf("ParseMountSpec(`%q`) should have failed validation. Err %v", path, err)
+		if mp, err := ParseMountRaw(path, "local"); err == nil {
+			t.Fatalf("ParseMountRaw(`%q`) should have failed validation. Err '%v' - MP: %v", path, err, mp)
 		} else {
 		} else {
 			if !strings.Contains(err.Error(), expectedError) {
 			if !strings.Contains(err.Error(), expectedError) {
-				t.Fatalf("ParseMountSpec(`%q`) error should contain %q, got %v", path, expectedError, err.Error())
+				t.Fatalf("ParseMountRaw(`%q`) error should contain %q, got %v", path, expectedError, err.Error())
 			}
 			}
 		}
 		}
 	}
 	}
 }
 }
 
 
-// testParseMountSpec is a structure used by TestParseMountSpecSplit for
-// specifying test cases for the ParseMountSpec() function.
-type testParseMountSpec struct {
+// testParseMountRaw is a structure used by TestParseMountRawSplit for
+// specifying test cases for the ParseMountRaw() function.
+type testParseMountRaw struct {
 	bind      string
 	bind      string
 	driver    string
 	driver    string
 	expDest   string
 	expDest   string
@@ -147,10 +151,10 @@ type testParseMountSpec struct {
 	fail      bool
 	fail      bool
 }
 }
 
 
-func TestParseMountSpecSplit(t *testing.T) {
-	var cases []testParseMountSpec
+func TestParseMountRawSplit(t *testing.T) {
+	var cases []testParseMountRaw
 	if runtime.GOOS == "windows" {
 	if runtime.GOOS == "windows" {
-		cases = []testParseMountSpec{
+		cases = []testParseMountRaw{
 			{`c:\:d:`, "local", `d:`, `c:\`, ``, "", true, false},
 			{`c:\:d:`, "local", `d:`, `c:\`, ``, "", true, false},
 			{`c:\:d:\`, "local", `d:\`, `c:\`, ``, "", true, false},
 			{`c:\:d:\`, "local", `d:\`, `c:\`, ``, "", true, false},
 			// TODO Windows post TP5 - Add readonly support {`c:\:d:\:ro`, "local", `d:\`, `c:\`, ``, "", false, false},
 			// TODO Windows post TP5 - Add readonly support {`c:\:d:\:ro`, "local", `d:\`, `c:\`, ``, "", false, false},
@@ -159,25 +163,26 @@ func TestParseMountSpecSplit(t *testing.T) {
 			{`name:d::rw`, "local", `d:`, ``, `name`, "local", true, false},
 			{`name:d::rw`, "local", `d:`, ``, `name`, "local", true, false},
 			{`name:d:`, "local", `d:`, ``, `name`, "local", true, false},
 			{`name:d:`, "local", `d:`, ``, `name`, "local", true, false},
 			// TODO Windows post TP5 - Add readonly support {`name:d::ro`, "local", `d:`, ``, `name`, "local", false, false},
 			// TODO Windows post TP5 - Add readonly support {`name:d::ro`, "local", `d:`, ``, `name`, "local", false, false},
-			{`name:c:`, "", ``, ``, ``, "", true, true},
-			{`driver/name:c:`, "", ``, ``, ``, "", true, true},
+			{`name:c:`, "", ``, ``, ``, DefaultDriverName, true, true},
+			{`driver/name:c:`, "", ``, ``, ``, DefaultDriverName, true, true},
 		}
 		}
 	} else {
 	} else {
-		cases = []testParseMountSpec{
+		cases = []testParseMountRaw{
 			{"/tmp:/tmp1", "", "/tmp1", "/tmp", "", "", true, false},
 			{"/tmp:/tmp1", "", "/tmp1", "/tmp", "", "", true, false},
 			{"/tmp:/tmp2:ro", "", "/tmp2", "/tmp", "", "", false, false},
 			{"/tmp:/tmp2:ro", "", "/tmp2", "/tmp", "", "", false, false},
 			{"/tmp:/tmp3:rw", "", "/tmp3", "/tmp", "", "", true, false},
 			{"/tmp:/tmp3:rw", "", "/tmp3", "/tmp", "", "", true, false},
 			{"/tmp:/tmp4:foo", "", "", "", "", "", false, true},
 			{"/tmp:/tmp4:foo", "", "", "", "", "", false, true},
-			{"name:/named1", "", "/named1", "", "name", "", true, false},
+			{"name:/named1", "", "/named1", "", "name", DefaultDriverName, true, false},
 			{"name:/named2", "external", "/named2", "", "name", "external", true, false},
 			{"name:/named2", "external", "/named2", "", "name", "external", true, false},
 			{"name:/named3:ro", "local", "/named3", "", "name", "local", false, false},
 			{"name:/named3:ro", "local", "/named3", "", "name", "local", false, false},
-			{"local/name:/tmp:rw", "", "/tmp", "", "local/name", "", true, false},
+			{"local/name:/tmp:rw", "", "/tmp", "", "local/name", DefaultDriverName, true, false},
 			{"/tmp:tmp", "", "", "", "", "", true, true},
 			{"/tmp:tmp", "", "", "", "", "", true, true},
 		}
 		}
 	}
 	}
 
 
-	for _, c := range cases {
-		m, err := ParseMountSpec(c.bind, c.driver)
+	for i, c := range cases {
+		t.Logf("case %d", i)
+		m, err := ParseMountRaw(c.bind, c.driver)
 		if c.fail {
 		if c.fail {
 			if err == nil {
 			if err == nil {
 				t.Fatalf("Expected error, was nil, for spec %s\n", c.bind)
 				t.Fatalf("Expected error, was nil, for spec %s\n", c.bind)
@@ -186,28 +191,79 @@ func TestParseMountSpecSplit(t *testing.T) {
 		}
 		}
 
 
 		if m == nil || err != nil {
 		if m == nil || err != nil {
-			t.Fatalf("ParseMountSpec failed for spec %s driver %s error %v\n", c.bind, c.driver, err.Error())
+			t.Fatalf("ParseMountRaw failed for spec '%s', driver '%s', error '%v'", c.bind, c.driver, err.Error())
 			continue
 			continue
 		}
 		}
 
 
 		if m.Destination != c.expDest {
 		if m.Destination != c.expDest {
-			t.Fatalf("Expected destination %s, was %s, for spec %s\n", c.expDest, m.Destination, c.bind)
+			t.Fatalf("Expected destination '%s, was %s', for spec '%s'", c.expDest, m.Destination, c.bind)
 		}
 		}
 
 
 		if m.Source != c.expSource {
 		if m.Source != c.expSource {
-			t.Fatalf("Expected source %s, was %s, for spec %s\n", c.expSource, m.Source, c.bind)
+			t.Fatalf("Expected source '%s', was '%s', for spec '%s'", c.expSource, m.Source, c.bind)
 		}
 		}
 
 
 		if m.Name != c.expName {
 		if m.Name != c.expName {
-			t.Fatalf("Expected name %s, was %s for spec %s\n", c.expName, m.Name, c.bind)
+			t.Fatalf("Expected name '%s', was '%s' for spec '%s'", c.expName, m.Name, c.bind)
 		}
 		}
 
 
-		if m.Driver != c.expDriver {
-			t.Fatalf("Expected driver %s, was %s, for spec %s\n", c.expDriver, m.Driver, c.bind)
+		if (m.Driver != c.expDriver) || (m.Driver == DefaultDriverName && c.expDriver == "") {
+			t.Fatalf("Expected driver '%s', was '%s', for spec '%s'", c.expDriver, m.Driver, c.bind)
 		}
 		}
 
 
 		if m.RW != c.expRW {
 		if m.RW != c.expRW {
-			t.Fatalf("Expected RW %v, was %v for spec %s\n", c.expRW, m.RW, c.bind)
+			t.Fatalf("Expected RW '%v', was '%v' for spec '%s'", c.expRW, m.RW, c.bind)
+		}
+	}
+}
+
+func TestParseMountSpec(t *testing.T) {
+	type c struct {
+		input    mount.Mount
+		expected MountPoint
+	}
+	testDir, err := ioutil.TempDir("", "test-mount-config")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(testDir)
+
+	cases := []c{
+		{mount.Mount{Type: mount.TypeBind, Source: testDir, Target: testDestinationPath, ReadOnly: true}, MountPoint{Type: mount.TypeBind, Source: testDir, Destination: testDestinationPath}},
+		{mount.Mount{Type: mount.TypeBind, Source: testDir, Target: testDestinationPath}, MountPoint{Type: mount.TypeBind, Source: testDir, Destination: testDestinationPath, RW: true}},
+		{mount.Mount{Type: mount.TypeBind, Source: testDir + string(os.PathSeparator), Target: testDestinationPath, ReadOnly: true}, MountPoint{Type: mount.TypeBind, Source: testDir, Destination: testDestinationPath}},
+		{mount.Mount{Type: mount.TypeBind, Source: testDir, Target: testDestinationPath + string(os.PathSeparator), ReadOnly: true}, MountPoint{Type: mount.TypeBind, Source: testDir, Destination: testDestinationPath}},
+		{mount.Mount{Type: mount.TypeVolume, Target: testDestinationPath}, MountPoint{Type: mount.TypeVolume, Destination: testDestinationPath, RW: true, Driver: DefaultDriverName, CopyData: DefaultCopyMode}},
+		{mount.Mount{Type: mount.TypeVolume, Target: testDestinationPath + string(os.PathSeparator)}, MountPoint{Type: mount.TypeVolume, Destination: testDestinationPath, RW: true, Driver: DefaultDriverName, CopyData: DefaultCopyMode}},
+	}
+
+	for i, c := range cases {
+		t.Logf("case %d", i)
+		mp, err := ParseMountSpec(c.input)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if c.expected.Type != mp.Type {
+			t.Fatalf("Expected mount types to match. Expected: '%s', Actual: '%s'", c.expected.Type, mp.Type)
+		}
+		if c.expected.Destination != mp.Destination {
+			t.Fatalf("Expected mount destination to match. Expected: '%s', Actual: '%s'", c.expected.Destination, mp.Destination)
+		}
+		if c.expected.Source != mp.Source {
+			t.Fatalf("Expected mount source to match. Expected: '%s', Actual: '%s'", c.expected.Source, mp.Source)
+		}
+		if c.expected.RW != mp.RW {
+			t.Fatalf("Expected mount writable to match. Expected: '%v', Actual: '%s'", c.expected.RW, mp.RW)
+		}
+		if c.expected.Propagation != mp.Propagation {
+			t.Fatalf("Expected mount propagation to match. Expected: '%v', Actual: '%s'", c.expected.Propagation, mp.Propagation)
+		}
+		if c.expected.Driver != mp.Driver {
+			t.Fatalf("Expected mount driver to match. Expected: '%v', Actual: '%s'", c.expected.Driver, mp.Driver)
+		}
+		if c.expected.CopyData != mp.CopyData {
+			t.Fatalf("Expected mount copy data to match. Expected: '%v', Actual: '%v'", c.expected.CopyData, mp.CopyData)
 		}
 		}
 	}
 	}
 }
 }

+ 48 - 98
volume/volume_unix.go

@@ -4,12 +4,20 @@ package volume
 
 
 import (
 import (
 	"fmt"
 	"fmt"
+	"os"
 	"path/filepath"
 	"path/filepath"
 	"strings"
 	"strings"
 
 
 	mounttypes "github.com/docker/docker/api/types/mount"
 	mounttypes "github.com/docker/docker/api/types/mount"
 )
 )
 
 
+var platformRawValidationOpts = []func(o *validateOpts){
+	// need to make sure to not error out if the bind source does not exist on unix
+	// this is supported for historical reasons, the path will be automatically
+	// created later.
+	func(o *validateOpts) { o.skipBindSourceCheck = true },
+}
+
 // read-write modes
 // read-write modes
 var rwModes = map[string]bool{
 var rwModes = map[string]bool{
 	"rw": true,
 	"rw": true,
@@ -38,103 +46,6 @@ func (m *MountPoint) HasResource(absolutePath string) bool {
 	return err == nil && relPath != ".." && !strings.HasPrefix(relPath, fmt.Sprintf("..%c", filepath.Separator))
 	return err == nil && relPath != ".." && !strings.HasPrefix(relPath, fmt.Sprintf("..%c", filepath.Separator))
 }
 }
 
 
-// ParseMountSpec validates the configuration of mount information is valid.
-func ParseMountSpec(spec, volumeDriver string) (*MountPoint, error) {
-	spec = filepath.ToSlash(spec)
-
-	mp := &MountPoint{
-		RW:          true,
-		Propagation: DefaultPropagationMode,
-	}
-	if strings.Count(spec, ":") > 2 {
-		return nil, errInvalidSpec(spec)
-	}
-
-	arr := strings.SplitN(spec, ":", 3)
-	if arr[0] == "" {
-		return nil, errInvalidSpec(spec)
-	}
-
-	switch len(arr) {
-	case 1:
-		// Just a destination path in the container
-		mp.Destination = filepath.Clean(arr[0])
-	case 2:
-		if isValid := ValidMountMode(arr[1]); isValid {
-			// Destination + Mode is not a valid volume - volumes
-			// cannot include a mode. eg /foo:rw
-			return nil, errInvalidSpec(spec)
-		}
-		// Host Source Path or Name + Destination
-		mp.Source = arr[0]
-		mp.Destination = arr[1]
-	case 3:
-		// HostSourcePath+DestinationPath+Mode
-		mp.Source = arr[0]
-		mp.Destination = arr[1]
-		mp.Mode = arr[2] // Mode field is used by SELinux to decide whether to apply label
-		if !ValidMountMode(mp.Mode) {
-			return nil, errInvalidMode(mp.Mode)
-		}
-		mp.RW = ReadWrite(mp.Mode)
-		mp.Propagation = GetPropagation(mp.Mode)
-	default:
-		return nil, errInvalidSpec(spec)
-	}
-
-	//validate the volumes destination path
-	mp.Destination = filepath.Clean(mp.Destination)
-	if !filepath.IsAbs(mp.Destination) {
-		return nil, fmt.Errorf("Invalid volume destination path: '%s' mount path must be absolute.", mp.Destination)
-	}
-
-	// Destination cannot be "/"
-	if mp.Destination == "/" {
-		return nil, fmt.Errorf("Invalid specification: destination can't be '/' in '%s'", spec)
-	}
-
-	name, source := ParseVolumeSource(mp.Source)
-	if len(source) == 0 {
-		mp.Source = "" // Clear it out as we previously assumed it was not a name
-		mp.Driver = volumeDriver
-		// Named volumes can't have propagation properties specified.
-		// Their defaults will be decided by docker. This is just a
-		// safeguard. Don't want to get into situations where named
-		// volumes were mounted as '[r]shared' inside container and
-		// container does further mounts under that volume and these
-		// mounts become visible on  host and later original volume
-		// cleanup becomes an issue if container does not unmount
-		// submounts explicitly.
-		if HasPropagation(mp.Mode) {
-			return nil, errInvalidSpec(spec)
-		}
-	} else {
-		mp.Source = filepath.Clean(source)
-	}
-
-	copyData, isSet := getCopyMode(mp.Mode)
-	// do not allow copy modes on binds
-	if len(name) == 0 && isSet {
-		return nil, errInvalidMode(mp.Mode)
-	}
-
-	mp.CopyData = copyData
-	mp.Name = name
-
-	return mp, nil
-}
-
-// ParseVolumeSource parses the origin sources that's mounted into the container.
-// It returns a name and a source. It looks to see if the spec passed in
-// is an absolute file. If it is, it assumes the spec is a source. If not,
-// it assumes the spec is a name.
-func ParseVolumeSource(spec string) (string, string) {
-	if !filepath.IsAbs(spec) {
-		return spec, ""
-	}
-	return "", spec
-}
-
 // IsVolumeNameValid checks a volume name in a platform specific manner.
 // IsVolumeNameValid checks a volume name in a platform specific manner.
 func IsVolumeNameValid(name string) (bool, error) {
 func IsVolumeNameValid(name string) (bool, error) {
 	return true, nil
 	return true, nil
@@ -143,6 +54,10 @@ func IsVolumeNameValid(name string) (bool, error) {
 // ValidMountMode will make sure the mount mode is valid.
 // ValidMountMode will make sure the mount mode is valid.
 // returns if it's a valid mount mode or not.
 // returns if it's a valid mount mode or not.
 func ValidMountMode(mode string) bool {
 func ValidMountMode(mode string) bool {
+	if mode == "" {
+		return true
+	}
+
 	rwModeCount := 0
 	rwModeCount := 0
 	labelModeCount := 0
 	labelModeCount := 0
 	propagationModeCount := 0
 	propagationModeCount := 0
@@ -183,6 +98,41 @@ func ReadWrite(mode string) bool {
 			return false
 			return false
 		}
 		}
 	}
 	}
-
 	return true
 	return true
 }
 }
+
+func validateNotRoot(p string) error {
+	p = filepath.Clean(convertSlash(p))
+	if p == "/" {
+		return fmt.Errorf("invalid specification: destination can't be '/'")
+	}
+	return nil
+}
+
+func validateCopyMode(mode bool) error {
+	return nil
+}
+
+func convertSlash(p string) string {
+	return filepath.ToSlash(p)
+}
+
+func splitRawSpec(raw string) ([]string, error) {
+	if strings.Count(raw, ":") > 2 {
+		return nil, errInvalidSpec(raw)
+	}
+
+	arr := strings.SplitN(raw, ":", 3)
+	if arr[0] == "" {
+		return nil, errInvalidSpec(raw)
+	}
+	return arr, nil
+}
+
+func clean(p string) string {
+	return filepath.Clean(p)
+}
+
+func validateStat(fi os.FileInfo) error {
+	return nil
+}

+ 73 - 84
volume/volume_windows.go

@@ -7,7 +7,6 @@ import (
 	"regexp"
 	"regexp"
 	"strings"
 	"strings"
 
 
-	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/pkg/system"
 	"github.com/docker/docker/pkg/system"
 )
 )
 
 
@@ -21,6 +20,15 @@ var roModes = map[string]bool{
 	"ro": true,
 	"ro": true,
 }
 }
 
 
+var platformRawValidationOpts = []func(*validateOpts){
+	// filepath.IsAbs is weird on Windows:
+	//	`c:` is not considered an absolute path
+	//	`c:\` is considered an absolute path
+	// In any case, the regex matching below ensures absolute paths
+	// TODO: consider this a bug with filepath.IsAbs (?)
+	func(o *validateOpts) { o.skipAbsolutePathCheck = true },
+}
+
 const (
 const (
 	// Spec should be in the format [source:]destination[:mode]
 	// Spec should be in the format [source:]destination[:mode]
 	//
 	//
@@ -94,109 +102,54 @@ func (m *MountPoint) BackwardsCompatible() bool {
 	return false
 	return false
 }
 }
 
 
-// ParseMountSpec validates the configuration of mount information is valid.
-func ParseMountSpec(spec string, volumeDriver string) (*MountPoint, error) {
-	var specExp = regexp.MustCompile(`^` + RXSource + RXDestination + RXMode + `$`)
-
-	// Ensure in platform semantics for matching. The CLI will send in Unix semantics.
-	match := specExp.FindStringSubmatch(filepath.FromSlash(strings.ToLower(spec)))
+func splitRawSpec(raw string) ([]string, error) {
+	specExp := regexp.MustCompile(`^` + RXSource + RXDestination + RXMode + `$`)
+	match := specExp.FindStringSubmatch(strings.ToLower(raw))
 
 
 	// Must have something back
 	// Must have something back
 	if len(match) == 0 {
 	if len(match) == 0 {
-		return nil, errInvalidSpec(spec)
+		return nil, errInvalidSpec(raw)
 	}
 	}
 
 
-	// Pull out the sub expressions from the named capture groups
+	var split []string
 	matchgroups := make(map[string]string)
 	matchgroups := make(map[string]string)
+	// Pull out the sub expressions from the named capture groups
 	for i, name := range specExp.SubexpNames() {
 	for i, name := range specExp.SubexpNames() {
 		matchgroups[name] = strings.ToLower(match[i])
 		matchgroups[name] = strings.ToLower(match[i])
 	}
 	}
-
-	mp := &MountPoint{
-		Source:      matchgroups["source"],
-		Destination: matchgroups["destination"],
-		RW:          true,
-	}
-	if strings.ToLower(matchgroups["mode"]) == "ro" {
-		mp.RW = false
-	}
-
-	// Volumes cannot include an explicitly supplied mode eg c:\path:rw
-	if mp.Source == "" && mp.Destination != "" && matchgroups["mode"] != "" {
-		return nil, errInvalidSpec(spec)
-	}
-
-	// Note: No need to check if destination is absolute as it must be by
-	// definition of matching the regex.
-
-	if filepath.VolumeName(mp.Destination) == mp.Destination {
-		// Ensure the destination path, if a drive letter, is not the c drive
-		if strings.ToLower(mp.Destination) == "c:" {
-			return nil, fmt.Errorf("Destination drive letter in '%s' cannot be c:", spec)
-		}
-	} else {
-		// So we know the destination is a path, not drive letter. Clean it up.
-		mp.Destination = filepath.Clean(mp.Destination)
-		// Ensure the destination path, if a path, is not the c root directory
-		if strings.ToLower(mp.Destination) == `c:\` {
-			return nil, fmt.Errorf(`Destination path in '%s' cannot be c:\`, spec)
+	if source, exists := matchgroups["source"]; exists {
+		if source != "" {
+			split = append(split, source)
 		}
 		}
 	}
 	}
-
-	// See if the source is a name instead of a host directory
-	if len(mp.Source) > 0 {
-		validName, err := IsVolumeNameValid(mp.Source)
-		if err != nil {
-			return nil, err
-		}
-		if validName {
-			// OK, so the source is a name.
-			mp.Name = mp.Source
-			mp.Source = ""
-
-			// Set the driver accordingly
-			mp.Driver = volumeDriver
-			if len(mp.Driver) == 0 {
-				mp.Driver = DefaultDriverName
-			}
-		} else {
-			// OK, so the source must be a host directory. Make sure it's clean.
-			mp.Source = filepath.Clean(mp.Source)
+	if destination, exists := matchgroups["destination"]; exists {
+		if destination != "" {
+			split = append(split, destination)
 		}
 		}
 	}
 	}
-
-	// Ensure the host path source, if supplied, exists and is a directory
-	if len(mp.Source) > 0 {
-		var fi os.FileInfo
-		var err error
-		if fi, err = os.Stat(mp.Source); err != nil {
-			return nil, fmt.Errorf("Source directory '%s' could not be found: %s", mp.Source, err)
-		}
-		if !fi.IsDir() {
-			return nil, fmt.Errorf("Source '%s' is not a directory", mp.Source)
+	if mode, exists := matchgroups["mode"]; exists {
+		if mode != "" {
+			split = append(split, mode)
 		}
 		}
 	}
 	}
-
 	// Fix #26329. If the destination appears to be a file, and the source is null,
 	// Fix #26329. If the destination appears to be a file, and the source is null,
 	// it may be because we've fallen through the possible naming regex and hit a
 	// it may be because we've fallen through the possible naming regex and hit a
 	// situation where the user intention was to map a file into a container through
 	// situation where the user intention was to map a file into a container through
 	// a local volume, but this is not supported by the platform.
 	// a local volume, but this is not supported by the platform.
-	if len(mp.Source) == 0 && len(mp.Destination) > 0 {
-		var fi os.FileInfo
-		var err error
-		if fi, err = os.Stat(mp.Destination); err == nil {
-			validName, err := IsVolumeNameValid(mp.Destination)
-			if err != nil {
-				return nil, err
-			}
-			if !validName && !fi.IsDir() {
-				return nil, fmt.Errorf("file '%s' cannot be mapped. Only directories can be mapped on this platform", mp.Destination)
+	if matchgroups["source"] == "" && matchgroups["destination"] != "" {
+		validName, err := IsVolumeNameValid(matchgroups["destination"])
+		if err != nil {
+			return nil, err
+		}
+		if !validName {
+			if fi, err := os.Stat(matchgroups["destination"]); err == nil {
+				if !fi.IsDir() {
+					return nil, fmt.Errorf("file '%s' cannot be mapped. Only directories can be mapped on this platform", matchgroups["destination"])
+				}
 			}
 			}
 		}
 		}
 	}
 	}
-
-	logrus.Debugf("MP: Source '%s', Dest '%s', RW %t, Name '%s', Driver '%s'", mp.Source, mp.Destination, mp.RW, mp.Name, mp.Driver)
-	return mp, nil
+	return split, nil
 }
 }
 
 
 // IsVolumeNameValid checks a volume name in a platform specific manner.
 // IsVolumeNameValid checks a volume name in a platform specific manner.
@@ -207,7 +160,7 @@ func IsVolumeNameValid(name string) (bool, error) {
 	}
 	}
 	nameExp = regexp.MustCompile(`^` + RXReservedNames + `$`)
 	nameExp = regexp.MustCompile(`^` + RXReservedNames + `$`)
 	if nameExp.MatchString(name) {
 	if nameExp.MatchString(name) {
-		return false, fmt.Errorf("Volume name %q cannot be a reserved word for Windows filenames", name)
+		return false, fmt.Errorf("volume name %q cannot be a reserved word for Windows filenames", name)
 	}
 	}
 	return true, nil
 	return true, nil
 }
 }
@@ -215,10 +168,46 @@ func IsVolumeNameValid(name string) (bool, error) {
 // ValidMountMode will make sure the mount mode is valid.
 // ValidMountMode will make sure the mount mode is valid.
 // returns if it's a valid mount mode or not.
 // returns if it's a valid mount mode or not.
 func ValidMountMode(mode string) bool {
 func ValidMountMode(mode string) bool {
+	if mode == "" {
+		return true
+	}
 	return roModes[strings.ToLower(mode)] || rwModes[strings.ToLower(mode)]
 	return roModes[strings.ToLower(mode)] || rwModes[strings.ToLower(mode)]
 }
 }
 
 
 // ReadWrite tells you if a mode string is a valid read-write mode or not.
 // ReadWrite tells you if a mode string is a valid read-write mode or not.
 func ReadWrite(mode string) bool {
 func ReadWrite(mode string) bool {
-	return rwModes[strings.ToLower(mode)]
+	return rwModes[strings.ToLower(mode)] || mode == ""
+}
+
+func validateNotRoot(p string) error {
+	p = strings.ToLower(convertSlash(p))
+	if p == "c:" || p == `c:\` {
+		return fmt.Errorf("destination path cannot be `c:` or `c:\\`: %v", p)
+	}
+	return nil
+}
+
+func validateCopyMode(mode bool) error {
+	if mode {
+		return fmt.Errorf("Windows does not support copying image path content")
+	}
+	return nil
+}
+
+func convertSlash(p string) string {
+	return filepath.FromSlash(p)
+}
+
+func clean(p string) string {
+	if match, _ := regexp.MatchString("^[a-z]:$", p); match {
+		return p
+	}
+	return filepath.Clean(p)
+}
+
+func validateStat(fi os.FileInfo) error {
+	if !fi.IsDir() {
+		return fmt.Errorf("source path must be a directory")
+	}
+	return nil
 }
 }