----- Original Message -----
> From: "Jan Chaloupka" <jchal...@redhat.com>
> To: "Ivana Hutarova Varekova" <varek...@redhat.com>, 
> libcg-devel@lists.sourceforge.net, "Dhaval Giani"
> <dhaval.gi...@gmail.com>
> Sent: Thursday, July 24, 2014 2:46:46 PM
> Subject: Re: [Libcg-devel] [PATCH] cgcreate: use "*" character as a meta 
> character for all mounted controllers
> 
> Review comments now inlined


Thanks for the review, I put "cgcreate_add_all_controllers" function to api. 
And resend this patch as a patchset based on this change.

Add your comments regarding goto statements: I prefer to use them because the 
code seems more readable for me. They are not bogus and they are used in 
libcgroup. In libcgroup project there is no standard or recommendation not to 
use this structure. If you insist on this change, please write me your reasons 
to do this changes.

Ivana


> 
> On 07/23/2014 03:17 PM, Ivana Hutarova Varekova wrote:
> >      This patch adds the possibility to use meta character "*" as a
> >      shortcut for all mounted controllers. This meta character can be used
> >      in "-g" option.
> >      For example:
> >      $ cgcreate -g *:first  -g cpu:second
> >      $ lssubsys
> >      cpuset:/
> >      cpuset:/first
> >      cpu,cpuacct:/
> >      cpu,cpuacct:/first
> >      cpu,cpuacct:/second
> >      memory:/
> >      memory:/first
> >      devices:/
> >      devices:/first
> >      freezer:/
> >      freezer:/first
> >      net_cls,net_prio:/
> >      net_cls,net_prio:/first
> >      blkio:/
> >      blkio:/first
> >      perf_event:/
> >      perf_event:/first
> >      hugetlb:/
> >      hugetlb:/first
> >
> >
> > Signed-off-by: Ivana Hutarova Varekova <varek...@redhat.com>
> > ---
> >   doc/man/cgcreate.1   |    3 +-
> >   src/tools/cgcreate.c |   82
> >   +++++++++++++++++++++++++++++++++++++++++++++-----
> >   2 files changed, 75 insertions(+), 10 deletions(-)
> >
> > diff --git a/doc/man/cgcreate.1 b/doc/man/cgcreate.1
> > index 7068073..557b5ae 100644
> > --- a/doc/man/cgcreate.1
> > +++ b/doc/man/cgcreate.1
> > @@ -38,7 +38,8 @@ others permissions to the owners permissions).
> >   .TP
> >   .B -g <controllers>:<path>
> >   defines control groups to be added.
> > -\fBcontrollers\fR is a list of controllers and
> > +\fBcontrollers\fR is a list of controllers. Character "*" can be used
> > +as a shortcut for "all mounted controllers".
> >   \fBpath\fR is the relative path to control groups
> >   in the given controllers list. This option can be specified
> >   multiple times.
> > diff --git a/src/tools/cgcreate.c b/src/tools/cgcreate.c
> > index 73abd91..b9c9b9d 100644
> > --- a/src/tools/cgcreate.c
> > +++ b/src/tools/cgcreate.c
> > @@ -54,6 +54,57 @@ static void usage(int status, const char *program_name)
> >     printf("  -t <tuid>:<tgid>              Owner of the tasks file\n");
> >   }
> >   
> > +/* add all controllers to control cgroup structure */
> > +int cgcreate_add_all_controllers(struct cgroup *cgroup)
> 
> "cgcreate_add_all_controllers" function should be placed in
> wrapper.c as there are cgroup_add_controller and other functions working
> with controllers.
> 
> > +{
> > +   int ret;
> > +   void *handle;
> > +   struct controller_data info;
> > +   struct cgroup_controller *cgc;
> > +
> > +   /* go through the controller list */
> > +   ret = cgroup_get_all_controller_begin(&handle, &info);
> > +   if ((ret != 0) && (ret != ECGEOF)) {
> > +           fprintf(stderr, "cannot read controller data: %s\n",
> > +                   cgroup_strerror(ret));
> > +           return ret;
> > +   }
> > +
> > +   while (ret == 0) {
> 
> > +           if (info.hierarchy == 0) {
> > +                   /* the controller is not attached to any hierarchy
> > +                      skip it */
> > +                   goto next;
> > +           }
> > +
> > +           /* add mounted controller to cgroup structure */
> > +           cgc = cgroup_add_controller(cgroup, info.name);
> > +           if (!cgc) {
> > +                   ret = ECGINVAL;
> > +                   fprintf(stderr, "controller %s can't be add\n",
> > +                           info.name);
> > +           }
> > +
> > +next:
> 
> can be replaced with
> 
> /* the controller is attached to at least one hierarchy */
> if (info.hierarchy != 0) {
>           /* add mounted controller to cgroup structure */
>           cgc = cgroup_add_controller(cgroup, info.name);
>           if (!cgc) {
>               ret = ECGINVAL;
>               fprintf(stderr, "controller %s can't be add\n",
>                   info.name);
>           }
> }
> 
> > +           ret = cgroup_get_all_controller_next(&handle, &info);
> > +           if (ret && ret != ECGEOF)
> > +                   goto end;
> 
> "goto end" can be replaced with break
> 
> > +   }
> > +
> > +end:
> > +   cgroup_get_all_controller_end(&handle);
> > +
> > +   if (ret == ECGEOF)
> > +           ret = 0;
> > +
> > +   if (ret)
> > +           fprintf(stderr,
> > +                   "cgroup_get_controller_begin/next failed (%s)\n",
> > +                   cgroup_strerror(ret));
> > +
> > +   return ret;
> > +}
> > +
> >   
> >   int main(int argc, char *argv[])
> >   {
> > @@ -195,16 +246,29 @@ int main(int argc, char *argv[])
> >             /* add controllers to the new cgroup */
> >             j = 0;
> >             while (cgroup_list[i]->controllers[j]) {
> > -                   cgc = cgroup_add_controller(cgroup,
> > -                           cgroup_list[i]->controllers[j]);
> > -                   if (!cgc) {
> > -                           ret = ECGINVAL;
> > -                           fprintf(stderr, "%s: "
> > -                                   "controller %s can't be add\n",
> > -                                   argv[0],
> > +                   if (strcmp(cgroup_list[i]->controllers[j], "*") == 0) {
> > +                           /* itis meta character -> add all controllers */
> > +                           ret = cgcreate_add_all_controllers(cgroup);
> > +                           if (ret != 0) {
> > +                                   ret = ECGINVAL;
> > +                                   fprintf(stderr, "%s: can't add ",
> > +                                           argv[0]);
> > +                                   fprintf(stderr, "all controllers\n");
> > +                                   cgroup_free(&cgroup);
> > +                                   goto err;
> > +                           }
> > +                   } else {
> > +                           cgc = cgroup_add_controller(cgroup,
> >                                     cgroup_list[i]->controllers[j]);
> > -                           cgroup_free(&cgroup);
> > -                           goto err;
> > +                           if (!cgc) {
> > +                                   ret = ECGINVAL;
> > +                                   fprintf(stderr, "%s: ", argv[0]);
> > +                                   fprintf(stderr, "controller %s",
> > +                                           cgroup_list[i]->controllers[j]);
> > +                                   fprintf(stderr, "can't be add\n");
> > +                                   cgroup_free(&cgroup);
> > +                                   goto err;
> > +                           }
> >                     }
> >                     j++;
> >             }
> >
> >
> > ------------------------------------------------------------------------------
> > Want fast and easy access to all the code in your enterprise? Index and
> > search up to 200,000 lines of code with a free copy of Black Duck
> > Code Sight - the same software that powers the world's largest code
> > search on Ohloh, the Black Duck Open Hub! Try it now.
> > http://p.sf.net/sfu/bds
> > _______________________________________________
> > Libcg-devel mailing list
> > Libcg-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/libcg-devel
> 

------------------------------------------------------------------------------
Infragistics Professional
Build stunning WinForms apps today!
Reboot your WinForms applications with our WinForms controls. 
Build a bridge from your legacy apps to the future.
http://pubads.g.doubleclick.net/gampad/clk?id=153845071&iu=/4140/ostg.clktrk
_______________________________________________
Libcg-devel mailing list
Libcg-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libcg-devel

Reply via email to