----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42992/#review117089 -----------------------------------------------------------
I think that you also need to update all of the summary of the patch chain to make sure it starts with a capital letter, otherwise, you will not able to apply your patch. src/common/resources.cpp (lines 733 - 737) <https://reviews.apache.org/r/42992/#comment178139> Can you please add more accurate error message for this? Such as if (resource.name() != "disk") { return Error( "ShareInfo should not be set for " + resource.name() + " resource"); } if (!resource.has_disk() || !resource.disk().has_persistence()) { return Error( "Only persistent volumes can be shared"); } src/common/resources.cpp (line 899) <https://reviews.apache.org/r/42992/#comment178154> Do we also need to check contain here? I think that the unsharable should be failed if the current resoures does not contain the unshared resources. src/common/resources.cpp (line 1134) <https://reviews.apache.org/r/42992/#comment178149> Here not checking "if the volume exists" but "if the volume is a subset of current resources" src/common/resources.cpp (lines 1652 - 1654) <https://reviews.apache.org/r/42992/#comment178155> I think that we already set the num consumers as 0 when initialize, so when the logic can go here? What is the difference of {None} and {0} here? src/common/resources_utils.cpp (lines 71 - 73) <https://reviews.apache.org/r/42992/#comment178156> Do we need to move this under 68 under `isPersistentVolume` condition section? src/master/master.cpp (line 3220) <https://reviews.apache.org/r/42992/#comment178158> Remove @ - Guangya Liu On 一月 30, 2016, 12:26 a.m., Anindya Sinha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42992/ > ----------------------------------------------------------- > > (Updated 一月 30, 2016, 12:26 a.m.) > > > Review request for mesos and Adam B. > > > Bugs: MESOS-4431 > https://issues.apache.org/jira/browse/MESOS-4431 > > > Repository: mesos > > > Description > ------- > > * Added new Offer::Operation of SHARE and UNSHARE for resources. > * Added ShareInfo within Resources protobuf to allow for sharing of resources > and keep track of consumers of such resources. > * Allow DESTROY or UNSHARE for shared volumes only if reference count is 0. > > > Diffs > ----- > > include/mesos/mesos.proto 194750e92020753e60154083a47bdc3398d31466 > include/mesos/resources.hpp 6bfac2e7e8799e74d87c7570fc5eef320ba76eb1 > include/mesos/v1/mesos.proto 1102bbc92f46f97c1915c03a71c7cf829003e0ed > include/mesos/v1/resources.hpp 5a88c0756db2ea8db0f5df7ea3019b511ea135af > src/common/resources.cpp 588a279c3cdebbeb58047bfbff5e78a42f53fc13 > src/common/resources_utils.cpp 70e6f025d89383084ab8b2cda23ab1cd55d959b2 > src/master/allocator/mesos/hierarchical.cpp > 1a07d69016407e5aad2209586da37fecbcddb765 > src/master/allocator/sorter/drf/sorter.cpp > db47d640e36c0302d7c6254a9c58caa878feac01 > src/master/master.hpp 3a7e18232323a1c051bcc97915484b1195fffe58 > src/master/master.cpp 98441a543611d4083b2495ee103f0ab5e2187e83 > src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c > src/master/validation.cpp f2bc1bad79e3b0812c019be3774cd65b58ea2d07 > src/v1/resources.cpp be4a5d153e9313cb71a6e85d1ed25a358537f2b7 > > Diff: https://reviews.apache.org/r/42992/diff/ > > > Testing > ------- > > make check done. > > > Thanks, > > Anindya Sinha > >