From da9ff50d73bd4e4a96232d32c4d599766064c5b0 Mon Sep 17 00:00:00 2001 From: Qiang Huang Date: Mon, 20 Jul 2015 16:10:10 +0800 Subject: [PATCH] Simplify swappiness check As suggested in https://github.com/docker/docker/pull/14004/files#r34022527 The concern there is we can't differentiate whether user explicitly asked for an invalid value of -1 or he did not specify anything. I don't think this would be a problem, because: - like all other default values like zero, we can't differentiate user specify it or not, most of which, zeros are also invalid, so default is default, we show these default values in help info, so users would know if they set value as default, it'll be like they set nothing. - we can't do this kind of string check in REST api request, so it'll make the behave different from docker command and RESTapi. Signed-off-by: Qiang Huang Upstream-commit: 6f8ddec1d0e67058c7a4a15c7d4d9a75bc1e5dea Component: engine --- components/engine/docs/reference/run.md | 3 ++- .../integration-cli/docker_cli_run_test.go | 5 ----- components/engine/runconfig/parse.go | 18 +++++------------- 3 files changed, 7 insertions(+), 19 deletions(-) diff --git a/components/engine/docs/reference/run.md b/components/engine/docs/reference/run.md index dcc9e663a5..859f3f2966 100644 --- a/components/engine/docs/reference/run.md +++ b/components/engine/docs/reference/run.md @@ -617,7 +617,8 @@ and require killing system processes to free memory. By default, a container's kernel can swap out a percentage of anonymous pages. To set this percentage for a container, specify a `--memory-swappiness` value between 0 and 100. A value of 0 turns off anonymous page swapping. A value of -100 sets all anonymous pages as swappable. +100 sets all anonymous pages as swappable. By default, if you are not using +`--memory-swappiness`, memory swappiness value will be inherited from the parent. For example, you can set: diff --git a/components/engine/integration-cli/docker_cli_run_test.go b/components/engine/integration-cli/docker_cli_run_test.go index 957c1a2ad4..986b010367 100644 --- a/components/engine/integration-cli/docker_cli_run_test.go +++ b/components/engine/integration-cli/docker_cli_run_test.go @@ -75,11 +75,6 @@ func (s *DockerSuite) TestRunWithSwappinessInvalid(c *check.C) { if err == nil { c.Fatalf("failed. test was able to set invalid value, output: %q", out) } - runCmd = exec.Command(dockerBinary, "run", "--memory-swappiness", "-1", "busybox", "true") - out, _, err = runCommandWithOutput(runCmd) - if err == nil { - c.Fatalf("failed. test was able to set invalid value, output: %q", out) - } } // "test" should be printed diff --git a/components/engine/runconfig/parse.go b/components/engine/runconfig/parse.go index 8ae0469916..b04398e5ff 100644 --- a/components/engine/runconfig/parse.go +++ b/components/engine/runconfig/parse.go @@ -73,7 +73,6 @@ func Parse(cmd *flag.FlagSet, args []string) (*Config, *HostConfig, *flag.FlagSe flStdin = cmd.Bool([]string{"i", "-interactive"}, false, "Keep STDIN open even if not attached") flTty = cmd.Bool([]string{"t", "-tty"}, false, "Allocate a pseudo-TTY") flOomKillDisable = cmd.Bool([]string{"-oom-kill-disable"}, false, "Disable OOM Killer") - flSwappinessStr = cmd.String([]string{"-memory-swappiness"}, "", "Tuning container memory swappiness (0 to 100)") flContainerIDFile = cmd.String([]string{"#cidfile", "-cidfile"}, "", "Write the container ID to the file") flEntrypoint = cmd.String([]string{"#entrypoint", "-entrypoint"}, "", "Overwrite the default ENTRYPOINT of the image") flHostname = cmd.String([]string{"h", "-hostname"}, "", "Container host name") @@ -87,6 +86,7 @@ func Parse(cmd *flag.FlagSet, args []string) (*Config, *HostConfig, *flag.FlagSe flCpusetCpus = cmd.String([]string{"#-cpuset", "-cpuset-cpus"}, "", "CPUs in which to allow execution (0-3, 0,1)") flCpusetMems = cmd.String([]string{"-cpuset-mems"}, "", "MEMs in which to allow execution (0-3, 0,1)") flBlkioWeight = cmd.Int64([]string{"-blkio-weight"}, 0, "Block IO (relative weight), between 10 and 1000") + flSwappiness = cmd.Int64([]string{"-memory-swappiness"}, -1, "Tuning container memory swappiness (0 to 100)") flNetMode = cmd.String([]string{"-net"}, "default", "Set the Network mode for the container") flMacAddress = cmd.String([]string{"-mac-address"}, "", "Container MAC address (e.g. 92:d0:c6:0a:29:33)") flIpcMode = cmd.String([]string{"-ipc"}, "", "IPC namespace to use") @@ -190,17 +190,9 @@ func Parse(cmd *flag.FlagSet, args []string) (*Config, *HostConfig, *flag.FlagSe } } - var parsedSwappiness int64 - var flSwappiness int64 - - if *flSwappinessStr != "" { - parsedSwappiness, err = strconv.ParseInt(*flSwappinessStr, 10, 64) - if err != nil || parsedSwappiness < 0 || parsedSwappiness > 100 { - return nil, nil, cmd, fmt.Errorf("invalid value:%s. valid memory swappiness range is 0-100", *flSwappinessStr) - } - flSwappiness = parsedSwappiness - } else { - flSwappiness = -1 + swappiness := *flSwappiness + if swappiness != -1 && (swappiness < 0 || swappiness > 100) { + return nil, nil, cmd, fmt.Errorf("Invalid value: %d. Valid memory swappiness range is 0-100", swappiness) } var binds []string @@ -355,7 +347,7 @@ func Parse(cmd *flag.FlagSet, args []string) (*Config, *HostConfig, *flag.FlagSe CpuQuota: *flCpuQuota, BlkioWeight: *flBlkioWeight, OomKillDisable: *flOomKillDisable, - MemorySwappiness: flSwappiness, + MemorySwappiness: swappiness, Privileged: *flPrivileged, PortBindings: portBindings, Links: flLinks.GetAll(),