You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jie Yu <yu...@gmail.com> on 2015/09/29 19:38:03 UTC
Review Request 38855: Added support for getting shared and slave
mount peer group ID.
-----------------------------------------------------------
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.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38855/#review100983
-----------------------------------------------------------
Patch looks great!
Reviews applied: [38855]
All tests passed.
- Mesos ReviewBot
On Sept. 29, 2015, 5:38 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, 5:38 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.
Posted by Jie Yu <yu...@gmail.com>.
> On Sept. 29, 2015, 8:46 p.m., Ian Downes wrote:
> > src/tests/containerizer/fs_tests.cpp, line 200
> > <https://reviews.apache.org/r/38855/diff/2/?file=1087312#file1087312line200>
> >
> > 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.
The ID is not the mount id, it's the peer group id. They are different. A new peer group id is generated every time a new peer groups is created.
- 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.
Posted by Jie Yu <yu...@gmail.com>.
> On Sept. 29, 2015, 8:46 p.m., Ian Downes wrote:
> > src/tests/containerizer/fs_tests.cpp, line 203
> > <https://reviews.apache.org/r/38855/diff/2/?file=1087312#file1087312line203>
> >
> > 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.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/38855/#comment158309>
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)
<https://reviews.apache.org/r/38855/#comment158308>
s/Cleanup/Clean up/
src/tests/containerizer/fs_tests.cpp (line 201)
<https://reviews.apache.org/r/38855/#comment158312>
So the mount won't get cleaned up if the test asserts early?
src/tests/containerizer/fs_tests.cpp (line 241)
<https://reviews.apache.org/r/38855/#comment158311>
Like above, can you verify the master id is correct?
src/tests/containerizer/fs_tests.cpp (line 243)
<https://reviews.apache.org/r/38855/#comment158310>
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.
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On Sept. 29, 2015, 2:09 p.m., Timothy Chen wrote:
> > src/linux/fs.cpp, line 156
> > <https://reviews.apache.org/r/38855/diff/2/?file=1087311#file1087311line156>
> >
> > Do you know in what situations will the conversion fail? Also for master: as well.
> > I wonder if we should return Result<int> 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.
Posted by Jie Yu <yu...@gmail.com>.
> On Sept. 29, 2015, 9:09 p.m., Timothy Chen wrote:
> > src/linux/fs.cpp, line 156
> > <https://reviews.apache.org/r/38855/diff/2/?file=1087311#file1087311line156>
> >
> > Do you know in what situations will the conversion fail? Also for master: as well.
> > I wonder if we should return Result<int> 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.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38855/#review101014
-----------------------------------------------------------
src/linux/fs.cpp (line 156)
<https://reviews.apache.org/r/38855/#comment158321>
Do you know in what situations will the conversion fail? Also for master: as well.
I wonder if we should return Result<int> 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.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
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
>
>
Re: Review Request 38855: Added support for getting shared and slave
mount peer group ID.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
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.
Posted by Jie Yu <yu...@gmail.com>.
> On Sept. 29, 2015, 9:20 p.m., Jiang Yan Xu wrote:
> > src/linux/fs.hpp, line 174
> > <https://reviews.apache.org/r/38855/diff/2/?file=1087310#file1087310line174>
> >
> > 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.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/38855/#comment158323>
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.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
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