> On Dec. 13, 2018, 8:20 a.m., Gilbert Song wrote: > > include/mesos/slave/containerizer.proto > > Lines 200-215 (patched) > > <https://reviews.apache.org/r/68017/diff/10/?file=2109592#file2109592line200> > > > > I saw you remove all `SCMP_` prefix and implemented a hashmap converter > > in the next patch. You do this for `AUDIT_` architecture on pupose?
Adding `SCMP_` prefix leads to compilation errors due to interference with libseccomp's constants. > On Dec. 13, 2018, 8:20 a.m., Gilbert Song wrote: > > include/mesos/slave/containerizer.proto > > Lines 257-259 (patched) > > <https://reviews.apache.org/r/68017/diff/10/?file=2109592#file2109592line257> > > > > Do we want `Arches`? or we could leave a TODO here. I am think about > > whether we should have a linux specific proto enum for Architecture like > > S390X etc. No, we don't. Filtering of seccomp rules by architecture happens during parsing of a Seccomp profile. > On Dec. 13, 2018, 8:20 a.m., Gilbert Song wrote: > > include/mesos/slave/containerizer.proto > > Lines 262 (patched) > > <https://reviews.apache.org/r/68017/diff/10/?file=2109592#file2109592line262> > > > > Is an `action` repeated in this case? No. See https://github.com/moby/moby/blob/master/profiles/seccomp/default.json#L363 > On Dec. 13, 2018, 8:20 a.m., Gilbert Song wrote: > > include/mesos/slave/containerizer.proto > > Lines 263-264 (patched) > > <https://reviews.apache.org/r/68017/diff/10/?file=2109592#file2109592line263> > > > > Even we do not use `comments` now, but it may be used in the future. I > > would suggest to add it now with no-ops, or add a TODO _Why_ do we need `comments` in a protobuf message? The only user is a c'zer launcher process. Also, we want to keep this protobuf message as small as possible - it is serialized on disk via the `ContainerLaunchInfo` proto! > On Dec. 13, 2018, 8:20 a.m., Gilbert Song wrote: > > include/mesos/slave/containerizer.proto > > Lines 269 (patched) > > <https://reviews.apache.org/r/68017/diff/10/?file=2109592#file2109592line269> > > > > How do we add repeated `subArchitectures` under the current > > `Architecture` in the future? We add `subArchitectures` to `architectures` field (when the current `Architecture` matches the native architecure) during parsing of a Seccomp profile. See `parseArchMap()` function. - Andrei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68017/#review211264 ----------------------------------------------------------- On Nov. 8, 2018, 3:24 p.m., Andrei Budnik wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68017/ > ----------------------------------------------------------- > > (Updated Nov. 8, 2018, 3:24 p.m.) > > > Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang. > > > Bugs: MESOS-9033 > https://issues.apache.org/jira/browse/MESOS-9033 > > > Repository: mesos > > > Description > ------- > > See summary. > > > Diffs > ----- > > include/mesos/mesos.proto 06a901d26693757edc653cd833d55aa42e4ff2c6 > include/mesos/slave/containerizer.proto > 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9 > include/mesos/v1/mesos.proto 75cdb2889b2b645e23d9f5ab263ee63bf62b4221 > > > Diff: https://reviews.apache.org/r/68017/diff/10/ > > > Testing > ------- > > > Thanks, > > Andrei Budnik > >
