> On Dec. 20, 2018, 9:39 a.m., Qian Zhang wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 569 (patched)
> > <https://reviews.apache.org/r/68016/diff/10/?file=2114531#file2114531line569>
> >
> >     Just curious why we need add this definition? It seems we do not do 
> > this for other isolators, e.g., `ENABLE_NETWORK_PORTS_ISOLATOR`.

Removed this code and added an item to the `target_compile_definitions()`.


> On Dec. 20, 2018, 9:39 a.m., Qian Zhang wrote:
> > configure.ac
> > Lines 356 (patched)
> > <https://reviews.apache.org/r/68016/diff/10/?file=2114532#file2114532line356>
> >
> >     Can you describe why this flag will be deprecated in the future here? 
> > E.g., we will deprecate it when we do not support the old kernel in future.

Updated the comment.


> On Dec. 20, 2018, 9:39 a.m., Qian Zhang wrote:
> > configure.ac
> > Lines 1599 (patched)
> > <https://reviews.apache.org/r/68016/diff/10/?file=2114532#file2114532line1599>
> >
> >     So here we check the header regardless using bundled libseccomp or 
> > pre-installed libseccomp? I see you have another header check in line 1617 
> > which I think should be the right place to check header (i.e., after 
> > `CPPFLAGS`&`LDFLAGS` are updated as per `with_libseccomp`).

`linux/seccomp.h` - system header which is always available on Linux 3.5+. This 
header is required in all cases.
`seccomp.h` - `libseccomp`'s header file, which we require *only* when we build 
*without* bundled `libseccomp`.


> On Dec. 20, 2018, 9:39 a.m., Qian Zhang wrote:
> > configure.ac
> > Lines 1607-1608 (patched)
> > <https://reviews.apache.org/r/68016/diff/10/?file=2114532#file2114532line1607>
> >
> >     What is the difference between `use a preinstalled libseccomp` and 
> > `ignore all bundled libraries`? Aren't they same?

It's a copypaste...
Updated the comment (`preinstalled` -> `pre-bundled`) and then moved it to the 
next `if` condition.


> On Dec. 20, 2018, 9:39 a.m., Qian Zhang wrote:
> > configure.ac
> > Lines 1615 (patched)
> > <https://reviews.apache.org/r/68016/diff/10/?file=2114532#file2114532line1615>
> >
> >     Do we also need to check `test "x$enable_bundled" != "xyes"`?

Fixed.


> On Dec. 20, 2018, 9:39 a.m., Qian Zhang wrote:
> > configure.ac
> > Lines 1617 (patched)
> > <https://reviews.apache.org/r/68016/diff/10/?file=2114532#file2114532line1617>
> >
> >     I see in line 1599 you check the header `linux/seccomp.h`, so what's 
> > difference between `linux/seccomp.h` and `seccomp.h`?

See my previous comment.


- Andrei


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


On Nov. 8, 2018, 3:23 p.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68016/
> -----------------------------------------------------------
> 
> (Updated Nov. 8, 2018, 3:23 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Gilbert Song, Jie Yu, James 
> Peach, and Qian Zhang.
> 
> 
> Bugs: MESOS-9032
>     https://issues.apache.org/jira/browse/MESOS-9032
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This library is needed to implement Seccomp syscall filtering in the
> Mesos containerizer. This patch introduces `seccomp-isolator` build
> flag, which is used to include or exclude sources related to Seccomp
> from the build. Since Seccomp is a Linux-specific feature, the flag
> is disabled by default. Enabling `seccomp-isolator` means either:
> 
> 1. Compiling and linking against the bundled version of libseccomp from
>    sources (default).
> 
> 2. Linking against the libseccomp installed in the OS,
>    if `--with-libseccomp` build flag is provided.
> 
> 
> Diffs
> -----
> 
>   3rdparty/CMakeLists.txt 703808d063e4bba58f647b5d48b78724003bcc4e 
>   3rdparty/Makefile.am e625e7be1743348d02c6dbb8e0a92d1a395b0ef4 
>   3rdparty/cmake/Versions.cmake 69fc594ec5ba2887b20b88ec0767a5d801411411 
>   3rdparty/versions.am 99ef92087f6958d83ba415e84db5cbbb0c597573 
>   cmake/CompilationConfigure.cmake 2485a8a580dcc2ad9b026e389b6525ef3a19f98e 
>   configure.ac 6778f119570def1838e26cddf7b0192bfe6e37d4 
>   src/CMakeLists.txt bde070445b644e15d46c390d1c983caabaa1fec8 
>   src/Makefile.am 7a4904a3d67479267087fd2313a263d8218843fa 
>   src/python/native_common/ext_modules.py.in 
> 1f2e6c131d18e3e2fbc2e865c4698c83e73b87ba 
> 
> 
> Diff: https://reviews.apache.org/r/68016/diff/11/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Reply via email to