----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61901/#review183968 -----------------------------------------------------------
src/common/validation.cpp Lines 206 (patched) <https://reviews.apache.org/r/61901/#comment260012> Seems like you could start with adding this: ``` if (!path::absolute(volume.host_path()) && !path::absolute(volume.container_path())) { ... } ``` (Some form of which exists in the filesystem/linux isolator and Docker containerizer) src/common/validation.cpp Lines 212-217 (patched) <https://reviews.apache.org/r/61901/#comment260010> Might be clearer to count the number of occurrences rather than bit-mask them. Then error if the number of occurrences is not equal to one. --- Also, I'm surprised that we don't have this validation already. It seems like, in the existing code, if you: 1) Specify some value for all three fields. 2) Enable the appropriate isolators (filesystem/linux, docker/volume, volume/sandbox_path). 3) All three isolators will do their thing, possibly running over each other in the process. src/common/validation.cpp Lines 249-250 (patched) <https://reviews.apache.org/r/61901/#comment260007> The default case should be an error on its own. You could leave out the default case so that this code won't compile when a new Source type is defined. - Joseph Wu On Aug. 24, 2017, 4:58 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61901/ > ----------------------------------------------------------- > > (Updated Aug. 24, 2017, 4:58 p.m.) > > > Review request for mesos, Gilbert Song and Joseph Wu. > > > Bugs: MESOS-7306 > https://issues.apache.org/jira/browse/MESOS-7306 > > > Repository: mesos > > > Description > ------- > > Added common validation for Volume. > > > Diffs > ----- > > src/Makefile.am 5157b2baac4ff6c4493faaeb7a311381e8e76fe7 > src/common/validation.hpp 8c92436eb6c8aa87ffd6d6cf3f42079d6c90e525 > src/common/validation.cpp 33b501bfa6613dab70a663f552a424ffa8a15b0a > src/tests/CMakeLists.txt 6c05ea93c3288fb9608c191234f95e270390d89e > src/tests/common_validation_tests.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/61901/diff/1/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
