Re: Review Request 38855: Added support for getting shared and slave mount peer group ID.

2015-09-29 Thread Jie Yu 


> On Sept. 29, 2015, 8:46 p.m., Ian Downes wrote:
> > src/tests/containerizer/fs_tests.cpp, line 203
> > 
> >
> > So the mount won't get cleaned up if the test asserts early?

We have a universal cleanup code here:
https://github.com/apache/mesos/blob/master/src/tests/environment.cpp#L458

So the mount will be cleaned up properly even if test asserts early.


- Jie


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


On Sept. 29, 2015, 7:09 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38855/
> ---
> 
> (Updated Sept. 29, 2015, 7:09 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3539
> https://issues.apache.org/jira/browse/MESOS-3539
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for getting shared and slave mount peer group ID.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp f3aa0c2a385527eab5895f5d10c29e3d68e30a49 
>   src/linux/fs.cpp 8631d892ed6c132d6a9dc2031c2ca040623e9acc 
>   src/tests/containerizer/fs_tests.cpp 
> 34d3c41045373326fb6096031cd1daef0e7b203a 
> 
> Diff: https://reviews.apache.org/r/38855/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38855: Added support for getting shared and slave mount peer group ID.

2015-09-29 Thread Ian Downes 

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

Ship it!



src/tests/containerizer/fs_tests.cpp (line 198)


Can you get the mount id before you do the mount and then verify the shared 
sibling id is correct? This might be easiest to do with a second self bind 
mount so you can find the sibling.



src/tests/containerizer/fs_tests.cpp (line 200)


s/Cleanup/Clean up/



src/tests/containerizer/fs_tests.cpp (line 201)


So the mount won't get cleaned up if the test asserts early?



src/tests/containerizer/fs_tests.cpp (line 241)


Like above, can you verify the master id is correct?



src/tests/containerizer/fs_tests.cpp (line 243)


s/Cleanup/Clean up/


- Ian Downes


On Sept. 29, 2015, 12:09 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38855/
> ---
> 
> (Updated Sept. 29, 2015, 12:09 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3539
> https://issues.apache.org/jira/browse/MESOS-3539
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for getting shared and slave mount peer group ID.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp f3aa0c2a385527eab5895f5d10c29e3d68e30a49 
>   src/linux/fs.cpp 8631d892ed6c132d6a9dc2031c2ca040623e9acc 
>   src/tests/containerizer/fs_tests.cpp 
> 34d3c41045373326fb6096031cd1daef0e7b203a 
> 
> Diff: https://reviews.apache.org/r/38855/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38855: Added support for getting shared and slave mount peer group ID.

2015-09-29 Thread Jie Yu 


> On Sept. 29, 2015, 9:20 p.m., Jiang Yan Xu wrote:
> > src/linux/fs.hpp, line 174
> > 
> >
> > s/resides/resides in/

'in which' (in is already there).


- Jie


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


On Sept. 29, 2015, 7:09 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38855/
> ---
> 
> (Updated Sept. 29, 2015, 7:09 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3539
> https://issues.apache.org/jira/browse/MESOS-3539
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for getting shared and slave mount peer group ID.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp f3aa0c2a385527eab5895f5d10c29e3d68e30a49 
>   src/linux/fs.cpp 8631d892ed6c132d6a9dc2031c2ca040623e9acc 
>   src/tests/containerizer/fs_tests.cpp 
> 34d3c41045373326fb6096031cd1daef0e7b203a 
> 
> Diff: https://reviews.apache.org/r/38855/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38855: Added support for getting shared and slave mount peer group ID.

2015-09-29 Thread Jie Yu 


> On Sept. 29, 2015, 9:09 p.m., Timothy Chen wrote:
> > src/linux/fs.cpp, line 156
> > 
> >
> > Do you know in what situations will the conversion fail? Also for 
> > master: as well.
> > I wonder if we should return Result instead of ignoring the error.

It shouldn't unless the kernel has a bug. Maybe a CHECK is more appropriate?


- Jie


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


On Sept. 29, 2015, 7:09 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38855/
> ---
> 
> (Updated Sept. 29, 2015, 7:09 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3539
> https://issues.apache.org/jira/browse/MESOS-3539
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for getting shared and slave mount peer group ID.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp f3aa0c2a385527eab5895f5d10c29e3d68e30a49 
>   src/linux/fs.cpp 8631d892ed6c132d6a9dc2031c2ca040623e9acc 
>   src/tests/containerizer/fs_tests.cpp 
> 34d3c41045373326fb6096031cd1daef0e7b203a 
> 
> Diff: https://reviews.apache.org/r/38855/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38855: Added support for getting shared and slave mount peer group ID.

2015-09-29 Thread Jie Yu 

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

(Updated Sept. 29, 2015, 11:12 p.m.)


Review request for mesos, Ian Downes, Timothy Chen, and Jiang Yan Xu.


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

Added support for getting shared and slave mount peer group ID.


Diffs (updated)
-

  src/linux/fs.hpp f3aa0c2a385527eab5895f5d10c29e3d68e30a49 
  src/linux/fs.cpp 8631d892ed6c132d6a9dc2031c2ca040623e9acc 
  src/tests/containerizer/fs_tests.cpp 34d3c41045373326fb6096031cd1daef0e7b203a 

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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 38855: Added support for getting shared and slave mount peer group ID.

2015-09-29 Thread Jiang Yan Xu 


> On Sept. 29, 2015, 2:09 p.m., Timothy Chen wrote:
> > src/linux/fs.cpp, line 156
> > 
> >
> > Do you know in what situations will the conversion fail? Also for 
> > master: as well.
> > I wonder if we should return Result instead of ignoring the error.
> 
> Jie Yu wrote:
> It shouldn't unless the kernel has a bug. Maybe a CHECK is more 
> appropriate?

+1 for CHECK


- Jiang Yan


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


On Sept. 29, 2015, 12:09 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38855/
> ---
> 
> (Updated Sept. 29, 2015, 12:09 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3539
> https://issues.apache.org/jira/browse/MESOS-3539
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for getting shared and slave mount peer group ID.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp f3aa0c2a385527eab5895f5d10c29e3d68e30a49 
>   src/linux/fs.cpp 8631d892ed6c132d6a9dc2031c2ca040623e9acc 
>   src/tests/containerizer/fs_tests.cpp 
> 34d3c41045373326fb6096031cd1daef0e7b203a 
> 
> Diff: https://reviews.apache.org/r/38855/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38855: Added support for getting shared and slave mount peer group ID.

2015-09-29 Thread Timothy Chen 

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



src/linux/fs.cpp (line 156)


Do you know in what situations will the conversion fail? Also for master: 
as well.
I wonder if we should return Result instead of ignoring the error.


- Timothy Chen


On Sept. 29, 2015, 7:09 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38855/
> ---
> 
> (Updated Sept. 29, 2015, 7:09 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3539
> https://issues.apache.org/jira/browse/MESOS-3539
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for getting shared and slave mount peer group ID.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp f3aa0c2a385527eab5895f5d10c29e3d68e30a49 
>   src/linux/fs.cpp 8631d892ed6c132d6a9dc2031c2ca040623e9acc 
>   src/tests/containerizer/fs_tests.cpp 
> 34d3c41045373326fb6096031cd1daef0e7b203a 
> 
> Diff: https://reviews.apache.org/r/38855/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38855: Added support for getting shared and slave mount peer group ID.

2015-09-29 Thread Jiang Yan Xu 

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

Ship it!


Modulo


src/linux/fs.hpp (line 174)


s/resides/resides in/


- Jiang Yan Xu


On Sept. 29, 2015, 12:09 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38855/
> ---
> 
> (Updated Sept. 29, 2015, 12:09 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3539
> https://issues.apache.org/jira/browse/MESOS-3539
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for getting shared and slave mount peer group ID.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp f3aa0c2a385527eab5895f5d10c29e3d68e30a49 
>   src/linux/fs.cpp 8631d892ed6c132d6a9dc2031c2ca040623e9acc 
>   src/tests/containerizer/fs_tests.cpp 
> 34d3c41045373326fb6096031cd1daef0e7b203a 
> 
> Diff: https://reviews.apache.org/r/38855/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38855: Added support for getting shared and slave mount peer group ID.

2015-09-29 Thread Timothy Chen 

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

Ship it!


Ship It!

- Timothy Chen


On Sept. 29, 2015, 11:12 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38855/
> ---
> 
> (Updated Sept. 29, 2015, 11:12 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3539
> https://issues.apache.org/jira/browse/MESOS-3539
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for getting shared and slave mount peer group ID.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp f3aa0c2a385527eab5895f5d10c29e3d68e30a49 
>   src/linux/fs.cpp 8631d892ed6c132d6a9dc2031c2ca040623e9acc 
>   src/tests/containerizer/fs_tests.cpp 
> 34d3c41045373326fb6096031cd1daef0e7b203a 
> 
> Diff: https://reviews.apache.org/r/38855/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 38855: Added support for getting shared and slave mount peer group ID.

2015-09-29 Thread Jie Yu 

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

Review request for mesos, Ian Downes, Timothy Chen, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Added support for getting shared and slave mount peer group ID.


Diffs
-

  src/linux/fs.hpp f3aa0c2a385527eab5895f5d10c29e3d68e30a49 
  src/linux/fs.cpp 8631d892ed6c132d6a9dc2031c2ca040623e9acc 
  src/tests/containerizer/fs_tests.cpp 34d3c41045373326fb6096031cd1daef0e7b203a 

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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 38855: Added support for getting shared and slave mount peer group ID.

2015-09-29 Thread Jie Yu 

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

(Updated Sept. 29, 2015, 7:09 p.m.)


Review request for mesos, Ian Downes, Timothy Chen, and Jiang Yan Xu.


Changes
---

Added const for the methods.


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


Repository: mesos


Description
---

Added support for getting shared and slave mount peer group ID.


Diffs (updated)
-

  src/linux/fs.hpp f3aa0c2a385527eab5895f5d10c29e3d68e30a49 
  src/linux/fs.cpp 8631d892ed6c132d6a9dc2031c2ca040623e9acc 
  src/tests/containerizer/fs_tests.cpp 34d3c41045373326fb6096031cd1daef0e7b203a 

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


Testing
---

sudo make check


Thanks,

Jie Yu