From 64d054a62a3deb2be8d0b93e51282aa8c227eb5c Mon Sep 17 00:00:00 2001 From: Glauber Costa Date: Fri, 27 Jul 2012 12:55:55 +0200 Subject: [PATCH] Fix cgroup_modify_cgroup gratuitous failure Around one year ago, the following was reported: http://comments.gmane.org/gmane.comp.lib.libcg.devel/3116 (Error when calling cgroup_modify_cgroup()) I ran into the very same error. Inspecting the code in libcg, it seemed to me that the best thing to avoid that is to never attempt to write something the user never wrote to. That is because if the user actually tries to write to a read-only file, we should yield an error, making skipping read-only a bad solution. My solution is to add a field to the value structure indicating whether or not it is dirty. That value will indicate whether or not an error in the write-to-filesystem routine is considered fatal or not. Non-dirty values will still be written, but their failures are not considered fatal. cgroup_modify_cgroup then becomes a simple flusher, and the problem goes away. [ v2: Also mark dirty value writes using cgroup_set_value_* ] [ v3: fail if write fails only for dirty values ] Signed-off-by: Glauber Costa --- src/api.c | 12 +++++++++++- src/libcgroup-internal.h | 1 + src/wrapper.c | 5 +++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/api.c b/src/api.c index 147fc17f..ea75cca8 100644 --- a/src/api.c +++ b/src/api.c @@ -1441,8 +1441,15 @@ int cgroup_modify_cgroup(struct cgroup *cgroup) cgroup->controller[i]->values[j]->value); free(path); path = NULL; + /* don't consider error in files directly written by + * the user as fatal */ + if (error && !cgroup->controller[i]->values[j]->dirty) { + error = 0; + continue; + } if (error) goto err; + cgroup->controller[i]->values[j]->dirty = false; } } err: @@ -2327,7 +2334,7 @@ fill_error: */ int cgroup_get_cgroup(struct cgroup *cgroup) { - int i; + int i, j; char path[FILENAME_MAX]; DIR *dir = NULL; struct dirent *ctrl_dir = NULL; @@ -2421,6 +2428,9 @@ int cgroup_get_cgroup(struct cgroup *cgroup) continue; error = cgroup_fill_cgc(ctrl_dir, cgroup, cgc, i); + for (j = 0; j < cgc->index; j++) + cgc->values[j]->dirty = false; + if (error == ECGFAIL) { closedir(dir); goto unlock_error; diff --git a/src/libcgroup-internal.h b/src/libcgroup-internal.h index 1b3daf9c..b253828e 100644 --- a/src/libcgroup-internal.h +++ b/src/libcgroup-internal.h @@ -71,6 +71,7 @@ __BEGIN_DECLS struct control_value { char name[FILENAME_MAX]; char value[CG_VALUE_MAX]; + bool dirty; }; struct cgroup_controller { diff --git a/src/wrapper.c b/src/wrapper.c index 50b8013e..2d925d24 100644 --- a/src/wrapper.c +++ b/src/wrapper.c @@ -146,6 +146,7 @@ int cgroup_add_value_string(struct cgroup_controller *controller, strncpy(cntl_value->name, name, sizeof(cntl_value->name)); strncpy(cntl_value->value, value, sizeof(cntl_value->value)); + cntl_value->dirty = true; controller->values[controller->index] = cntl_value; controller->index++; @@ -355,6 +356,7 @@ int cgroup_set_value_string(struct cgroup_controller *controller, struct control_value *val = controller->values[i]; if (!strcmp(val->name, name)) { strncpy(val->value, value, CG_VALUE_MAX); + val->dirty = true; return 0; } } @@ -404,6 +406,7 @@ int cgroup_set_value_int64(struct cgroup_controller *controller, if (ret >= sizeof(val->value) || ret < 0) return ECGINVAL; + val->dirty = true; return 0; } } @@ -452,6 +455,7 @@ int cgroup_set_value_uint64(struct cgroup_controller *controller, if (ret >= sizeof(val->value) || ret < 0) return ECGINVAL; + val->dirty = true; return 0; } } @@ -511,6 +515,7 @@ int cgroup_set_value_bool(struct cgroup_controller *controller, if (ret >= sizeof(val->value) || ret < 0) return ECGINVAL; + val->dirty = true; return 0; } -- 2.47.2