fix shm size handling

Signed-off-by: Antonio Murdaca <runcom@redhat.com>

api/client/build.go

@@ -211,18 +211,6 @@ func (cli *DockerCli) CmdBuild(args ...string) error {
 		}
 	}
 
-	var shmSize int64 = 67108864 // initial SHM size is 64MB
-	if *flShmSize != "" {
-		parsedShmSize, err := units.RAMInBytes(*flShmSize)
-		if err != nil {
-			return err
-		}
-		if parsedShmSize <= 0 {
-			return fmt.Errorf("--shm-size: SHM size must be greater than 0 . You specified: %v ", parsedShmSize)
-		}
-		shmSize = parsedShmSize
-	}
-
 	// Send the build context
 	v := url.Values{
 		"t": flTags.GetAll(),
@@ -261,9 +249,16 @@ func (cli *DockerCli) CmdBuild(args ...string) error {
 	v.Set("cpuperiod", strconv.FormatInt(*flCPUPeriod, 10))
 	v.Set("memory", strconv.FormatInt(memory, 10))
 	v.Set("memswap", strconv.FormatInt(memorySwap, 10))
-	v.Set("shmsize", strconv.FormatInt(shmSize, 10))
 	v.Set("cgroupparent", *flCgroupParent)
 
+	if *flShmSize != "" {
+		parsedShmSize, err := units.RAMInBytes(*flShmSize)
+		if err != nil {
+			return err
+		}
+		v.Set("shmsize", strconv.FormatInt(parsedShmSize, 10))
+	}
+
 	v.Set("dockerfile", relDockerfile)
 
 	ulimitsVar := flUlimits.GetList()

api/server/router/local/image.go

@@ -357,7 +357,11 @@ func (s *router) postBuild(ctx context.Context, w http.ResponseWriter, r *http.R
 	buildConfig.ForceRemove = httputils.BoolValue(r, "forcerm")
 	buildConfig.MemorySwap = httputils.Int64ValueOrZero(r, "memswap")
 	buildConfig.Memory = httputils.Int64ValueOrZero(r, "memory")
-	buildConfig.ShmSize = httputils.Int64ValueOrZero(r, "shmsize")
+	shmSize, err := httputils.Int64ValueOrDefault(r, "shmsize", runconfig.DefaultSHMSize)
+	if err != nil {
+		return errf(err)
+	}
+	buildConfig.ShmSize = &shmSize
 	buildConfig.CPUShares = httputils.Int64ValueOrZero(r, "cpushares")
 	buildConfig.CPUPeriod = httputils.Int64ValueOrZero(r, "cpuperiod")
 	buildConfig.CPUQuota = httputils.Int64ValueOrZero(r, "cpuquota")

builder/dockerfile/builder.go

@@ -60,7 +60,7 @@ type Config struct {
 
 	Memory       int64
 	MemorySwap   int64
-	ShmSize      int64
+	ShmSize      *int64
 	CPUShares    int64
 	CPUPeriod    int64
 	CPUQuota     int64

daemon/container_unix.go

@@ -1358,11 +1358,12 @@ func (daemon *Daemon) setupIpcDirs(container *Container) error {
 			return err
 		}
 
-		// When ShmSize is 0 or less, the SHM size is set to 64MB.
-		if container.hostConfig.ShmSize <= 0 {
-			container.hostConfig.ShmSize = 67108864
+		shmSize := runconfig.DefaultSHMSize
+		if container.hostConfig.ShmSize != nil {
+			shmSize = *container.hostConfig.ShmSize
 		}
-		shmproperty := "mode=1777,size=" + strconv.FormatInt(container.hostConfig.ShmSize, 10)
+
+		shmproperty := "mode=1777,size=" + strconv.FormatInt(shmSize, 10)
 		if err := syscall.Mount("shm", shmPath, "tmpfs", uintptr(syscall.MS_NOEXEC|syscall.MS_NOSUID|syscall.MS_NODEV), label.FormatMountLabel(shmproperty, container.getMountLabel())); err != nil {
 			return fmt.Errorf("mounting shm tmpfs: %s", err)
 		}

daemon/daemon_unix.go

@@ -131,6 +131,10 @@ func (daemon *Daemon) adaptContainerSettings(hostConfig *runconfig.HostConfig, a
 		// By default, MemorySwap is set to twice the size of Memory.
 		hostConfig.MemorySwap = hostConfig.Memory * 2
 	}
+	if hostConfig.ShmSize == nil {
+		shmSize := runconfig.DefaultSHMSize
+		hostConfig.ShmSize = &shmSize
+	}
 }
 
 // verifyPlatformContainerSettings performs platform-specific validation of the
@@ -144,6 +148,10 @@ func verifyPlatformContainerSettings(daemon *Daemon, hostConfig *runconfig.HostC
 		return warnings, err
 	}
 
+	if hostConfig.ShmSize != nil && *hostConfig.ShmSize <= 0 {
+		return warnings, fmt.Errorf("SHM size must be greater then 0")
+	}
+
 	// memory subsystem checks and adjustments
 	if hostConfig.Memory != 0 && hostConfig.Memory < 4194304 {
 		return warnings, fmt.Errorf("Minimum memory limit allowed is 4MB")

integration-cli/docker_api_containers_test.go

@@ -10,6 +10,7 @@ import (
 	"net/http/httputil"
 	"net/url"
 	"os"
+	"regexp"
 	"strconv"
 	"strings"
 	"time"
@@ -1388,3 +1389,138 @@ func (s *DockerSuite) TestStartWithNilDNS(c *check.C) {
 	c.Assert(err, checker.IsNil)
 	c.Assert(dns, checker.Equals, "[]")
 }
+
+func (s *DockerSuite) TestPostContainersCreateShmSizeNegative(c *check.C) {
+	config := map[string]interface{}{
+		"Image":      "busybox",
+		"HostConfig": map[string]interface{}{"ShmSize": -1},
+	}
+
+	status, body, err := sockRequest("POST", "/containers/create", config)
+	c.Assert(err, check.IsNil)
+	c.Assert(status, check.Equals, http.StatusInternalServerError)
+	c.Assert(string(body), checker.Contains, "SHM size must be greater then 0")
+}
+
+func (s *DockerSuite) TestPostContainersCreateShmSizeZero(c *check.C) {
+	config := map[string]interface{}{
+		"Image":      "busybox",
+		"HostConfig": map[string]interface{}{"ShmSize": 0},
+	}
+
+	status, body, err := sockRequest("POST", "/containers/create", config)
+	c.Assert(err, check.IsNil)
+	c.Assert(status, check.Equals, http.StatusInternalServerError)
+	c.Assert(string(body), checker.Contains, "SHM size must be greater then 0")
+}
+
+func (s *DockerSuite) TestPostContainersCreateShmSizeHostConfigOmitted(c *check.C) {
+	config := map[string]interface{}{
+		"Image": "busybox",
+		"Cmd":   "mount",
+	}
+
+	status, body, err := sockRequest("POST", "/containers/create", config)
+	c.Assert(err, check.IsNil)
+	c.Assert(status, check.Equals, http.StatusCreated)
+
+	var container types.ContainerCreateResponse
+	c.Assert(json.Unmarshal(body, &container), check.IsNil)
+
+	status, body, err = sockRequest("GET", "/containers/"+container.ID+"/json", nil)
+	c.Assert(err, check.IsNil)
+	c.Assert(status, check.Equals, http.StatusOK)
+
+	var containerJSON types.ContainerJSON
+	c.Assert(json.Unmarshal(body, &containerJSON), check.IsNil)
+
+	c.Assert(containerJSON.HostConfig.ShmSize, check.IsNil)
+
+	out, _ := dockerCmd(c, "start", "-i", containerJSON.ID)
+	shmRegexp := regexp.MustCompile(`shm on /dev/shm type tmpfs(.*)size=65536k`)
+	if !shmRegexp.MatchString(out) {
+		c.Fatalf("Expected shm of 64MB in mount command, got %v", out)
+	}
+}
+
+func (s *DockerSuite) TestPostContainersCreateShmSizeOmitted(c *check.C) {
+	config := map[string]interface{}{
+		"Image":      "busybox",
+		"HostConfig": map[string]interface{}{},
+		"Cmd":        "mount",
+	}
+
+	status, body, err := sockRequest("POST", "/containers/create", config)
+	c.Assert(err, check.IsNil)
+	c.Assert(status, check.Equals, http.StatusCreated)
+
+	var container types.ContainerCreateResponse
+	c.Assert(json.Unmarshal(body, &container), check.IsNil)
+
+	status, body, err = sockRequest("GET", "/containers/"+container.ID+"/json", nil)
+	c.Assert(err, check.IsNil)
+	c.Assert(status, check.Equals, http.StatusOK)
+
+	var containerJSON types.ContainerJSON
+	c.Assert(json.Unmarshal(body, &containerJSON), check.IsNil)
+
+	c.Assert(*containerJSON.HostConfig.ShmSize, check.Equals, runconfig.DefaultSHMSize)
+
+	out, _ := dockerCmd(c, "start", "-i", containerJSON.ID)
+	shmRegexp := regexp.MustCompile(`shm on /dev/shm type tmpfs(.*)size=65536k`)
+	if !shmRegexp.MatchString(out) {
+		c.Fatalf("Expected shm of 64MB in mount command, got %v", out)
+	}
+}
+
+func (s *DockerSuite) TestPostContainersCreateWithShmSize(c *check.C) {
+	config := map[string]interface{}{
+		"Image":      "busybox",
+		"Cmd":        "mount",
+		"HostConfig": map[string]interface{}{"ShmSize": 1073741824},
+	}
+
+	status, body, err := sockRequest("POST", "/containers/create", config)
+	c.Assert(err, check.IsNil)
+	c.Assert(status, check.Equals, http.StatusCreated)
+
+	var container types.ContainerCreateResponse
+	c.Assert(json.Unmarshal(body, &container), check.IsNil)
+
+	status, body, err = sockRequest("GET", "/containers/"+container.ID+"/json", nil)
+	c.Assert(err, check.IsNil)
+	c.Assert(status, check.Equals, http.StatusOK)
+
+	var containerJSON types.ContainerJSON
+	c.Assert(json.Unmarshal(body, &containerJSON), check.IsNil)
+
+	c.Assert(*containerJSON.HostConfig.ShmSize, check.Equals, int64(1073741824))
+
+	out, _ := dockerCmd(c, "start", "-i", containerJSON.ID)
+	shmRegex := regexp.MustCompile(`shm on /dev/shm type tmpfs(.*)size=1048576k`)
+	if !shmRegex.MatchString(out) {
+		c.Fatalf("Expected shm of 1GB in mount command, got %v", out)
+	}
+}
+
+func (s *DockerSuite) TestPostContainersCreateMemorySwappinessHostConfigOmitted(c *check.C) {
+	config := map[string]interface{}{
+		"Image": "busybox",
+	}
+
+	status, body, err := sockRequest("POST", "/containers/create", config)
+	c.Assert(err, check.IsNil)
+	c.Assert(status, check.Equals, http.StatusCreated)
+
+	var container types.ContainerCreateResponse
+	c.Assert(json.Unmarshal(body, &container), check.IsNil)
+
+	status, body, err = sockRequest("GET", "/containers/"+container.ID+"/json", nil)
+	c.Assert(err, check.IsNil)
+	c.Assert(status, check.Equals, http.StatusOK)
+
+	var containerJSON types.ContainerJSON
+	c.Assert(json.Unmarshal(body, &containerJSON), check.IsNil)
+
+	c.Assert(containerJSON.HostConfig.MemorySwappiness, check.IsNil)
+}

integration-cli/docker_cli_run_unix_test.go

@@ -9,6 +9,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"regexp"
 	"strconv"
 	"strings"
 	"time"
@@ -409,3 +410,31 @@ func (s *DockerSuite) TestRunInvalidCPUShares(c *check.C) {
 	expected = "The maximum allowed cpu-shares is"
 	c.Assert(out, checker.Contains, expected)
 }
+
+func (s *DockerSuite) TestRunWithDefaultShmSize(c *check.C) {
+	testRequires(c, DaemonIsLinux)
+
+	name := "shm-default"
+	out, _ := dockerCmd(c, "run", "--name", name, "busybox", "mount")
+	shmRegex := regexp.MustCompile(`shm on /dev/shm type tmpfs(.*)size=65536k`)
+	if !shmRegex.MatchString(out) {
+		c.Fatalf("Expected shm of 64MB in mount command, got %v", out)
+	}
+	shmSize, err := inspectField(name, "HostConfig.ShmSize")
+	c.Assert(err, check.IsNil)
+	c.Assert(shmSize, check.Equals, "67108864")
+}
+
+func (s *DockerSuite) TestRunWithShmSize(c *check.C) {
+	testRequires(c, DaemonIsLinux)
+
+	name := "shm"
+	out, _ := dockerCmd(c, "run", "--name", name, "--shm-size=1G", "busybox", "mount")
+	shmRegex := regexp.MustCompile(`shm on /dev/shm type tmpfs(.*)size=1048576k`)
+	if !shmRegex.MatchString(out) {
+		c.Fatalf("Expected shm of 1GB in mount command, got %v", out)
+	}
+	shmSize, err := inspectField(name, "HostConfig.ShmSize")
+	c.Assert(err, check.IsNil)
+	c.Assert(shmSize, check.Equals, "1073741824")
+}

runconfig/hostconfig.go

@@ -218,7 +218,7 @@ type HostConfig struct {
 	ReadonlyRootfs  bool                  // Is the container root filesystem in read-only
 	SecurityOpt     []string              // List of string values to customize labels for MLS systems, such as SELinux.
 	UTSMode         UTSMode               // UTS namespace to use for the container
-	ShmSize         int64                 // Total shm memory usage
+	ShmSize         *int64                // Total shm memory usage
 
 	// Applicable to Windows
 	ConsoleSize [2]int         // Initial console size

runconfig/parse.go

@@ -42,6 +42,9 @@ var (
 	ErrConflictNetworkExposePorts = fmt.Errorf("Conflicting options: --expose and the network mode (--net)")
 )
 
+// DefaultSHMSize is the default size (64MB) of the SHM which will be mounted in the container
+const DefaultSHMSize int64 = 67108864
+
 // Parse parses the specified args for the specified command and generates a Config,
 // a HostConfig and returns them with the specified command.
 // If the specified args are not valid, it will return an error.
@@ -201,16 +204,13 @@ func Parse(cmd *flag.FlagSet, args []string) (*Config, *HostConfig, *flag.FlagSe
 		return nil, nil, cmd, fmt.Errorf("Invalid value: %d. Valid memory swappiness range is 0-100", swappiness)
 	}
 
-	var parsedShm int64 = 67108864 // initial SHM size is 64MB
+	var parsedShm *int64
 	if *flShmSize != "" {
-		var err error
-		parsedShm, err = units.RAMInBytes(*flShmSize)
+		shmSize, err := units.RAMInBytes(*flShmSize)
 		if err != nil {
-			return nil, nil, cmd, fmt.Errorf("--shm-size: invalid SHM size")
-		}
-		if parsedShm <= 0 {
-			return nil, nil, cmd, fmt.Errorf("--shm-size: SHM size must be greater than 0 . You specified: %v ", parsedShm)
+			return nil, nil, cmd, err
 		}
+		parsedShm = &shmSize
 	}
 
 	var binds []string

runconfig/parse_test.go

@@ -525,16 +525,16 @@ func TestParseModes(t *testing.T) {
 		t.Fatalf("Expected a valid UTSMode, got %v", hostconfig.UTSMode)
 	}
 	// shm-size ko
-	if _, _, _, err = parseRun([]string{"--shm-size=a128m", "img", "cmd"}); err == nil || err.Error() != "--shm-size: invalid SHM size" {
-		t.Fatalf("Expected an error with message '--shm-size: invalid SHM size', got %v", err)
+	if _, _, _, err = parseRun([]string{"--shm-size=a128m", "img", "cmd"}); err == nil || err.Error() != "invalid size: 'a128m'" {
+		t.Fatalf("Expected an error with message 'invalid size: a128m', got %v", err)
 	}
 	// shm-size ok
 	_, hostconfig, _, err = parseRun([]string{"--shm-size=128m", "img", "cmd"})
 	if err != nil {
 		t.Fatal(err)
 	}
-	if hostconfig.ShmSize != 134217728 {
-		t.Fatalf("Expected a valid ShmSize, got %v", hostconfig.ShmSize)
+	if *hostconfig.ShmSize != 134217728 {
+		t.Fatalf("Expected a valid ShmSize, got %d", *hostconfig.ShmSize)
 	}
 }