> On April 15, 2014, 12:17 a.m., Ian Downes wrote:
> > src/linux/routing/link/link.cpp, line 114
> > <https://reviews.apache.org/r/20292/diff/1/?file=555694#file555694line114>
> >
> >     What does it mean to create a virtual pair of interfaces if pid is None 
> > - both are in the same namespace? I thought one of the pair was in the 
> > parent's namespace and the other had to be in a different namespace.

No, it's not required. Having the peer in the same namespace can simply the 
tests a lot (no need to create a child process).


> On April 15, 2014, 12:17 a.m., Ian Downes wrote:
> > src/linux/routing/link/internal.hpp, line 69
> > <https://reviews.apache.org/r/20292/diff/1/?file=555692#file555692line69>
> >
> >     Can this not be named "link" with the function argument as "_link" as 
> > you have done in test()?

I use the following convention: I use a single letter variable to name any raw 
libnl pointer.


> On April 15, 2014, 12:17 a.m., Ian Downes wrote:
> > src/linux/routing/link/internal.hpp, line 63
> > <https://reviews.apache.org/r/20292/diff/1/?file=555692#file555692line63>
> >
> >     It's not intuitive to read "if (err != 0) return Error()" what about 
> > calling it status? "if (status != 0) return Error()" reads better to me.

err == 0 means no error
err != 0 means error

Looks fine to me. Feel lazy to change:) Let me know if you insist!


> On April 15, 2014, 12:17 a.m., Ian Downes wrote:
> > configure.ac, line 711
> > <https://reviews.apache.org/r/20292/diff/1/?file=555689#file555689line711>
> >
> >     Are these CFLAGS or CPPFLAGS? You append them to MESOS_CPPFLAGS in the 
> > Makefile.

Libnl is written in C, so I prefer to using CFLAGS.

IIUC, CPPFLAGS is for preprocessing and CXXFLAGS is for c++ compiler.


> On April 15, 2014, 12:17 a.m., Ian Downes wrote:
> > src/tests/routing_tests.cpp, line 172
> > <https://reviews.apache.org/r/20292/diff/1/?file=555695#file555695line172>
> >
> >     Is this as simple as checking the output of an os::shell() with "ip 
> > setns exec ..."?

Manually verified. Will add more sophisticated tests in the following patches.


- Jie


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


On April 15, 2014, 4:41 a.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20292/
> -----------------------------------------------------------
> 
> (Updated April 15, 2014, 4:41 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod 
> Kone, and Cong Wang.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This is Part 1 of the linux routing library.
> 
> 
> Diffs
> -----
> 
>   configure.ac c1de6d7eec1fc740c031bebb21a662e654328943 
>   src/Makefile.am 560b4c77a4e44ef1833b3b4afd95bb8040c9f229 
>   src/linux/routing/internal.hpp PRE-CREATION 
>   src/linux/routing/link/internal.hpp PRE-CREATION 
>   src/linux/routing/link/link.hpp PRE-CREATION 
>   src/linux/routing/link/link.cpp PRE-CREATION 
>   src/tests/routing_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/20292/diff/
> 
> 
> Testing
> -------
> 
> make check
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Reply via email to