> On Aug. 15, 2016, 10:41 p.m., Qian Zhang wrote: > > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp, lines 87-91 > > <https://reviews.apache.org/r/51091/diff/1/?file=1472960#file1472960line87> > > > > Do we really need this? I think we have already guaranteed this in > > `MesosContainerizer::create()`, so if there is anything wrong in > > `--isolation` (e.g., `cgroups/xyz`), it must have been detected in > > `MesosContainerizer::create()`, right? > > > > If it is really needed, maybe we should do it as > > `CHECK(isolatorMap.contains(isolator))`? > > haosdent huang wrote: > We need this because we skip the check in `MesosContainerizer::create()` > > ``` > if (strings::startsWith(isolation, "cgroups/")) { > if (usedCgroups) { > // Skip when `CgroupsIsolatorProcess` have been created. > continue; > } else { > usedCgroups = true; > } > } > ``` > > Or we put this check at there?
Yes, you are right, the check will be skipped in `MesosContainerizer::create()`, so I agree with you that we need "`if (!isolatorMap.contains(isolator))`" in `CgroupsIsolatorProcess::create()`. - Qian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51091/#review145757 ----------------------------------------------------------- On Aug. 15, 2016, 9:07 p.m., haosdent huang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51091/ > ----------------------------------------------------------- > > (Updated Aug. 15, 2016, 9:07 p.m.) > > > Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha. > > > Repository: mesos > > > Description > ------- > > Disallowed unknown cgroups isolator. > > > Diffs > ----- > > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp > 7e205a3eea4ad89faccf564bdb2ad0041fdee249 > > Diff: https://reviews.apache.org/r/51091/diff/ > > > Testing > ------- > > > Thanks, > > haosdent huang > >
