----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51879/#review149374 -----------------------------------------------------------
src/slave/containerizer/containerizer.hpp (lines 57 - 60) <https://reviews.apache.org/r/51879/#comment216956> I think we should avoid adding the `diskSize` in the definition header of `containerizer` because it looks irrelevant. Suppose we would like to modular the containerizer, it would make us in trouble as the TODO mentioned below. ``` // TODO(idownes): Consider making this non-static and moving to // containerizer implementations to enable a containerizer to best // determine the resources, particularly if containerizeration is // delegated. static Try<Resources> resources(const Flags& flags); ``` `fs.hpp`, `resources.cpp` or `resources_utils.cpp` would be a better place? src/slave/containerizer/containerizer.cpp (line 169) <https://reviews.apache.org/r/51879/#comment216963> Use 0 to represent that determine the disk size automatically a bit nonintuitive. Please update docs/attributes-resources.md if we have to use this way eventually. src/slave/containerizer/containerizer.cpp (lines 171 - 172) <https://reviews.apache.org/r/51879/#comment216955> Should be the filesystem `work_dir` mounted instead of `root disk`? src/slave/containerizer/containerizer.cpp (lines 183 - 244) <https://reviews.apache.org/r/51879/#comment216965> I suggest that we move the logic which parse `disk(0)` to resources.cpp or resources_utils.cpp. src/slave/containerizer/containerizer.cpp (line 186) <https://reviews.apache.org/r/51879/#comment216959> `!resource.has_disk()` src/slave/containerizer/containerizer.cpp (lines 213 - 214) <https://reviews.apache.org/r/51879/#comment216958> `.disk().source().type()` should be enough? - haosdent huang On Sept. 18, 2016, 4:55 p.m., Anindya Sinha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51879/ > ----------------------------------------------------------- > > (Updated Sept. 18, 2016, 4:55 p.m.) > > > Review request for mesos and Jiang Yan Xu. > > > Bugs: MESOS-6062 > https://issues.apache.org/jira/browse/MESOS-6062 > > > Repository: mesos > > > Description > ------- > > When static resources indicate disks with a positive size, we use that > for the disk resources on the agent. However, --resources can include > disks with size of 0, which indicates that mesos agent determine the > size of those disks from the host and uses that information instead. > Note that this is not allowed for PATH disks since PATH disks can be > carved into smaller chunks and we cannot assume that the whole > physical disk is available to the PATH disk. > > With this change, JSON or textual representation for disk resources > that specify scalar value of 0 would not result in an error, but those > resources will not be accounted for until a valid size is determined > for such resources. A scalar value of -1 in JSON or textual formats > still results in an invalid resource. > > > Diffs > ----- > > src/slave/containerizer/containerizer.hpp > f13669d0dfc4ce3287cfe630cabd0470dc765b51 > src/slave/containerizer/containerizer.cpp > d46882baa904fd439bffb23c324828b777228f1c > > Diff: https://reviews.apache.org/r/51879/diff/ > > > Testing > ------- > > Tests passed. > > > Thanks, > > Anindya Sinha > >
