----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51274/#review146411 -----------------------------------------------------------
src/slave/containerizer/mesos/launcher.cpp (lines 50 - 64) <https://reviews.apache.org/r/51274/#comment212833> I'm fine with changing this since it is essentially the same logic (it's just unclear why it needed to be changed). It's clearer to me as a loop. Also, adding the suffix `ForContainer` seems redundant for the same reason mentioned below. src/slave/containerizer/mesos/launcher.cpp (line 65) <https://reviews.apache.org/r/51274/#comment212837> Adding the suffix "ForContainer" seems redundant here since we pass it a containerID. I'd probably just call it `getRuntimePath()`. - Kevin Klues On Aug. 22, 2016, 4:54 a.m., Benjamin Hindman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51274/ > ----------------------------------------------------------- > > (Updated Aug. 22, 2016, 4:54 a.m.) > > > Review request for mesos, Jie Yu and Kevin Klues. > > > Repository: mesos > > > Description > ------- > > Refactorings include: > > * 'buildPathFromHierarchy' was both reimplemented recursively for > easy of understanding and renamed to 'buildPathForContainer' in an > effort to better capture it's intended use case (i.e., "build a > path to be used to represent a container in cgroup subsystems like > the freezer"). Ultimately I hope that all isolators use this > function to "name" their cgroups/containers. > > * Added a new function 'getRuntimePathForContainer' that takes a > 'flags' argument and computes the runtime path using > 'flags.launcher' rather than having to have a POSIX_LAUNCHER_NAME > and LINUX_LAUNCHER_NAME. This was made possibly by making > 'flags.launcher' always have a value in a previous commit. > > > Diffs > ----- > > src/slave/containerizer/mesos/launcher.hpp > 0eae09515d550a2c71ae1414d4a22943f1d09db9 > src/slave/containerizer/mesos/launcher.cpp > 413a8afdc56127a58c9599c436d17d6c98e62434 > src/slave/containerizer/mesos/linux_launcher.hpp > 8fbe1e9742df85b0fc6de46ac81a0c064c845a63 > src/slave/containerizer/mesos/linux_launcher.cpp > 7377316776646e3d660086da3c0d5b494ce8ace4 > > Diff: https://reviews.apache.org/r/51274/diff/ > > > Testing > ------- > > > Thanks, > > Benjamin Hindman > >
