mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Qian Zhang <zhq527...@gmail.com>
Subject Re: Review Request 68016: Added libseccomp to the build.
Date Thu, 20 Dec 2018 09:39:16 GMT

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




3rdparty/CMakeLists.txt
Lines 637 (patched)
<https://reviews.apache.org/r/68016/#comment296600>

    Do we need to check if we are using bundled libseccomp or pre-installed?
    
    This `CMakeLists.txt` can be used to build on Windows as well, so do we need to do this
on Linux only?



cmake/CompilationConfigure.cmake
Lines 569 (patched)
<https://reviews.apache.org/r/68016/#comment296602>

    Just curious why we need add this definition? It seems we do not do this for other isolators,
e.g., `ENABLE_NETWORK_PORTS_ISOLATOR`.



configure.ac
Lines 356 (patched)
<https://reviews.apache.org/r/68016/#comment296593>

    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.



configure.ac
Lines 1599 (patched)
<https://reviews.apache.org/r/68016/#comment296599>

    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`).



configure.ac
Lines 1607-1608 (patched)
<https://reviews.apache.org/r/68016/#comment296596>

    What is the difference between `use a preinstalled libseccomp` and `ignore all bundled
libraries`? Aren't they same?



configure.ac
Lines 1615 (patched)
<https://reviews.apache.org/r/68016/#comment296597>

    Do we also need to check `test "x$enable_bundled" != "xyes"`?



configure.ac
Lines 1617 (patched)
<https://reviews.apache.org/r/68016/#comment296598>

    I see in line 1599 you check the header `linux/seccomp.h`, so what's difference between
`linux/seccomp.h` and `seccomp.h`?


- Qian Zhang


On Nov. 8, 2018, 11: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, 11: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/10/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message