----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44974/#review124265 -----------------------------------------------------------
src/linux/cgroups.hpp (line 629) <https://reviews.apache.org/r/44974/#comment186737> I talked with Ben (who actually has commit rights), and he'd prefer if this was just called `ListEntry`. So it becomes `cgroups::devices::ListEntry` when it actually gets used. src/linux/cgroups.hpp (lines 633 - 655) <https://reviews.apache.org/r/44974/#comment186725> I talked to Ben (who actually has commit rights), and he wants us to just smash all of these structs down into a collection of fields with no constructors. He also suggested using an Option<dev_t> for the major/minor numbers i.e.: ``` struct Selector { enum Type { ALL, BLOCK, CHARACTER }; Type type; Option<dev_t> major; Option<dev_t> minor; }; ``` This way we can attempt use the `Option<>` type to determine if the value is an integer or not. If `major/minor.isNone()`, we assume `major/minor == "*"`. If `major/minor.isSome()`, we use its `dev_t` value via `major/minor.get()`. The `dev_t` type is defined in `linux/types.h`. src/linux/cgroups.hpp (lines 659 - 668) <https://reviews.apache.org/r/44974/#comment186726> Likewise, Ben would like to remove all constructors here. src/linux/cgroups.hpp (lines 673 - 681) <https://reviews.apache.org/r/44974/#comment186727> And no constructors here either. src/linux/cgroups.cpp (lines 2427 - 2438) <https://reviews.apache.org/r/44974/#comment186739> According to the style, `case` entries should be indented by 2 spaces. src/linux/cgroups.cpp (lines 2499 - 2562) <https://reviews.apache.org/r/44974/#comment186728> These will go away. src/linux/cgroups.cpp (lines 2663 - 2697) <https://reviews.apache.org/r/44974/#comment186729> These will go away. - Kevin Klues On March 18, 2016, 9:41 a.m., Abhishek Dasgupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44974/ > ----------------------------------------------------------- > > (Updated March 18, 2016, 9:41 a.m.) > > > Review request for mesos, Ben Mahler, Felix Abecassis, Kevin Klues, and > Niklas Nielsen. > > > Bugs: MESOS-3368 > https://issues.apache.org/jira/browse/MESOS-3368 > > > Repository: mesos > > > Description > ------- > > There are some helper methods added to support device cgroups in cgroups > abstraction to aid isolators controlling access to devices. > > > Diffs > ----- > > src/linux/cgroups.hpp 51ccefd1fd2d0989b9bd31342d3d1e3701f88ed2 > src/linux/cgroups.cpp df18ed46a2a96871f67c7eb4233c3b4c27b7aa1c > > Diff: https://reviews.apache.org/r/44974/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Abhishek Dasgupta > >
