Re: Review Request 43096: Added helper function in cgroup for supporting net_cls subsystem.

[Avinash sridharan]

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

(Updated Feb. 8, 2016, 7:16 p.m.)


Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Added helper function in cgroup for supporting net_cls subsystem.


Diffs
-

  src/linux/cgroups.hpp 83b3e401ac69194bef496087dee32e0532b834ab 
  src/linux/cgroups.cpp bbc1fb3c60e53dffb1e8029942725e7102eb9aee 

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


Testing
---

make and make check.


Thanks,

Avinash sridharan

Re: Review Request 43096: Added helper function in cgroup for supporting net_cls subsystem.

[Avinash sridharan]

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

(Updated Feb. 5, 2016, 10:22 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebase.


Repository: mesos


Description
---

Added helper function in cgroup for supporting net_cls subsystem.


Diffs (updated)
-

  src/linux/cgroups.hpp 83b3e401ac69194bef496087dee32e0532b834ab 
  src/linux/cgroups.cpp bbc1fb3c60e53dffb1e8029942725e7102eb9aee 

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


Testing
---

make and make check.


Thanks,

Avinash sridharan

Re: Review Request 43096: Added helper function in cgroup for supporting net_cls subsystem.

[Jie Yu]

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


Fix it, then Ship it!





src/linux/cgroups.cpp (line 2471)


use ' instead of `



src/linux/cgroups.cpp (line 2488)


no need for this temp variable.


- Jie Yu


On Feb. 3, 2016, 10:41 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43096/
> ---
> 
> (Updated Feb. 3, 2016, 10:41 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helper function in cgroup for supporting net_cls subsystem.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp 83b3e401ac69194bef496087dee32e0532b834ab 
>   src/linux/cgroups.cpp bbc1fb3c60e53dffb1e8029942725e7102eb9aee 
> 
> Diff: https://reviews.apache.org/r/43096/diff/
> 
> 
> Testing
> ---
> 
> make and make check.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 43096: Added helper function in cgroup for supporting net_cls subsystem.

[Avinash sridharan]

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

(Updated Feb. 5, 2016, 1:22 a.m.)


Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Added helper function in cgroup for supporting net_cls subsystem.


Diffs (updated)
-

  src/linux/cgroups.hpp 83b3e401ac69194bef496087dee32e0532b834ab 
  src/linux/cgroups.cpp bbc1fb3c60e53dffb1e8029942725e7102eb9aee 

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


Testing
---

make and make check.


Thanks,

Avinash sridharan

Re: Review Request 43096: Added helper function in cgroup for supporting net_cls subsystem.

[Avinash sridharan]

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

(Updated Feb. 3, 2016, 9:17 p.m.)


Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Added helper function in cgroup for supporting net_cls subsystem.


Diffs (updated)
-

  src/linux/cgroups.hpp 83b3e401ac69194bef496087dee32e0532b834ab 
  src/linux/cgroups.cpp bbc1fb3c60e53dffb1e8029942725e7102eb9aee 

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


Testing
---

make and make check.


Thanks,

Avinash sridharan

Re: Review Request 43096: Added helper function in cgroup for supporting net_cls subsystem.

[Avinash sridharan]

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

(Updated Feb. 3, 2016, 10:41 p.m.)


Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Added helper function in cgroup for supporting net_cls subsystem.


Diffs (updated)
-

  src/linux/cgroups.hpp 83b3e401ac69194bef496087dee32e0532b834ab 
  src/linux/cgroups.cpp bbc1fb3c60e53dffb1e8029942725e7102eb9aee 

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


Testing
---

make and make check.


Thanks,

Avinash sridharan

Re: Review Request 43096: Added helper function in cgroup for supporting net_cls subsystem.

[Jie Yu]

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




src/linux/cgroups.hpp (lines 656 - 657)


I would still use multiple lines here because the code surrounding uses the 
same pattern. we value consistency in our code base.

The indentation here is not correct. I'll let you find out by looking at 
the surrounding code.



src/linux/cgroups.hpp (lines 662 - 664)


Ditto on indentation.



src/linux/cgroups.cpp (line 2488)


Have you checked out the impl. of 'stringify'? Does it return a 'Try'?


- Jie Yu


On Feb. 3, 2016, 7:17 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43096/
> ---
> 
> (Updated Feb. 3, 2016, 7:17 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helper function in cgroup for supporting net_cls subsystem.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp 83b3e401ac69194bef496087dee32e0532b834ab 
>   src/linux/cgroups.cpp bbc1fb3c60e53dffb1e8029942725e7102eb9aee 
> 
> Diff: https://reviews.apache.org/r/43096/diff/
> 
> 
> Testing
> ---
> 
> make and make check.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 43096: Added helper function in cgroup for supporting net_cls subsystem.

[Avinash sridharan]

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

(Updated Feb. 2, 2016, 7:05 p.m.)


Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Added helper function in cgroup for supporting net_cls subsystem.


Diffs (updated)
-

  src/linux/cgroups.hpp 83b3e401ac69194bef496087dee32e0532b834ab 
  src/linux/cgroups.cpp bbc1fb3c60e53dffb1e8029942725e7102eb9aee 

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


Testing
---

make and make check.


Thanks,

Avinash sridharan

Re: Review Request 43096: Added helper function in cgroup for supporting net_cls subsystem.

[Avinash sridharan]

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

(Updated Feb. 3, 2016, 7:17 a.m.)


Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Added helper function in cgroup for supporting net_cls subsystem.


Diffs (updated)
-

  src/linux/cgroups.hpp 83b3e401ac69194bef496087dee32e0532b834ab 
  src/linux/cgroups.cpp bbc1fb3c60e53dffb1e8029942725e7102eb9aee 

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


Testing
---

make and make check.


Thanks,

Avinash sridharan

Re: Review Request 43096: Added helper function in cgroup for supporting net_cls subsystem.

[Avinash sridharan]

> On Feb. 3, 2016, 1:32 a.m., Jie Yu wrote:
> > src/linux/cgroups.cpp, line 2489
> > 
> >
> > Where is the stringify function?
> > 
> > Have you verified that if we can just write a decimal number to 
> > net_cls.classid, or it has to be a hex number?

Hi Jie,
 Tried it out. We can write a decimal number direclty. The stringify is the 
stout::stringify function. It was being used here as well:
 
https://github.com/apache/mesos/blob/765c025dd43e04360b29c19bd9a66837954c5a20/src/linux/cgroups.cpp#L960
 
 
 I couldn't find an overloaded cgorups::write that took the value as an int, 
and hence had to stringify before using cgroups::write


- Avinash


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


On Feb. 3, 2016, 7:17 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43096/
> ---
> 
> (Updated Feb. 3, 2016, 7:17 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helper function in cgroup for supporting net_cls subsystem.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp 83b3e401ac69194bef496087dee32e0532b834ab 
>   src/linux/cgroups.cpp bbc1fb3c60e53dffb1e8029942725e7102eb9aee 
> 
> Diff: https://reviews.apache.org/r/43096/diff/
> 
> 
> Testing
> ---
> 
> make and make check.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 43096: Added helper function in cgroup for supporting net_cls subsystem.

[Anand Mazumdar]

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



Primarily trivial comments around style/error messages.


src/linux/cgroups.hpp (line 656)


Should fit in one line?



src/linux/cgroups.cpp (line 2463)


Ditto.



src/linux/cgroups.cpp (line 2471)


Space after `net_cls.classid` and a colon too



src/linux/cgroups.cpp (line 2476)


Wondering if we should return more info to the caller here?



src/linux/cgroups.cpp (line 2492)


Space after colon



src/linux/cgroups.cpp (line 2495)


Period at the end.


- Anand Mazumdar


On Feb. 3, 2016, 7:17 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43096/
> ---
> 
> (Updated Feb. 3, 2016, 7:17 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helper function in cgroup for supporting net_cls subsystem.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp 83b3e401ac69194bef496087dee32e0532b834ab 
>   src/linux/cgroups.cpp bbc1fb3c60e53dffb1e8029942725e7102eb9aee 
> 
> Diff: https://reviews.apache.org/r/43096/diff/
> 
> 
> Testing
> ---
> 
> make and make check.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>