> On June 21, 2016, 10:05 p.m., Benjamin Mahler wrote: > > Can you describe why you made this change? > > > > This follows the same pattern as other isolators (e.g. cpushare.cpp, > > mem.cpp), so it's hard to tell what the issue is here. > > Qian Zhang wrote: > I think for `cgroups/devices` isolator, we need to follow the same > pattern as `cgroups/perf_event` isolator rather than `cgroups/cpu` or > `cgroups/mem`, the reason is `cgroups/devices` isolator does not implement > its own `update()` method, so in its `prepare()`, we do not need to call > `update()`, instead we can directly return None() just like what > `cgroups/perf_event` isolator does. But for `cgroups/cpu` and `cgroups/mem` > isolators, they have their own implementation of `update()` method, so we > definitely need to call it in their `prepare()` method.
Got it, I incorrectly thought this was touching the GPU isolator rather than the devices isolator. Can you please add the reasoning into the description before I commit this? The description will become the commit message and it's a helpful way to provide context for posterity. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49013/#review138951 ----------------------------------------------------------- On June 21, 2016, 8:43 a.m., Qian Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49013/ > ----------------------------------------------------------- > > (Updated June 21, 2016, 8:43 a.m.) > > > Review request for mesos, Benjamin Mahler and Kevin Klues. > > > Bugs: MESOS-5582 > https://issues.apache.org/jira/browse/MESOS-5582 > > > Repository: mesos > > > Description > ------- > > Updated cgroups/devices isolator's prepare() to directly return None(). > > > Diffs > ----- > > src/slave/containerizer/mesos/isolators/cgroups/devices.cpp > 16f962894e89317a436a0e38465338f42bfb83ef > > Diff: https://reviews.apache.org/r/49013/diff/ > > > Testing > ------- > > > Thanks, > > Qian Zhang > >
