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

Reply via email to