----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46369/#review132086 -----------------------------------------------------------
include/mesos/mesos.proto (line 1737) <https://reviews.apache.org/r/46369/#comment196175> I would call it `LinuxInfo` instead. ProcessConfig sounds weird to me, even in the OCI spec. I understand things like capability, rlimit are process specific, but the only process we have control is the init process (i.e., the executor process in our case). I would rather simply that. All configurations are container level, if things like rlimit, capability are process specific, it'll be applied to the init process. include/mesos/mesos.proto (line 1739) <https://reviews.apache.org/r/46369/#comment196184> Can you move this to the top level (i.e., not nested in LinuxInfo). include/mesos/mesos.proto (line 1741) <https://reviews.apache.org/r/46369/#comment196185> Having an enum value being 0 is bad. See BenM's email about enumeration type semantics in protobuf. Basically, an unknown enum value will be mapped to 0. For instance, if in the future, linux adds a new capability. If we simply extend this list, the old master/agent (using the old protobuf) will map that to 0 (CHOWN), which is not expected. I understand that you want the value to be the same as that defined in `linux/capability.h`. But I don't think that's strictly necessary, right? I would suggest that you start the first enum at `1000` (i.e., `CHOWN = 1000`) so that you still have a systematic way to do the conversion. Also, we should set `UNKNOWN = 0`. - Jie Yu On May 6, 2016, 5:02 p.m., Jojy Varghese wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46369/ > ----------------------------------------------------------- > > (Updated May 6, 2016, 5:02 p.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-5232 > https://issues.apache.org/jira/browse/MESOS-5232 > > > Repository: mesos > > > Description > ------- > > Added capabilities support in ContanerInfo protobuf. > > > Diffs > ----- > > include/mesos/mesos.proto 9a180304996895e2e003085690a7dff9ec561e9c > include/mesos/v1/mesos.proto 44b4f8a059f9dfdcbf02f0c30c1b859898c2e617 > > Diff: https://reviews.apache.org/r/46369/diff/ > > > Testing > ------- > > make check. > > > Thanks, > > Jojy Varghese > >
