Hello, new patch sent. Comments are in text. Ivana ----- Original Message ----- > From: "Jan Safranek" <jsafr...@redhat.com> > To: "Ivana Hutarova Varekova" <varek...@redhat.com> > Cc: Libcg-devel@lists.sourceforge.net > Sent: Friday, September 21, 2012 2:24:33 PM > Subject: Re: [Libcg-devel] [PATCH] cgroup_change_cgroup_flags: create control > template group on the fly > > On 09/20/2012 08:37 PM, Ivana Hutarova Varekova wrote: > > cgroup_change_cgroup_flags: if wanted group is template group then > > this patch enables cgroup_change_cgroup_flags procedure to create > > the control group on the fly. It uses > > cgroup_create_cgroup procedure - will be changed in the next patch. > > Template cgroups are control groups which contains % variable like > > %U. > > > > e.g. > > @students devices people/students/%U > > will create a cgroup /people/students/john if user john from group > > students run a command and the people does not exist yet. > > > > Signed-off-by: Ivana Hutarova Varekova <varek...@redhat.com> > > --- > > > > src/api.c | 87 > > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 files changed, 87 insertions(+), 0 deletions(-) > > > > diff --git a/src/api.c b/src/api.c > > index ea75cca..2d3f4bd 100644 > > --- a/src/api.c > > +++ b/src/api.c > > @@ -2635,6 +2635,86 @@ static struct cgroup_rule > > *cgroup_find_matching_rule(uid_t uid, > > return ret; > > } > > > > +int cgroup_create_template_group(char *dest, struct cgroup_rule > > *tmp) > > It does not create *template*, it creates new group based on > template, > or do I miss something? You are right, it is group I call it "template_group" it is shorter then group based on template.
> > +{ > > + char *prefix; /* investigate cgroup */ > > + char *suffix; /* the rest of directory path not investigate yet > > */ > > + struct cgroup *template_cgroup = NULL; > > + int ret = 0; > > + int i; > > + struct cgroup_controller *controller; > > + > > + prefix = (char *)malloc(sizeof(char *) * strlen(dest)); > > + if (!prefix) { > > + last_errno = errno; > > + ret = ECGOTHER; > > + return ret; > > + } > > + > > + /* nothing investigated yet */ > > + suffix = dest; > > + > > + while (suffix != NULL) { > > + /* prefix contains "not created/tested" directory cgroup which > > + * is closest to the root in given destination path > > + */ > > + > > + /* move suffix to deeper directory and prefix to path to it */ > > + suffix = strchr(suffix, '/'); > > + if (suffix != NULL) { > > + suffix[0] = '\0'; > > + strcpy(prefix, dest); > > + suffix[0] = '/'; > > + suffix++; > > + } else { > > + strcpy(prefix, dest); > > + } > > + > > + /* we investigate prefix group now */ > > + template_cgroup = cgroup_new_cgroup(prefix); > > + if (template_cgroup == NULL) { > > + ret = ECGFAIL; > > + goto finished; > > + } > > + > > + ret = cgroup_get_cgroup(template_cgroup); > > cgroup_get_cgroup() is really heavy weight, it reads complete cgroup > from filesystem which is quite slow and completely unnecessary in > this > case. I'd rather see new function which checks only cgroup's > existence. > > In addition, if user does not have read permission to any parameter > of > any parent group, cgroup_get_cgroup() fails, while the user might > have > permissions to create and manage subgroups and the function might > succeed. fixed in new patch. > > > + if ((ret != 0) && (ret != ECGROUPNOTEXIST)) { > > + ret = ECGFAIL; > > + goto finished; > > + } > > + > > + if (ret == ECGROUPNOTEXIST) { > > + /* group prefix have to be created */ > > + cgroup_dbg("Group %s have to be created\n", prefix); > > + > > + /* add controllers */ > > + i = 0; > > + while (tmp->controllers[i] != NULL) { > > + controller = cgroup_add_controller( > > + template_cgroup, tmp->controllers[i]); > > + if (!controller) { > > + ret = ECGFAIL; > > + goto finished; > > + } > > + i++; > > + } > > + > > + /* create cgroup */ > > + ret = cgroup_create_cgroup(template_cgroup, 0); > > + if (ret != 0) { > > + cgroup_free(&template_cgroup); > > + goto finished; > > + } > > + cgroup_dbg("Group %s created\n", prefix); > > + } > > + cgroup_free(&template_cgroup); > > + } > > + > > +finished: > > + free(prefix); > > + return ret; > > +} > > + > > int cgroup_change_cgroup_flags(uid_t uid, gid_t gid, > > const char *procname, pid_t pid, int flags) > > { > > @@ -2783,7 +2863,14 @@ int cgroup_change_cgroup_flags(uid_t uid, > > gid_t gid, > > newdest[j] = tmp->destination[i]; > > } > > } > > + > > newdest[j] = 0; > > + if (strcmp(newdest, tmp->destination) != 0) { > > + /* destination tag contains templates */ > > + > > + cgroup_dbg("control group %s is template\n", newdest); > > + ret = cgroup_create_template_group(newdest, tmp); > > + } > > Umm... I hope it will be possible to override this behavior in > following > patches. Some admins might not want the groups created automatically. Yep it is a plan. > > > > /* Apply the rule */ > > ret = cgroup_change_cgroup_path(newdest, > > > > > > ------------------------------------------------------------------------------ > > Everyone hates slow websites. So do we. > > Make your web apps faster with AppDynamics > > Download AppDynamics Lite for free today: > > http://ad.doubleclick.net/clk;258768047;13503038;j? > > http://info.appdynamics.com/FreeJavaPerformanceDownload.html > > _______________________________________________ > > Libcg-devel mailing list > > Libcg-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/libcg-devel > > > > ------------------------------------------------------------------------------ Got visibility? Most devs has no idea what their production app looks like. Find out how fast your code is with AppDynamics Lite. http://ad.doubleclick.net/clk;262219671;13503038;y? http://info.appdynamics.com/FreeJavaPerformanceDownload.html _______________________________________________ Libcg-devel mailing list Libcg-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/libcg-devel