You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Benjamin Hindman <be...@berkeley.edu> on 2012/05/16 00:34:27 UTC
Review Request: Added an interface for getting information from the /proc
filesystem.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5132/
-----------------------------------------------------------
Review request for mesos, Alex Degtiar, John Sirois, Vinod Kone, and Jie Yu.
Summary
-------
This patch is a refactor of some parts of the patch at https://reviews.apache.org/r/4167. The plan is to take the components from that patch, break them into smaller patches, and get them committed.
Diffs
-----
src/Makefile.am 333234d
src/linux/proc.hpp PRE-CREATION
src/linux/proc.cpp PRE-CREATION
src/tests/proc_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/5132/diff
Testing
-------
On a Linux machine:
make check
Thanks,
Benjamin
Re: Review Request: Added an interface for getting information from the
/proc filesystem.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On 2012-05-15 23:40:05, Vinod Kone wrote:
> > src/linux/proc.hpp, line 61
> > <https://reviews.apache.org/r/5132/diff/1/?file=109078#file109078line61>
> >
> > doc
Done.
> On 2012-05-15 23:40:05, Vinod Kone wrote:
> > src/linux/proc.cpp, line 129
> > <https://reviews.apache.org/r/5132/diff/1/?file=109079#file109079line129>
> >
> > is this format guaranteed?
Yup.
> On 2012-05-15 23:40:05, Vinod Kone wrote:
> > src/tests/proc_tests.cpp, lines 44-45
> > <https://reviews.apache.org/r/5132/diff/1/?file=109080#file109080line44>
> >
> > s/EXPECT_NE(0/EXPECT_EQ(1/
Done.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5132/#review7920
-----------------------------------------------------------
On 2012-05-15 22:34:27, Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5132/
> -----------------------------------------------------------
>
> (Updated 2012-05-15 22:34:27)
>
>
> Review request for mesos, Alex Degtiar, John Sirois, Vinod Kone, and Jie Yu.
>
>
> Summary
> -------
>
> This patch is a refactor of some parts of the patch at https://reviews.apache.org/r/4167. The plan is to take the components from that patch, break them into smaller patches, and get them committed.
>
>
> Diffs
> -----
>
> src/Makefile.am 333234d
> src/linux/proc.hpp PRE-CREATION
> src/linux/proc.cpp PRE-CREATION
> src/tests/proc_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/5132/diff
>
>
> Testing
> -------
>
> On a Linux machine:
>
> make check
>
>
> Thanks,
>
> Benjamin
>
>
Re: Review Request: Added an interface for getting information from the
/proc filesystem.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5132/#review7920
-----------------------------------------------------------
Ship it!
src/linux/proc.hpp
<https://reviews.apache.org/r/5132/#comment17233>
doc
src/linux/proc.cpp
<https://reviews.apache.org/r/5132/#comment17236>
consider using utils::os::read()
src/linux/proc.cpp
<https://reviews.apache.org/r/5132/#comment17237>
:)
src/linux/proc.cpp
<https://reviews.apache.org/r/5132/#comment17238>
is this format guaranteed?
src/tests/proc_tests.cpp
<https://reviews.apache.org/r/5132/#comment17240>
s/EXPECT_NE(0/EXPECT_EQ(1/
- Vinod
On 2012-05-15 22:34:27, Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5132/
> -----------------------------------------------------------
>
> (Updated 2012-05-15 22:34:27)
>
>
> Review request for mesos, Alex Degtiar, John Sirois, Vinod Kone, and Jie Yu.
>
>
> Summary
> -------
>
> This patch is a refactor of some parts of the patch at https://reviews.apache.org/r/4167. The plan is to take the components from that patch, break them into smaller patches, and get them committed.
>
>
> Diffs
> -----
>
> src/Makefile.am 333234d
> src/linux/proc.hpp PRE-CREATION
> src/linux/proc.cpp PRE-CREATION
> src/tests/proc_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/5132/diff
>
>
> Testing
> -------
>
> On a Linux machine:
>
> make check
>
>
> Thanks,
>
> Benjamin
>
>
Re: Review Request: Added an interface for getting information from the
/proc filesystem.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On 2012-05-17 14:15:32, John Sirois wrote:
> > src/Makefile.am, line 195
> > <https://reviews.apache.org/r/5132/diff/1/?file=109077#file109077line195>
> >
> > I assume there is autotools magic keeping linux/proc.hpp out of darwin compiles for example? That header does
> > #include <linux/version.h>
> >
> > And later tests:
> > LINUX_VERSION_CODE
> >
> > Which I would guess fails on non-linucies
Yes, the .cpp files are only included when OS_LINUX is true.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5132/#review7959
-----------------------------------------------------------
On 2012-05-15 22:34:27, Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5132/
> -----------------------------------------------------------
>
> (Updated 2012-05-15 22:34:27)
>
>
> Review request for mesos, Alex Degtiar, John Sirois, Vinod Kone, and Jie Yu.
>
>
> Summary
> -------
>
> This patch is a refactor of some parts of the patch at https://reviews.apache.org/r/4167. The plan is to take the components from that patch, break them into smaller patches, and get them committed.
>
>
> Diffs
> -----
>
> src/Makefile.am 333234d
> src/linux/proc.hpp PRE-CREATION
> src/linux/proc.cpp PRE-CREATION
> src/tests/proc_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/5132/diff
>
>
> Testing
> -------
>
> On a Linux machine:
>
> make check
>
>
> Thanks,
>
> Benjamin
>
>
Re: Review Request: Added an interface for getting information from the
/proc filesystem.
Posted by John Sirois <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5132/#review7959
-----------------------------------------------------------
src/Makefile.am
<https://reviews.apache.org/r/5132/#comment17299>
I assume there is autotools magic keeping linux/proc.hpp out of darwin compiles for example? That header does
#include <linux/version.h>
And later tests:
LINUX_VERSION_CODE
Which I would guess fails on non-linucies
- John
On 2012-05-15 22:34:27, Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5132/
> -----------------------------------------------------------
>
> (Updated 2012-05-15 22:34:27)
>
>
> Review request for mesos, Alex Degtiar, John Sirois, Vinod Kone, and Jie Yu.
>
>
> Summary
> -------
>
> This patch is a refactor of some parts of the patch at https://reviews.apache.org/r/4167. The plan is to take the components from that patch, break them into smaller patches, and get them committed.
>
>
> Diffs
> -----
>
> src/Makefile.am 333234d
> src/linux/proc.hpp PRE-CREATION
> src/linux/proc.cpp PRE-CREATION
> src/tests/proc_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/5132/diff
>
>
> Testing
> -------
>
> On a Linux machine:
>
> make check
>
>
> Thanks,
>
> Benjamin
>
>
Re: Review Request: Added an interface for getting information from the
/proc filesystem.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On 2012-05-16 00:41:59, Alex Degtiar wrote:
> >
Thanks for the reviews guys, I'm submitting this now.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5132/#review7922
-----------------------------------------------------------
On 2012-05-15 22:34:27, Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5132/
> -----------------------------------------------------------
>
> (Updated 2012-05-15 22:34:27)
>
>
> Review request for mesos, Alex Degtiar, John Sirois, Vinod Kone, and Jie Yu.
>
>
> Summary
> -------
>
> This patch is a refactor of some parts of the patch at https://reviews.apache.org/r/4167. The plan is to take the components from that patch, break them into smaller patches, and get them committed.
>
>
> Diffs
> -----
>
> src/Makefile.am 333234d
> src/linux/proc.hpp PRE-CREATION
> src/linux/proc.cpp PRE-CREATION
> src/tests/proc_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/5132/diff
>
>
> Testing
> -------
>
> On a Linux machine:
>
> make check
>
>
> Thanks,
>
> Benjamin
>
>
Re: Review Request: Added an interface for getting information from the
/proc filesystem.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On 2012-05-16 00:41:59, Alex Degtiar wrote:
> > src/linux/proc.hpp, line 73
> > <https://reviews.apache.org/r/5132/diff/1/?file=109078#file109078line73>
> >
> > Some of these fields, including ppid, session, and perhaps pgrp and others, have the pid_t type according to unistd.h. Do we want to be consistent with that, or is there a reason to keep them as ints?
You're right! Awesome catch! I'm switching them to pid_t. Thanks!
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5132/#review7922
-----------------------------------------------------------
On 2012-05-15 22:34:27, Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5132/
> -----------------------------------------------------------
>
> (Updated 2012-05-15 22:34:27)
>
>
> Review request for mesos, Alex Degtiar, John Sirois, Vinod Kone, and Jie Yu.
>
>
> Summary
> -------
>
> This patch is a refactor of some parts of the patch at https://reviews.apache.org/r/4167. The plan is to take the components from that patch, break them into smaller patches, and get them committed.
>
>
> Diffs
> -----
>
> src/Makefile.am 333234d
> src/linux/proc.hpp PRE-CREATION
> src/linux/proc.cpp PRE-CREATION
> src/tests/proc_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/5132/diff
>
>
> Testing
> -------
>
> On a Linux machine:
>
> make check
>
>
> Thanks,
>
> Benjamin
>
>
Re: Review Request: Added an interface for getting information from the
/proc filesystem.
Posted by Alex Degtiar <al...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5132/#review7922
-----------------------------------------------------------
Ship it!
src/linux/proc.hpp
<https://reviews.apache.org/r/5132/#comment17242>
Some of these fields, including ppid, session, and perhaps pgrp and others, have the pid_t type according to unistd.h. Do we want to be consistent with that, or is there a reason to keep them as ints?
- Alex
On 2012-05-15 22:34:27, Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5132/
> -----------------------------------------------------------
>
> (Updated 2012-05-15 22:34:27)
>
>
> Review request for mesos, Alex Degtiar, John Sirois, Vinod Kone, and Jie Yu.
>
>
> Summary
> -------
>
> This patch is a refactor of some parts of the patch at https://reviews.apache.org/r/4167. The plan is to take the components from that patch, break them into smaller patches, and get them committed.
>
>
> Diffs
> -----
>
> src/Makefile.am 333234d
> src/linux/proc.hpp PRE-CREATION
> src/linux/proc.cpp PRE-CREATION
> src/tests/proc_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/5132/diff
>
>
> Testing
> -------
>
> On a Linux machine:
>
> make check
>
>
> Thanks,
>
> Benjamin
>
>