-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45084/#review127895
-----------------------------------------------------------



Does this patch compile on its own? I'd like each patch to be 'atomic' so that 
we can commit some of them if they look good.


src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (lines 24 - 25)
<https://reviews.apache.org/r/45084/#comment191252>

    Instead of that, can you just include <mesos/slave/isolator.hpp>?



src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (line 32)
<https://reviews.apache.org/r/45084/#comment191253>

    Why do you need this header?



src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (line 46)
<https://reviews.apache.org/r/45084/#comment191264>

    Could you please put `flags` as the first parameter?



src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (line 48)
<https://reviews.apache.org/r/45084/#comment191262>

    Why return a raw pointer, instead of an Owned pointer here?



src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (line 53)
<https://reviews.apache.org/r/45084/#comment191263>

    Could you please put `flags` as the first parameter?



src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (line 57)
<https://reviews.apache.org/r/45084/#comment191310>

    `virtual std::string name() const = 0;`
    
    We typically do not return const string.



src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (line 28)
<https://reviews.apache.org/r/45084/#comment191255>

    Please use explicit using clauses here.



src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (lines 68 - 83)
<https://reviews.apache.org/r/45084/#comment191257>

    Since you're just adding stubs in this patch, can you introduce such logics 
later? It's hard to review if you stick in logics like this without any context.


- Jie Yu


On April 7, 2016, 10:36 a.m., haosdent huang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45084/
> -----------------------------------------------------------
> 
> (Updated April 7, 2016, 10:36 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and 
> Kevin Klues.
> 
> 
> Bugs: MESOS-5039
>     https://issues.apache.org/jira/browse/MESOS-5039
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add `Subsystem` abstraction for cgroups.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45084/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Reply via email to