Hello,
new patch sent.
Comments are in text.
Ivana
----- Original Message -----
> From: "Jan Safranek" <[email protected]>
> To: "Ivana Hutarova Varekova" <[email protected]>
> Cc: [email protected]
> 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 <[email protected]>
> > ---
> >
> > 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
> > [email protected]
> > 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
[email protected]
https://lists.sourceforge.net/lists/listinfo/libcg-devel
