On Fri, Jun 25, 2010 at 5:33 AM, Eric Brower <[email protected]> wrote:
> Split cgroup_find_parent cgroup/parent on-disk validation from cgroup parent
> name generation.  This solves a chicken-and-egg problem for callers that need
> to obtain a parent name from a cgroup that has not yet been created, such as
> cgroup_create_cgroup_from_parent().  The new function enforces the documented
> behavior (return NULL) when a "root" cgroup is specified.
>
> Modify cgroup_create_cgroup_from_parent() to use this new function, and clean 
> up
> a few possibly-erroneous return values.
>
> diff --git a/src/api.c b/src/api.c
> index 4823006..a29fe80 100644
> --- a/src/api.c
> +++ b/src/api.c
> @@ -1428,8 +1435,56 @@ err:
>  }
>
>  /**
> + * Obtain the calculated parent name of specified cgroup; no validation
> + * of the existence of the child or parent group is performed.
> + *
> + * Given the path-like hierarchy of cgroup names, this function returns
> + * the dirname() of the cgroup name as the likely parent name; the caller
> + * is responsible for validating parent as appropriate.
> + *
> + * @param cgroup The cgroup to query for parent's name
> + * @param parent Output, name of parent's group, or NULL if the
> + *     provided cgroup is the root group.
> + *     Caller is responsible to free the returned string.
> + * @return 0 on success, > 0 on error
> + */
> +static int cgroup_get_parent_name(struct cgroup *cgroup, char **parent)
> +{
> +       int ret = 0;
> +       char *dir = NULL;
> +       char *pdir = NULL;
> +
> +       dir = strdup(cgroup->name);
> +       if (!dir) {
> +               return ECGFAIL;
> +       }
> +       cgroup_dbg("group name is %s\n", dir);
> +
> +       pdir = dirname(dir);
> +       cgroup_dbg("parent's group name is %s\n", pdir);
> +
> +       /* check for root group */
> +       if (strlen(cgroup->name) == 0 || !strcmp(cgroup->name, pdir)) {
> +               cgroup_dbg("specified cgroup \"%s\" is root group\n",
> +                       cgroup->name);
> +               *parent = NULL;
> +       }
> +       else {
> +               *parent = strdup(pdir);
> +       }
> +       free(dir);
> +
> +       if (*parent == NULL)
> +               ret = ECGFAIL;
> +
> +       return ret;
> +}
> +
> +/**
>  * Find the parent of the specified directory. It returns the parent (the
> - * parent is usually name/.. unless name is a mount point.
> + * parent is usually name/.. unless name is a mount point.  It is assumed
> + * both the cgroup (and, therefore, parent) already exist, and will fail
> + * otherwise.
>  *
>  * @param cgroup The cgroup
>  * @param parent Output, name of parent's group (if the group has parent) or
> @@ -1483,20 +1543,7 @@ static int cgroup_find_parent(struct cgroup
> *cgroup, char **parent)
>                ret = 0;
>                cgroup_dbg("Parent is on different device\n");
>        } else {
> -               dir = strdup(cgroup->name);
> -               cgroup_dbg("group name is %s\n", dir);
> -               if (!dir) {
> -                       ret = ECGFAIL;
> -                       goto free_parent;
> -               }
> -
> -               parent_dir = dirname(dir);
> -               cgroup_dbg("parent's group name is %s\n", parent_dir);
> -               *parent = strdup(parent_dir);
> -               free(dir);
> -
> -               if (*parent == NULL)
> -                       ret = ECGFAIL;
> +               ret = cgroup_get_parent_name(cgroup, parent);
>        }
>
>  free_parent:
> @@ -1519,7 +1569,7 @@ int cgroup_create_cgroup_from_parent(struct
> cgroup *cgroup,
>        if (!cgroup_initialized)
>                return ECGROUPNOTINITIALIZED;
>
> -       ret = cgroup_find_parent(cgroup, &parent);
> +       ret = cgroup_get_parent_name(cgroup, &parent);
>        if (ret)
>                return ret;
>
> @@ -1533,16 +1583,21 @@ int cgroup_create_cgroup_from_parent(struct
> cgroup *cgroup,
>
>        cgroup_dbg("parent is %s\n", parent);
>        parent_cgroup = cgroup_new_cgroup(parent);
> -       if (!parent_cgroup)
> +       if (!parent_cgroup) {
> +               ret = ECGFAIL;
>                goto err_nomem;
> +       }
>
> -       if (cgroup_get_cgroup(parent_cgroup))
> +       if (cgroup_get_cgroup(parent_cgroup)) {
> +               ret = ECGFAIL;
>                goto err_parent;
> +       }
>
>        cgroup_dbg("got parent group for %s\n", parent_cgroup->name);
>        ret = cgroup_copy_cgroup(cgroup, parent_cgroup);
> -       if (ret)
> +       if (ret) {
>                goto err_parent;
> +       }
>
>        cgroup_dbg("copied parent group %s to %s\n", parent_cgroup->name,
>                                                        cgroup->name);
>

Looks reasonable to me

Acked-by: Balbir Singh <[email protected]>

Balbir

------------------------------------------------------------------------------
This SF.net email is sponsored by Sprint
What will you do first with EVO, the first 4G phone?
Visit sprint.com/first -- http://p.sf.net/sfu/sprint-com-first
_______________________________________________
Libcg-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/libcg-devel

Reply via email to