Browse code

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 <h.huangqiang@huawei.com>

Qiang Huang authored on 2015/07/20 17:10:10
Showing 3 changed files
... ...
@@ -617,7 +617,8 @@ and require killing system processes to free memory.
617 617
 By default, a container's kernel can swap out a percentage of anonymous pages.
618 618
 To set this percentage for a container, specify a `--memory-swappiness` value
619 619
 between 0 and 100. A value of 0 turns off anonymous page swapping. A value of
620
-100 sets all anonymous pages as swappable.
620
+100 sets all anonymous pages as swappable. By default, if you are not using
621
+`--memory-swappiness`, memory swappiness value will be inherited from the parent.
621 622
 
622 623
 For example, you can set:
623 624
 
... ...
@@ -75,11 +75,6 @@ func (s *DockerSuite) TestRunWithSwappinessInvalid(c *check.C) {
75 75
 	if err == nil {
76 76
 		c.Fatalf("failed. test was able to set invalid value, output: %q", out)
77 77
 	}
78
-	runCmd = exec.Command(dockerBinary, "run", "--memory-swappiness", "-1", "busybox", "true")
79
-	out, _, err = runCommandWithOutput(runCmd)
80
-	if err == nil {
81
-		c.Fatalf("failed. test was able to set invalid value, output: %q", out)
82
-	}
83 78
 }
84 79
 
85 80
 // "test" should be printed
... ...
@@ -73,7 +73,6 @@ func Parse(cmd *flag.FlagSet, args []string) (*Config, *HostConfig, *flag.FlagSe
73 73
 		flStdin           = cmd.Bool([]string{"i", "-interactive"}, false, "Keep STDIN open even if not attached")
74 74
 		flTty             = cmd.Bool([]string{"t", "-tty"}, false, "Allocate a pseudo-TTY")
75 75
 		flOomKillDisable  = cmd.Bool([]string{"-oom-kill-disable"}, false, "Disable OOM Killer")
76
-		flSwappinessStr   = cmd.String([]string{"-memory-swappiness"}, "", "Tuning container memory swappiness (0 to 100)")
77 76
 		flContainerIDFile = cmd.String([]string{"#cidfile", "-cidfile"}, "", "Write the container ID to the file")
78 77
 		flEntrypoint      = cmd.String([]string{"#entrypoint", "-entrypoint"}, "", "Overwrite the default ENTRYPOINT of the image")
79 78
 		flHostname        = cmd.String([]string{"h", "-hostname"}, "", "Container host name")
... ...
@@ -87,6 +86,7 @@ func Parse(cmd *flag.FlagSet, args []string) (*Config, *HostConfig, *flag.FlagSe
87 87
 		flCpusetCpus      = cmd.String([]string{"#-cpuset", "-cpuset-cpus"}, "", "CPUs in which to allow execution (0-3, 0,1)")
88 88
 		flCpusetMems      = cmd.String([]string{"-cpuset-mems"}, "", "MEMs in which to allow execution (0-3, 0,1)")
89 89
 		flBlkioWeight     = cmd.Int64([]string{"-blkio-weight"}, 0, "Block IO (relative weight), between 10 and 1000")
90
+		flSwappiness      = cmd.Int64([]string{"-memory-swappiness"}, -1, "Tuning container memory swappiness (0 to 100)")
90 91
 		flNetMode         = cmd.String([]string{"-net"}, "default", "Set the Network mode for the container")
91 92
 		flMacAddress      = cmd.String([]string{"-mac-address"}, "", "Container MAC address (e.g. 92:d0:c6:0a:29:33)")
92 93
 		flIpcMode         = cmd.String([]string{"-ipc"}, "", "IPC namespace to use")
... ...
@@ -190,17 +190,9 @@ func Parse(cmd *flag.FlagSet, args []string) (*Config, *HostConfig, *flag.FlagSe
190 190
 		}
191 191
 	}
192 192
 
193
-	var parsedSwappiness int64
194
-	var flSwappiness int64
195
-
196
-	if *flSwappinessStr != "" {
197
-		parsedSwappiness, err = strconv.ParseInt(*flSwappinessStr, 10, 64)
198
-		if err != nil || parsedSwappiness < 0 || parsedSwappiness > 100 {
199
-			return nil, nil, cmd, fmt.Errorf("invalid value:%s. valid memory swappiness range is 0-100", *flSwappinessStr)
200
-		}
201
-		flSwappiness = parsedSwappiness
202
-	} else {
203
-		flSwappiness = -1
193
+	swappiness := *flSwappiness
194
+	if swappiness != -1 && (swappiness < 0 || swappiness > 100) {
195
+		return nil, nil, cmd, fmt.Errorf("Invalid value: %d. Valid memory swappiness range is 0-100", swappiness)
204 196
 	}
205 197
 
206 198
 	var binds []string
... ...
@@ -355,7 +347,7 @@ func Parse(cmd *flag.FlagSet, args []string) (*Config, *HostConfig, *flag.FlagSe
355 355
 		CpuQuota:         *flCpuQuota,
356 356
 		BlkioWeight:      *flBlkioWeight,
357 357
 		OomKillDisable:   *flOomKillDisable,
358
-		MemorySwappiness: flSwappiness,
358
+		MemorySwappiness: swappiness,
359 359
 		Privileged:       *flPrivileged,
360 360
 		PortBindings:     portBindings,
361 361
 		Links:            flLinks.GetAll(),