[PATCH 1/4] Support named cgroups hierarchies

Paul Menage menage at google.com
Wed Jul 22 23:27:23 PDT 2009


On Wed, Jul 22, 2009 at 11:20 PM, Li Zefan<lizf at cn.fujitsu.com> wrote:
>> +The name should match [\w.-]+
>> +
>
> "[\w._-]+" ?
>
> But I double we need to check this.

\w includes '_'

>>  static int cgroup_set_super(struct super_block *sb, void *data)
>>  {
>>       int ret;
>> -     struct cgroupfs_root *root = data;
>> +     struct cgroup_sb_opts *opts = data;
>> +
>> +     /* If we don't have a new root, we can't set up a new sb */
>> +     if (!opts->new_root)
>> +             return -EINVAL;
>> +
>
> I think this should be BUG_ON(). If set_super() is called,
> we are allocating a new root, so opts->new_root won't be NULL.

Not true - if you try to mount a hierarchy by name, but with no
subsystem options, then we don't construct a new root, but we still
call sget(). If we find a superblock with the right name then we use
it, else sget() will allocate a new superblock and call
cgroup_set_super(), at which point we need to fail.

>
>> +             struct cgroupfs_root *new_root = cgroup_root_from_opts(&opts);
>
> Why not just declare new_root in the beginning of cgroup_get_sb()?

Because it's not needed for the entire scope of the function. Keeping
its scope as small as possible makes it clearer what it's being used
for.

Paul


More information about the Containers mailing list