Pause/freeze containers during commit #6276
Conversation
| @@ -1522,6 +1522,7 @@ func (cli *DockerCli) CmdPs(args ...string) error { | |||
|
|
|||
| func (cli *DockerCli) CmdCommit(args ...string) error { | |||
| cmd := cli.Subcmd("commit", "[OPTIONS] CONTAINER [REPOSITORY[:TAG]]", "Create a new image from a container's changes") | |||
| flPause := cmd.String([]string{"p", "-pause"}, "", "Pause container during commit") | |||
There was a problem hiding this comment.
Should be a bool, false default to false
|
+1. This is crucial for making non-broken backups. |
|
Build error |
|
Yes, I broke the patch moving it from cmd.String to cmd.Bool. I'll update. On Wed, Jun 11, 2014 at 10:47 AM, Michael Crosby notifications@github.com
|
|
All I have to say is.. cool side effect. |
|
@imain Haskell programmer, I take it? In all seriousness, in order to have verifiably sane backups, you need either:
Of course, if all filesystems had snapshots, this wouldn't be an issue. However, since we live in the real world (with |
| @@ -434,6 +434,12 @@ func postCommit(eng *engine.Engine, version version.Version, w http.ResponseWrit | |||
| utils.Errorf("%s", err) | |||
| } | |||
|
|
|||
| if r.FormValue("pause") == "" && version.GreaterThanOrEqualTo("1.13") { | |||
There was a problem hiding this comment.
@ewindisch you need to bump the API version to 1.13
|
this needs to be documented |
|
As @SvenDowideit has stated above, this PR needs documentation. It also requires cli integration tests to make sure it works. |
|
@ewindisch do you have the time to do it or do you want me to ? |
|
@vieux I can do it, but not until Friday. |
|
ping @ewindisch |
|
@vieux this was updated on June 21st with a test and doc update. I'm not particularly happy with the test, to be honest, but it has one. |
|
@ewindisch sorry, feel free to ping me next time, I didn't notice. |
| during the process of commiting the image. This provides atomic copies which | ||
| prevent data corruption during the process of creating the commit. If this | ||
| behavior is undesired, set the 'p' option to false. | ||
|
|
There was a problem hiding this comment.
it pauses, but it doesn't tell the processes to flush, or wait for a process in the middle of writing, or call sync, so it doesn't prevent, it just reduces the risk?
|
@SvenDowideit semantics? I see prevention as reduction of risk rather than a cure, e.x. phrases such as "prime prevention". If you have a better turn of phrase you'd rather see, I'm welcome to suggestions of alternatives. I agree that sync should be used as well, something I intend as a follow-on, but imagine will be more controversial as there are several ways to do it with varying levels of impact to the system or Docker design (a discussion better left for later). |
|
@ewindisch can you rebase and update the 1.13 api documentation & what's new please ? |
|
@ewindisch I would avoid using the word |
|
ping @ewindisch if you're done with @SvenDowideit I can carry the pr to merge (and update the doc) |
|
@vieux: 👍 I certainly don't mind your help. I've updated the patch to rephrase the documentation and to bump the version number. It has also been rebased for good measure. |
|
LGTM - thank you for the more relaxed phrasing - might lead to one less support call :) |
Initiates a pause before committing a container,
adds a pause option to the commit command, defaulting to 'true'.
Fixes #6267
Fixes #3675
Docker-DCO-1.1-Signed-off-by: Eric Windisch ewindisch@docker.com (github: ewindisch)