----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62068/#review184496 -----------------------------------------------------------
Ship it! LGTM. src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp Lines 150-152 (original), 189-197 (patched) <https://reviews.apache.org/r/62068/#comment260666> How about: ``` switch (sandboxPath->type()) { case Volume::Source::SandboxPath::SELF: /* Insert the logic found below this blob */ case Volume::Source::SandboxPath::PARENT: /* Insert the logic found below this blob */ break; default: return Failure("Unknown SANDBOX_PATH volume type"); } ``` src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp Lines 311-312 (patched) <https://reviews.apache.org/r/62068/#comment260667> While this CHECK appears valid (given validation in the `SandboxPath::SELF` code above), it doesn't seem absolutely necessary since none of the code below refers to the SandboxPath part of the protobuf. - Joseph Wu On Sept. 4, 2017, 8:34 a.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62068/ > ----------------------------------------------------------- > > (Updated Sept. 4, 2017, 8:34 a.m.) > > > Review request for mesos, Gilbert Song and Joseph Wu. > > > Bugs: MESOS-7936 > https://issues.apache.org/jira/browse/MESOS-7936 > > > Repository: mesos > > > Description > ------- > > Before this patch, the sandbox path volume logics are in two places: > the 'filesystem/linux' isolator and the 'volume/sandbox_path' > isolator, depending on the type of the sandbox path volume (SELF or > PARENT). This patch moved all the sandbox path volume related logics > to the 'volume/sandbox_path' isolator. > > > Diffs > ----- > > src/slave/containerizer/mesos/containerizer.cpp > 4ff014e278100ea99ad93f02c6688ec9ac047059 > src/slave/containerizer/mesos/isolators/filesystem/linux.cpp > bc14324087860402b0b11076837df84413383c88 > src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp > b321b86d0ddd65263fb9291d7edac98c3e579467 > > > Diff: https://reviews.apache.org/r/62068/diff/1/ > > > Testing > ------- > > sudo make check > > > Thanks, > > Jie Yu > >
