----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26071/#review56742 -----------------------------------------------------------
src/examples/test_isolator_module.cpp <https://reviews.apache.org/r/26071/#comment97136> You wired up the modules - you can put in your own info (and serve the purpose of tracking down the right author of broken modules ;-) Here and below src/examples/test_isolator_module.cpp <https://reviews.apache.org/r/26071/#comment97135> 'Test' CPU Isolator module? src/examples/test_isolator_module.cpp <https://reviews.apache.org/r/26071/#comment97137> Is this just to exercise the compatible logic? It always returns true. Please comment if that is the purpose. src/slave/containerizer/isolator.hpp <https://reviews.apache.org/r/26071/#comment97124> Can we move this to src/modules instead? src/slave/containerizer/isolators/posix.hpp <https://reviews.apache.org/r/26071/#comment97131> Why this change? Here and below. src/tests/module.hpp <https://reviews.apache.org/r/26071/#comment97125> Can you make the name and use of this "module type" more explicit? I had to think twice, why this wasn't captured by the regular module 'kind' src/tests/module.hpp <https://reviews.apache.org/r/26071/#comment97127> Can ModuleHelper and TestModule be merged? src/tests/module.hpp <https://reviews.apache.org/r/26071/#comment97133> How about moving the statics into accessors instead? Here and below src/tests/module.hpp <https://reviews.apache.org/r/26071/#comment97126> TestModule is a bit vague term for what it does (and a bit confusing mixed with out previous notions of test/example module). I can't come up with a better one on the spot, but let's think about it :-) src/tests/module.cpp <https://reviews.apache.org/r/26071/#comment97130> So the idea is that moduleNames gets populated (later on) by flags? For example ./mesos-tests --modules="{}" --module_tests="{name: "cpu_isolator", "value": "org_apache_modules_FooBarIsolator"}" ? Or is it only for internal mapping? How about using strings to map instead of the enum? You would have to keep a second map from the enum to the string eventually? src/tests/module.cpp <https://reviews.apache.org/r/26071/#comment97122> How about using the libraryExtension constant as with the previous module patch? - Niklas Nielsen On Oct. 15, 2014, 9:52 a.m., Kapil Arya wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26071/ > ----------------------------------------------------------- > > (Updated Oct. 15, 2014, 9:52 a.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/manager.cpp 8cc79956a8d3d7cb27016889ec59d138a0b58e45 > src/slave/containerizer/isolator.hpp > e52e8b15c740c62ef64b49897d3d6ae5179d4719 > src/slave/containerizer/isolators/posix.hpp > f120aafef96343d84f93c5636484509dc972a0a8 > src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 > 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 > >
