----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58603/#review172699 -----------------------------------------------------------
At a high level, I'd suggest breaking this patch into several smaller patches: 1) add protobuf definitions (both unversioned and v1) 2) Add necessary helper functions for the protobuf (e.g., streaming, etc.) 3) Add new flags to agent 4) update isolator to use the new flag. docs/configuration.md Lines 1156 (patched) <https://reviews.apache.org/r/58603/#comment245759> I'd remove the `cgroups` prefix. This flag expresed a default list of devices that will be exposed to a container by default. Framework can overwrite it in the future by specifying `ContainerInfo.device_whitelist`. So, I'd rename this to: `--device_whitelist` include/mesos/mesos.proto Lines 2711 (patched) <https://reviews.apache.org/r/58603/#comment245752> 2 lines apart. See our style guide. 2 lines apart for top level definitions include/mesos/mesos.proto Lines 2713 (patched) <https://reviews.apache.org/r/58603/#comment245755> I wouldn't mention 'by path' here. In the future, we might want to describe a device by major/minor number. include/mesos/mesos.proto Lines 2715 (patched) <https://reviews.apache.org/r/58603/#comment245760> Please update v1 proto as well! include/mesos/mesos.proto Lines 2718 (patched) <https://reviews.apache.org/r/58603/#comment245753> 2 lines apart include/mesos/mesos.proto Lines 2731 (patched) <https://reviews.apache.org/r/58603/#comment245754> Ditto include/mesos/mesos.proto Lines 2732 (patched) <https://reviews.apache.org/r/58603/#comment245756> s/WhiteList/Whitelist/ Whitelist is a single english word include/mesos/mesos.proto Lines 2733 (patched) <https://reviews.apache.org/r/58603/#comment245757> s/devices/allowed_devices/ include/mesos/type_utils.hpp Line 372 (original), 377 (patched) <https://reviews.apache.org/r/58603/#comment245758> 2 lines apart - Jie Yu On April 21, 2017, 7:02 a.m., Zhongbo Tian wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58603/ > ----------------------------------------------------------- > > (Updated April 21, 2017, 7:02 a.m.) > > > Review request for mesos, haosdent huang and Jie Yu. > > > Bugs: MESOS-6791 > https://issues.apache.org/jira/browse/MESOS-6791 > > > Repository: mesos > > > Description > ------- > > Allowed whitelist additional devices in cgroups devices subsystem. > > > Diffs > ----- > > docs/configuration.md 159f946216299fc52171e0a58c7eb7c888c1eec8 > include/mesos/mesos.proto eaa2d2ac697cfc4f5aa56db0fb37363339608f43 > include/mesos/type_utils.hpp 5f771aaf2f4e76ac06bfd8f77b0b744ed2854b27 > src/common/parse.hpp e90738a91161e26a48a6e381765e631492294641 > src/common/type_utils.cpp dc0dd71f52581e2067fed279677bda8c82aa7298 > src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.hpp > ca2727142a9f257168f3cae0958f7b4665b63cf6 > src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp > 9b5cf83093796b0c0cc5057b612f80bc8b8ba72f > src/slave/flags.hpp c7a4604ed994e15c1db6accfaded2e882f1aec94 > src/slave/flags.cpp c50e43c0e0ba633f6b905b0d78668c0a0eebb173 > > > Diff: https://reviews.apache.org/r/58603/diff/1/ > > > Testing > ------- > > For test: > > - Launch without additional devices: > 1. Start agent with `sudo mesos-agent --master=127.0.0.1:5050 > --work_dir=/tmp/mesos --isolation=cgroups/devices` > 2. try open `/dev/rtc0` and failed with permission denied. `sudo > mesos-execute --master=127.0.0.1:5050 --name=test --command="head -c 0 > /dev/rtc0"` > > - Launch with additional devices: > 1. Start agent with `sudo mesos-agent --master=127.0.0.1:5050 > --work_dir=/tmp/mesos --isolation=cgroups/devices > --cgroups_whitelist_devices='{"devices":[{"device":{"path":"/dev/rtc0"}, > "access":{"mknod":true, "read":true, "write":true}}]}'` > 2. open `/dev/rtc0` successfully. `sudo mesos-execute > --master=127.0.0.1:5050 --name=test --command="head -c 0 /dev/rtc0"` > > > Thanks, > > Zhongbo Tian > >
