On Fri, Jul 09, 2010 at 07:51:32PM -0700, Sukadev Bhattiprolu wrote:
>
> From: Sukadev Bhattiprolu <suka...@linux.vnet.ibm.com>
> Subject: [PATCH 1/2] Ensure frezer state has changed
>
> A write to the freezer.state file does not gurantee that the state has
> changed. To ensure that the freezer state is either FROZEN or THAWED,
> read the freezer state and if it has not changed, repeat the write.
Technically this is only necessary for the THAWED -> FROZEN
transition. In other words, if we're FROZEN and write THAWED then
we don't need to read the state. However, it doesn't hurt to check.
Reviewed-by: Matt Helsley <matth...@us.ibm.com>
>
> Changelog[v2]:
> - Minor reorg of code
> - Comments from Daniel Lezcano:
> - lseek() before each read/write of freezer.state
> - Have lxc_freeze_unfreeze() return -1 on error
>
> Signed-off-by: Sukadev Bhattiprolu <suka...@linux.vnet.ibm.com>
> ---
> src/lxc/freezer.c | 50 +++++++++++++++++++++++++++++++++++++++++++-------
> 1 files changed, 43 insertions(+), 7 deletions(-)
>
> diff --git a/src/lxc/freezer.c b/src/lxc/freezer.c
> index cff954e..d144f89 100644
> --- a/src/lxc/freezer.c
> +++ b/src/lxc/freezer.c
> @@ -42,6 +42,7 @@ static int freeze_unfreeze(const char *name, int freeze)
> {
> char *nsgroup;
> char freezer[MAXPATHLEN], *f;
> + char tmpf[32];
> int fd, ret;
>
> ret = lxc_cgroup_path_get(&nsgroup, name);
> @@ -50,7 +51,7 @@ static int freeze_unfreeze(const char *name, int freeze)
>
> snprintf(freezer, MAXPATHLEN, "%s/freezer.state", nsgroup);
>
> - fd = open(freezer, O_WRONLY);
> + fd = open(freezer, O_RDWR);
> if (fd < 0) {
> SYSERROR("failed to open freezer for '%s'", name);
> return -1;
> @@ -58,22 +59,57 @@ static int freeze_unfreeze(const char *name, int freeze)
>
> if (freeze) {
> f = "FROZEN";
> - ret = write(fd, f, strlen(f) + 1) < 0;
> + ret = write(fd, f, strlen(f) + 1);
> } else {
> f = "THAWED";
> - ret = write(fd, f, strlen(f) + 1) < 0;
> + ret = write(fd, f, strlen(f) + 1);
>
> /* compatibility code with old freezer interface */
> - if (ret) {
> + if (ret < 0) {
> f = "RUNNING";
Just an informational note that occurred to me while reviewing this:
This only shows up for a single commit in the mainline kernel. The very next
commit changed the string to "THAWED". It doesn't even show up in an -rcX
kernel. So the only good reason to have it is for reliable kernel bisection
of any bug cases involving lxc.
> ret = write(fd, f, strlen(f) + 1) < 0;
> }
> }
>
> - close(fd);
> - if (ret)
> - SYSERROR("failed to write to '%s'", freezer);
> + if (ret < 0) {
> + SYSERROR("failed to write '%s' to '%s'", f, freezer);
> + goto out;
> + }
> +
> + while (1) {
You could shift the write to the top of the loop body and eliminate the
writes above I think.
> + ret = lseek(fd, 0L, SEEK_SET);
> + if (ret < 0) {
> + SYSERROR("failed to lseek on file '%s'", freezer);
> + goto out;
> + }
> +
> + ret = read(fd, tmpf, sizeof(tmpf));
> + if (ret < 0) {
> + SYSERROR("failed to read to '%s'", freezer);
> + goto out;
> + }
> +
> + ret = strncmp(f, tmpf, strlen(f));
> + if (!ret)
> + break; /* Success */
>
> + sleep(1);
> +
> + ret = lseek(fd, 0L, SEEK_SET);
> + if (ret < 0) {
> + SYSERROR("failed to lseek on file '%s'", freezer);
> + goto out;
> + }
> +
> + ret = write(fd, f, strlen(f) + 1);
> + if (ret < 0) {
> + SYSERROR("failed to write '%s' to '%s'", f, freezer);
> + goto out;
> + }
> + }
> +
> +out:
> + close(fd);
> return ret;
> }
>
> --
> 1.6.0.4
>
------------------------------------------------------------------------------
This SF.net email is sponsored by Sprint
What will you do first with EVO, the first 4G phone?
Visit sprint.com/first -- http://p.sf.net/sfu/sprint-com-first
_______________________________________________
Lxc-devel mailing list
Lxc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lxc-devel
