mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jie Yu <yujie....@gmail.com>
Subject Re: Review Request 42586: Defined the NetClsHandleMgr class.
Date Mon, 01 Feb 2016 17:49:56 GMT

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




src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 40)
<https://reviews.apache.org/r/42586/#comment178343>

    2 blank lines above please.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 43 - 44)
<https://reviews.apache.org/r/42586/#comment178344>

    I would say it's due to the fact that it depends on libnl headers and we cannot bundle
libnl since it has GPL licensing.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 51 - 52)
<https://reviews.apache.org/r/42586/#comment178346>

    `s/majHandle/_major/`
    `s/minHandle/_minor/`
    
    Also, remove the space right after `NetClsHandle`



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 54)
<https://reviews.apache.org/r/42586/#comment178347>

    Ditto on removing the extra space.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 60 - 61)
<https://reviews.apache.org/r/42586/#comment178345>

    s/majorHandle/major/
    s/minorHandle/minor/



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 65 - 75)
<https://reviews.apache.org/r/42586/#comment178357>

    Why do we need this?



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 78 - 85)
<https://reviews.apache.org/r/42586/#comment178356>

    I would suggest that we don't define a new data structure for this. Can you just inline
it in NetClsHandleManager?



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 98 - 100)
<https://reviews.apache.org/r/42586/#comment178348>

    ```
    NetClsHandleManager(const IntervalSet<uint16_t>& _majors)
      : majors(_majors) {}
    ```



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 101 - 104)
<https://reviews.apache.org/r/42586/#comment178349>

    ```
    majors(_majors)
    ```



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 108 - 113)
<https://reviews.apache.org/r/42586/#comment178350>

    We do you need this in this patch?



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 117)
<https://reviews.apache.org/r/42586/#comment178353>

    Remove the blank line here.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 118)
<https://reviews.apache.org/r/42586/#comment178351>

    s/majorHandle/major/



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 121)
<https://reviews.apache.org/r/42586/#comment178352>

    Remove the blank ine here.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 129)
<https://reviews.apache.org/r/42586/#comment178355>

    What's the key for this map?



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 131)
<https://reviews.apache.org/r/42586/#comment178354>

    `s/_majorHandles/majors/`



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (line 53)
<https://reviews.apache.org/r/42586/#comment178342>

    insert one blank line above.


- Jie Yu


On Feb. 1, 2016, 2:20 a.m., Avinash sridharan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42586/
> -----------------------------------------------------------
> 
> (Updated Feb. 1, 2016, 2:20 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4345
>     https://issues.apache.org/jira/browse/MESOS-4345
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This class will be responsible for keeping track of the free and allocated
> net_cls handles.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp b4bc52114389d1c1efce2830f4292bd89bb0de7c

>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp ddc1bf0939e5e8995e6f34fe7b8509b51704f63e

> 
> Diff: https://reviews.apache.org/r/42586/diff/
> 
> 
> Testing
> -------
> 
> make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message