> On June 21, 2019, 8:24 a.m., Gilbert Song wrote: > > src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp > > Lines 52-54 (patched) > > <https://reviews.apache.org/r/70798/diff/3/?file=2148577#file2148577line52> > > > > consider to return Result<string> for no parent case?
What do you mean for "no parent case"? This method will only be called for the nested container case, i.e., the container must have parent. > On June 21, 2019, 8:24 a.m., Gilbert Song wrote: > > src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp > > Lines 56 (patched) > > <https://reviews.apache.org/r/70798/diff/3/?file=2148577#file2148577line56> > > > > a bit confusing. the parentId could be assigned as a containerId > > without a parent? That is for the top-level container which does not have parent. However, I agree it is confusing, let me improve it. > On June 21, 2019, 8:24 a.m., Gilbert Song wrote: > > src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp > > Lines 90-93 (patched) > > <https://reviews.apache.org/r/70798/diff/3/?file=2148577#file2148577line90> > > > > what if for SHARE_PARENT case but the container does not have a parent? We will return `AGENT_SHM_PATH` because it is the case that a top-level container whose `ipc_mode` is `SHARE_PARENT` (i.e., it will share agent's /dev/shm). > On June 21, 2019, 8:24 a.m., Gilbert Song wrote: > > src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp > > Lines 137-143 (patched) > > <https://reviews.apache.org/r/70798/diff/3/?file=2148577#file2148577line137> > > > > is this a new dependency? Yes. > On June 21, 2019, 8:24 a.m., Gilbert Song wrote: > > src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp > > Lines 212 (patched) > > <https://reviews.apache.org/r/70798/diff/3/?file=2148577#file2148577line215> > > > > are we going to remove the `createContainerMount()` in linux filesystem > > isolator? Do you mean the `createContainerMount()` in linux filesystem isolator for /dev/shm? in https://reviews.apache.org/r/70820 I have updated linux filesystem to do `createContainerMount()` for /dev/shm only when `namespaces/ipc` isolator is not enabled. > On June 21, 2019, 8:24 a.m., Gilbert Song wrote: > > src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp > > Line 94 (original), 382 (patched) > > <https://reviews.apache.org/r/70798/diff/3/?file=2148577#file2148577line385> > > > > if we introduce cleanup(), does it mean we could avoid the dependency > > on linux filesystem isolator? No, we still need linux filesystem to ensure non-debug container has its own mount namespace and debug container enters its parent's mount namespace. We have the similar dependency in `namespaces/pid` isolator. > On June 21, 2019, 8:24 a.m., Gilbert Song wrote: > > src/slave/containerizer/mesos/paths.hpp > > Lines 85 (patched) > > <https://reviews.apache.org/r/70798/diff/3/?file=2148578#file2148578line85> > > > > how about `CONTAINER_SHM_DIRECTORY` or just `CONTAINER_SHM`? Agree! Let's go with `CONTAINER_SHM_DIRECTORY`. - Qian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70798/#review216026 ----------------------------------------------------------- On June 11, 2019, 10:33 p.m., Qian Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70798/ > ----------------------------------------------------------- > > (Updated June 11, 2019, 10:33 p.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9788 > https://issues.apache.org/jira/browse/MESOS-9788 > > > Repository: mesos > > > Description > ------- > > Improved `namespaces/ipc` isolator for configurable IPC support. > > > Diffs > ----- > > src/slave/containerizer/mesos/isolators/namespaces/ipc.hpp > 32c888309ca536d944e4d73641aed214805ccce2 > src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp > 6c8e8eed69ed355094f7d9b54bde25a8a9bc63f7 > src/slave/containerizer/mesos/paths.hpp > a5e0920bd4d753f6ba6f3b092c92cb405e9edf35 > src/slave/containerizer/mesos/paths.cpp > 4281abc522c942c87fcfd811af26f95cbd6f734f > src/tests/containerizer/isolator_tests.cpp > 9c14f3acbc19631b2f5cac4dc7cd9caba8527712 > > > Diff: https://reviews.apache.org/r/70798/diff/3/ > > > Testing > ------- > > > Thanks, > > Qian Zhang > >
