----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26071/#review57013 -----------------------------------------------------------
src/Makefile.am <https://reviews.apache.org/r/26071/#comment97452> 'Library containing test CPU and memory isolator modules'? src/examples/test_isolator_module.cpp <https://reviews.apache.org/r/26071/#comment97481> Let's use modu...@mesos.apache.org (it is working) instead - We don't want to generate modules traffic on the regular dev list. src/examples/test_isolator_module.cpp <https://reviews.apache.org/r/26071/#comment97480> Why 'Test' and not Test? src/slave/containerizer/isolator.hpp <https://reviews.apache.org/r/26071/#comment97474> Reinsert newline please src/tests/isolator_tests.cpp <https://reviews.apache.org/r/26071/#comment97473> Don't know if we have any specific wrapping rules wrt very long templates. This looks off to me - but let's look into that. src/tests/isolator_tests.cpp <https://reviews.apache.org/r/26071/#comment97472> same here src/tests/main.cpp <https://reviews.apache.org/r/26071/#comment97477> Mind expanding on, that the two modules lists are merged or that they compliment each other? src/tests/module.hpp <https://reviews.apache.org/r/26071/#comment97467> Mind adding a comment on why you neem this enum? src/tests/module.hpp <https://reviews.apache.org/r/26071/#comment97468> You could add a comment here too explaining how it is used with type_param'ed tests src/tests/module.cpp <https://reviews.apache.org/r/26071/#comment97479> Don't you need an include for hashmap (rather than relying on it being included in one of the headers?) Same for path::join src/tests/module.cpp <https://reviews.apache.org/r/26071/#comment97459> Mind adding a comment on how future test isolators have to be added? :-) src/tests/module.cpp <https://reviews.apache.org/r/26071/#comment97464> I have a hard time understanding how this comment ties into the functionality of initModules() Can you expand/clarify? Or does it belong to moduleNames() instead? src/tests/module.cpp <https://reviews.apache.org/r/26071/#comment97460> Please expand 'm' to a full word src/tests/module.cpp <https://reviews.apache.org/r/26071/#comment97462> Can you expand a bit on what went wrong? The error message is a bit too general / doesn't add much context. src/tests/module.cpp <https://reviews.apache.org/r/26071/#comment97478> Mind distinguish type and kind? Now, you fact called it an ID :-) src/tests/module.cpp <https://reviews.apache.org/r/26071/#comment97465> Can you include the module id value in the error message? Makes it a bit easier to debug. - Niklas Nielsen On Oct. 16, 2014, 12:20 p.m., Kapil Arya wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26071/ > ----------------------------------------------------------- > > (Updated Oct. 16, 2014, 12:20 p.m.) > > > Review request for mesos, Bernd Mathiske, Ian Downes, and Niklas Nielsen. > > > Bugs: MESOS-1384 > https://issues.apache.org/jira/browse/MESOS-1384 > > > Repository: mesos-git > > > Description > ------- > > Created CPU and Memory isolation modules based on Posix CPU/Memory Isolators. > These modules are also hooked up to the relevant Isolator tests using the > typed test mechanism. It also paves the way for future integration with > custom modules specified on the command line. > > > Diffs > ----- > > src/Makefile.am d503c8df73cda15a9d59254e8265e4a5d0e003a4 > src/examples/test_isolator_module.cpp PRE-CREATION > src/module/isolator.hpp PRE-CREATION > src/module/manager.hpp 797728a8c8e88dd1a13142a355cbe0b1491fb7a2 > src/module/manager.cpp 8cc79956a8d3d7cb27016889ec59d138a0b58e45 > src/slave/containerizer/isolator.hpp > e52e8b15c740c62ef64b49897d3d6ae5179d4719 > src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 > src/tests/main.cpp 90a7ddcb5695d4c9a7760e18285548d99e7bcfa8 > src/tests/module.hpp PRE-CREATION > src/tests/module.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/26071/diff/ > > > Testing > ------- > > make check with an Isolator module test. > > > Thanks, > > Kapil Arya > >
