volume/local.Create(): validate early

This moves validation of options to the start of the Create function
to prevent hitting the filesystem and having to remove the volume
from disk.

Also addressing some minor nits w.r.t. errors.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

volume/local/local.go

@@ -5,7 +5,6 @@ package local // import "github.com/docker/docker/volume/local"
 
 import (
 	"encoding/json"
-	"fmt"
 	"os"
 	"path/filepath"
 	"reflect"
@@ -31,7 +30,7 @@ const (
 
 var (
 	// ErrNotFound is the typed error returned when the requested volume name can't be found
-	ErrNotFound = fmt.Errorf("volume not found")
+	ErrNotFound = errors.New("volume not found")
 	// volumeNameRegex ensures the name assigned for the volume is valid.
 	// This name is used to create the bind directory, so we need to avoid characters that
 	// would make the path to escape the root directory.
@@ -137,6 +136,9 @@ func (r *Root) Create(name string, opts map[string]string) (volume.Volume, error
 	if err := r.validateName(name); err != nil {
 		return nil, err
 	}
+	if err := validateOpts(opts); err != nil {
+		return nil, err
+	}
 
 	r.m.Lock()
 	defer r.m.Unlock()
@@ -204,7 +206,7 @@ func (r *Root) Remove(v volume.Volume) error {
 	}
 
 	if lv.active.count > 0 {
-		return errdefs.System(errors.Errorf("volume has active mounts"))
+		return errdefs.System(errors.New("volume has active mounts"))
 	}
 
 	if err := lv.unmount(); err != nil {

volume/local/local_linux_test.go

@@ -134,6 +134,16 @@ func TestVolCreateValidation(t *testing.T) {
 			},
 			expectedErr: `"hello world" includes invalid characters for a local volume name, only "[a-zA-Z0-9][a-zA-Z0-9_.-]" are allowed. If you intended to pass a host directory, use absolute path`,
 		},
+		{
+			doc:         "invalid: unknown option",
+			opts:        map[string]string{"hello": "world"},
+			expectedErr: `invalid option: "hello"`,
+		},
+		{
+			doc:         "invalid: invalid size",
+			opts:        map[string]string{"size": "hello"},
+			expectedErr: `invalid size: 'hello'`,
+		},
 		{
 			doc:         "invalid: size, but no quotactl",
 			opts:        map[string]string{"size": "1234"},

volume/local/local_unix.go

@@ -51,10 +51,6 @@ func (v *localVolume) setOpts(opts map[string]string) error {
 	if len(opts) == 0 {
 		return nil
 	}
-	err := validateOpts(opts)
-	if err != nil {
-		return err
-	}
 	v.opts = &optsConfig{
 		MountType:   opts["type"],
 		MountOpts:   opts["o"],
@@ -63,10 +59,10 @@ func (v *localVolume) setOpts(opts map[string]string) error {
 	if val, ok := opts["size"]; ok {
 		size, err := units.RAMInBytes(val)
 		if err != nil {
-			return err
+			return errdefs.InvalidParameter(err)
 		}
 		if size > 0 && v.quotaCtl == nil {
-			return errdefs.InvalidParameter(errors.Errorf("quota size requested but no quota support"))
+			return errdefs.InvalidParameter(errors.New("quota size requested but no quota support"))
 		}
 		v.opts.Quota.Size = uint64(size)
 	}
@@ -82,6 +78,12 @@ func validateOpts(opts map[string]string) error {
 			return errdefs.InvalidParameter(errors.Errorf("invalid option: %q", opt))
 		}
 	}
+	if val, ok := opts["size"]; ok {
+		_, err := units.RAMInBytes(val)
+		if err != nil {
+			return errdefs.InvalidParameter(err)
+		}
+	}
 	for opt, reqopts := range mandatoryOpts {
 		if _, ok := opts[opt]; ok {
 			for _, reqopt := range reqopts {
@@ -143,7 +145,7 @@ func (v *localVolume) postMount() error {
 				return err
 			}
 		} else {
-			return fmt.Errorf("size quota requested for volume but no quota support")
+			return errors.New("size quota requested for volume but no quota support")
 		}
 	}
 	return nil

volume/local/local_windows.go

@@ -15,6 +15,11 @@ import (
 type optsConfig struct{}
 
 func (v *localVolume) setOpts(opts map[string]string) error {
+	// Windows does not support any options currently
+	return nil
+}
+
+func validateOpts(opts map[string]string) error {
 	if len(opts) > 0 {
 		return errdefs.InvalidParameter(errors.New("options are not supported on this platform"))
 	}