Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

validate memory limits & error out if it's less than 524288 #892

Merged
merged 2 commits into from
Jun 14, 2013

Conversation

unclejack
Copy link
Contributor

Right now, we're not performing validation on the values passed as memory limits.
The following example shows what happens right now (it's not always the same, the printed errors vary):

docker run -i -m 512 ubuntu true
lxc-start: Device or resource busy - write /sys/fs/cgroup/memory/lxc/1b696c395503673593d80dd248439dbf15c1c68e6e8be0a83944bf2f0c4aea15/memory.limit_in_bytes : Device or resource busy
lxc-start: Error setting memory.limit_in_bytes to 512 for lxc/1b696c395503673593d80dd248439dbf15c1c68e6e8be0a83944bf2f0c4aea15

lxc-start: failed to setup the cgroups for '1b696c395503673593d80dd248439dbf15c1c68e6e8be0a83944bf2f0c4aea15'
lxc-start: failed to spawn '1b696c395503673593d80dd248439dbf15c1c68e6e8be0a83944bf2f0c4aea15'

# FAILED EXECUTION

Ideally, we'd want to catch this and throw an error - it's the best we can do. We can't make assumptions about the value we were given as a memory limit, so we have to set a minimum acceptable memory limit in bytes so that we can detect bad limits.

This PR makes a change in server.go so that we throw an error right away if the memory limit is set to less than 512 KB (524288).
This changes docker's behavior to this:

# let's make it fail
 docker run -i -m 16 ubuntu true
2013/06/14 19:59:20 Error: Memory limit must be given in bytes (minimum 524288 bytes)
# did we exit cleanly?
 echo $?
1
# oh, so it has to be in bytes
# let's try again
 docker run -i -m 16777216 ubuntu true
# did we exit cleanly?
 echo $?
0
# is the minimum limit real?
 docker run -i -m 524288 ubuntu true
 echo $?
0
 docker run -i -m 524287 ubuntu true
2013/06/14 20:02:48 Error: Memory limit must be given in bytes (minimum 524288 bytes)
# it looks like it is

@creack
Copy link
Contributor

creack commented Jun 14, 2013

LGTM

@creack
Copy link
Contributor

creack commented Jun 14, 2013

@unclejack Could you add a unit test?

@unclejack
Copy link
Contributor Author

@creack I'll take care of it, sure.

@unclejack
Copy link
Contributor Author

@creack I've added the test which checks that container creation fails when the memory limit is below the allowed limit.

@shykes
Copy link
Contributor

shykes commented Jun 14, 2013

LGTM

creack added a commit that referenced this pull request Jun 14, 2013
* Runtime: validate memory limits & error out if it's less than 524288
@creack creack merged commit 813771e into moby:master Jun 14, 2013
@unclejack unclejack deleted the validate_memory_limits branch June 14, 2013 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants