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



This patch looks good to me in general. Could we add a simple unit test for 
these helpers?


src/linux/cgroups.hpp
Lines 442 (patched)
<https://reviews.apache.org/r/59960/#comment251763>

    Add comments.



src/linux/cgroups.hpp
Lines 442-458 (patched)
<https://reviews.apache.org/r/59960/#comment251766>

    Is it possible to unify these two struct?
    
    ```
    struct Entry
    {
      Option<Device> device;
      Option<Operation> operation;
      uint64_t value;
    
      static Try<Entry> parse(const std::string& s);
    };
    ```



src/linux/cgroups.hpp
Lines 451 (patched)
<https://reviews.apache.org/r/59960/#comment251764>

    Add comments.



src/linux/cgroups.hpp
Lines 464-568 (patched)
<https://reviews.apache.org/r/59960/#comment251767>

    Should we have corresponding `namespace` as `throttle`? Considering we have 
`cfq` and `cfq_recursive` in protobuf. E.g.,
    ```
    namespace cfq {}
    namespace cfq_recursive {}
    ```


- Gilbert Song


On June 9, 2017, 4:24 p.m., Jason Lai wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59960/
> -----------------------------------------------------------
> 
> (Updated June 9, 2017, 4:24 p.m.)
> 
> 
> Review request for mesos, Eric Chung, Xiaojian Huang, Gilbert Song, haosdent 
> huang, Jie Yu, and Zhitao Li.
> 
> 
> Bugs: MESOS-6162
>     https://issues.apache.org/jira/browse/MESOS-6162
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> * Data structure for Blkio entities 
> * Stats helpers for `blkio.throttle.io*` (generic blkio stats)
> * Stats helpers for `blkio.io*` (CFQ related stats)
> * Comments from the kernel blkio doc for helper functions
> 
> 
> Diffs
> -----
> 
>   src/linux/cgroups.hpp eaf0dcad0ed38c507564624f1647e0c731b8b433 
>   src/linux/cgroups.cpp 334005abfc4ec9b20b7dc0212d852ba1f505dbb5 
> 
> 
> Diff: https://reviews.apache.org/r/59960/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jason Lai
> 
>

Reply via email to