[lxc-devel] [PATCH] lxc_cgroup_append_task_1of3() helper_1of2

[Greg Kurz]

On Thu, 2011-10-06 at 14:59 +0200, "Axel Schöner" wrote:
> Hi,
> 
> the new patchset consists of 3 patches:
> patch_1of3_lxc_cgroup_append_task_helper_cgroup
> patch_2of3_lxc_cgroup_append_task_helper_namespace
> patch_3of3_lxc_cgroup_append_task_in_lxc_attach
> 
> I hope it is much better now.
> Thanks to Greg
> 

Axel,

Some comments below.

> 
> 
> diff --git a/src/lxc/cgroup.h b/src/lxc/cgroup.h
> index 188d948..6669815 100644
> --- a/src/lxc/cgroup.h
> +++ b/src/lxc/cgroup.h
> @@ -31,4 +31,5 @@ int lxc_cgroup_destroy(const char *name);
>  int lxc_cgroup_path_get(char **path, const char *subsystem, const char *name);
>  int lxc_cgroup_nrtasks(const char *name);
>  int lxc_ns_is_mounted(void);
> +int lxc_cgroup_append_task(const char *name, pid_t pid);
>  #endif
> diff --git a/src/lxc/cgroup.c b/src/lxc/cgroup.c
> index a2b823e..d86891b 100644
> --- a/src/lxc/cgroup.c
> +++ b/src/lxc/cgroup.c
> @@ -265,6 +265,43 @@ out:
>         return err;
>  }
> 
> +/*
> + * for each mounted cgroup, get the cgroup for the container to append a task
> + */
> +int lxc_cgroup_append_task(const char *name, pid_t pid)
> +{
> +       struct mntent *mntent;
> +       FILE *file = NULL;
> +       int err = -1;
> +       char cgname[MAXPATHLEN];

If the declaration of cgname is moved somewhere below, then we end up
with yet another function that do something for each mounted cgroup. 
It will ease work when someone decides to factor out the loop out of
get_cgroup_mount(), lxc_cgroup_create(), lxc_cgroup_destroy() and now
lxc_cgroup_append_task(). :)

> +
> +       file = setmntent(MTAB, "r");
> +       if (!file) {
> +               SYSERROR("failed to open %s", MTAB);
> +               return -1;
> +       }
> +
> +       while ((mntent = getmntent(file))) {
> +
> +               DEBUG("checking '%s' (%s)", mntent->mnt_dir, mntent->mnt_type);
> +
> +               if (!strcmp(mntent->mnt_type, "cgroup")) {
> +
> +                       INFO("found cgroup mounted at '%s'", mntent->mnt_dir);

The following lines could be moved easily to separate function, say
cgroup_attach_by_name() or something like that.

                         char cgname[MAXPATHLEN];
> +                       snprintf(cgname, MAXPATHLEN, "%s/%s", mntent->mnt_dir, name);
> +                       /* Let's add the pid to the 'tasks' file */
> +                       err = cgroup_attach(cgname, pid);
> +                       if (err) {
> +                               SYSERROR("failed to attach pid '%d' to '%s'", pid, cgname);
> +                               endmntent(file);
> +                               return err;
> +                       }
> +               }
> +       }
> +       endmntent(file);
> +       return err;
> +}
> +
> 
>  int lxc_one_cgroup_destroy(const char *cgmnt, const char *name)
>  {
> 

Cheers.

-- 
Gregory Kurz                                     gkurz at fr.ibm.com
Software Engineer @ IBM/Meiosys                  http://www.ibm.com
Tel +33 (0)534 638 479                           Fax +33 (0)561 400 420

"Anarchy is about taking complete responsibility for yourself."
        Alan Moore.