----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45963/#review151731 -----------------------------------------------------------
See my comment in https://reviews.apache.org/r/45962/ as well; do you think it might make more sense to add the example framework shared volume features to the existing persistent volume framework, since the code for the two frameworks is so similar? In addition to the example framework, it would be great to verify this patch with a smaller integration test - i.e., verify that when a volume is used by a task, the ownership is set correctly, and that another task without write permissions would fail when attempting RW. src/examples/persistent_shared_volume_framework.cpp (line 191) <https://reviews.apache.org/r/45963/#comment220210> s/used/uses/ src/slave/containerizer/mesos/isolators/filesystem/posix.hpp (line 55) <https://reviews.apache.org/r/45963/#comment220369> If you end up removing the check for persistent volumes in this function per Yan's comment, then perhaps we could also add a comment here saying that the function assumes that its input is a persistent volume. Renaming the resource parameter to `persistentVolume` could help make this clear as well. - Greg Mann On Oct. 5, 2016, 12:04 a.m., Anindya Sinha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45963/ > ----------------------------------------------------------- > > (Updated Oct. 5, 2016, 12:04 a.m.) > > > Review request for mesos, Greg Mann and Jiang Yan Xu. > > > Bugs: MESOS-4324 > https://issues.apache.org/jira/browse/MESOS-4324 > > > Repository: mesos > > > Description > ------- > > Allow the task to specify the persistent volume access to be read-only > or read-write. Note that the persistent volume is always created as > read-write. > If the task is the first consumer of the shared persistent volume, then > set the ownership of the persistent volume to match that of the task. > Otherwise, allow the task to be executed only if the ownership of the > persistent volume matches that of the task. > Added an option to run the test in mixed (default) mode or shared-only > mode. In mixed mode, multiple shards alternate between shared and > unshared persistent volumes for the tasks. In shared-only mode, all > shards use shared persistent volumes for the tasks. > > > Diffs > ----- > > src/examples/persistent_shared_volume_framework.cpp PRE-CREATION > src/master/validation.hpp 035f721c610ae566c89a1cf0e65ff0df11679f15 > src/master/validation.cpp e5da3c9bf0a1042b42522f1ab74ce798fbb1738d > src/slave/containerizer/mesos/isolators/filesystem/linux.hpp > 0a85935550e36c9142d845465cfa70a1634a647a > src/slave/containerizer/mesos/isolators/filesystem/linux.cpp > ea418252956c8089acc5a491888ed7f6df6cafcd > src/slave/containerizer/mesos/isolators/filesystem/posix.hpp > 794b6e5990db5f8eb21a6535872f284ca02e0553 > src/slave/containerizer/mesos/isolators/filesystem/posix.cpp > af427c6e5691f1770ab3ebef79502eb2c2176c4a > > Diff: https://reviews.apache.org/r/45963/diff/ > > > Testing > ------- > > Tests successful. > > > Thanks, > > Anindya Sinha > >
