Re: Review Request 42586: Defined the NetClsHandleMgr class.

2016-02-02 Thread Avinash sridharan 

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

(Updated Feb. 3, 2016, 12:43 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebased.


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 (updated)
-

  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

Re: Review Request 42586: Defined the NetClsHandleMgr class.

2016-02-02 Thread Jie Yu 

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 74)


s/NetClsHandleMgr/NetClsHandleManager/



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 83)


s/allocMajor/allocPrimary/



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 91)


Two things:
1) Can you `s/_netClsHandles/used/`?
2) Can you just use bitset<0x1> here? No need for the constant.


- Jie Yu


On Feb. 3, 2016, 12:43 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42586/
> ---
> 
> (Updated Feb. 3, 2016, 12:43 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
> 
>

Re: Review Request 42586: Defined the NetClsHandleMgr class.

2016-02-02 Thread Avinash sridharan 


> On Feb. 1, 2016, 5:49 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp, lines 65-75
> > 
> >
> > Why do we need this?

Was using this data structure to pass on the usage of handles for a given major 
handle. It was useful in unit-testing, but thought will be useful in 
implementing a usage interface to the NetClsHandleMgr class. Should I keep it?


> On Feb. 1, 2016, 5:49 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp, lines 108-113
> > 
> >
> > We do you need this in this patch?

There are quite a few places where we are throwing erros and need to convert 
the major and minor handle into a hex string, hence thought implement this as a 
static function (maybe inline) . Should I keep this?


> On Feb. 1, 2016, 5:49 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp, line 40
> > 
> >
> > 2 blank lines above please.
> 
> Guangya Liu wrote:
> Does there are any policies for this when need one blank line and when 
> need two blank line?

The coding guide lines give the following rule:
"Elements outside classes (classes, structs, global functions, etc.) should be 
spaced apart by two empty lines."

So from that perspective this makes sense.


- Avinash


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


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
> 
>

Re: Review Request 42586: Defined the NetClsHandleMgr class.

2016-02-02 Thread Avinash sridharan 

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

(Updated Feb. 2, 2016, 5:52 p.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 (updated)
-

  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

Re: Review Request 42586: Defined the NetClsHandleMgr class.

2016-02-01 Thread Jie Yu 

---
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)


2 blank lines above please.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 43 - 44)


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)


`s/majHandle/_major/`
`s/minHandle/_minor/`

Also, remove the space right after `NetClsHandle`



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 54)


Ditto on removing the extra space.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 60 - 61)


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



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 65 - 75)


Why do we need this?



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 78 - 85)


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)


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



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 101 - 104)


```
majors(_majors)
```



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 108 - 113)


We do you need this in this patch?



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 117)


Remove the blank line here.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 118)


s/majorHandle/major/



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 121)


Remove the blank ine here.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 129)


What's the key for this map?



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 131)


`s/_majorHandles/majors/`



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (line 53)


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
> 
>

Re: Review Request 42586: Defined the NetClsHandleMgr class.

2016-02-01 Thread Guangya Liu 


> On 二月 1, 2016, 5:49 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp, line 40
> > 
> >
> > 2 blank lines above please.

Does there are any policies for this when need one blank line and when need two 
blank line?


- Guangya


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


On 二月 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 二月 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
> 
>

Re: Review Request 42586: Defined the NetClsHandleMgr class.

2016-01-31 Thread Avinash sridharan 

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

(Updated Jan. 31, 2016, 5:45 p.m.)


Review request for mesos and Jie Yu.


Bugs: MESOS-4345
https://issues.apache.org/jira/browse/MESOS-4345


Repository: mesos


Description (updated)
---

This class will be responsible for keeping track of the free and allocated
net_cls handles.


Diffs (updated)
-

  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

Re: Review Request 42586: Defined the NetClsHandleMgr class.

2016-01-31 Thread Guangya Liu 

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




src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 37 - 38)


Add a blank line here



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 44)


s/issue/issues



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 88 - 89)


"A net_cls handle is composed of a 16-bit major and a 16-bit minor handle." 
This was already mentioned above for `NetClsHandle`


- Guangya Liu


On 一月 31, 2016, 5:45 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42586/
> ---
> 
> (Updated 一月 31, 2016, 5:45 p.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
> 
>

Re: Review Request 42586: Defined the NetClsHandleMgr class.

2016-01-31 Thread Avinash sridharan 


> On Feb. 1, 2016, 1:46 a.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp, line 44
> > 
> >
> > s/issue/issues

Thanks for catching this.


- Avinash


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


On Jan. 31, 2016, 5:45 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42586/
> ---
> 
> (Updated Jan. 31, 2016, 5:45 p.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
> 
>

Re: Review Request 42586: Defined the NetClsHandleMgr class.

2016-01-31 Thread Avinash sridharan 

---
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 (updated)
-

  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

Re: Review Request 42586: Defined the NetClsHandleMgr class.

2016-01-25 Thread Avinash sridharan 

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

(Updated Jan. 26, 2016, 6:11 a.m.)


Review request for mesos and Jie Yu.


Summary (updated)
-

Defined the NetClsHandleMgr class.


Bugs: MESOS-4345
https://issues.apache.org/jira/browse/MESOS-4345


Repository: mesos


Description
---

Defined the NetClsHandleMgr class. This class will be responsible for keeping 
track of the free and allocated net_cls handles.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
7ce5243ed7476a1cfd1371b5ce25b62dc07432db 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
0798de747627ccc45b01a2668e16693757dc69a8 

Diff: https://reviews.apache.org/r/42586/diff/


Testing
---

make


Thanks,

Avinash sridharan