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 <glommer@parallels.com>

diff --git a/src/api.c b/src/api.c
index 147fc17fdde31e1d03627d762c7390b33944515c..ea75cca8dc50dcdc0753b94e5476acb532ffa333 100644 (file)
--- 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 1b3daf9c9ed593d8241013e20ab1c9cb074cd6e2..b253828e4467a1e29665c8c703963a679f0231e3 100644 (file)
--- 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 50b8013e5f1883aa63f17f20c04e4a8202ca4b75..2d925d2408ee903a210abed26a852f12e48dcb2c 100644 (file)
--- 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;
 
                }