Re: Review Request 54821: Refactored Docker::run() to make it only aware of docker cli options.

2016-12-21 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54821/#review159923 --- Basically it looks good to me. Have some simple issues we would l

Re: Review Request 54880: Added unit-test for dynamic addition/deletion of CNI config.

2016-12-21 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54880/#review159874 --- Fix it, then Ship it! This test takes a long time to finish. Ma

Re: Review Request 54946: Made sure classes deriving from FlagBase use virtual inheritance.

2016-12-21 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54946/#review159895 --- Ship it! Ship It! - Michael Park On Dec. 21, 2016, 1:07 p.m.

Re: Review Request 54953: Set -pie conditionally on whether hardening is enabled or not.

2016-12-21 Thread Aaron Wood
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54953/ --- (Updated Dec. 22, 2016, 12:30 a.m.) Review request for mesos and Michael Park.

Re: Review Request 54953: Set -pie conditionally on whether hardening is enabled or not.

2016-12-21 Thread Aaron Wood
> On Dec. 21, 2016, 11:57 p.m., Michael Park wrote: > > src/Makefile.am, line 112 > > > > > > Actually, I think we want to `+=` here instead? You're right, good catch. - Aaron ---

Re: Review Request 54693: Add ProtoBuf schema for Blkio cgroup subsystem

2016-12-21 Thread Jason Lai
> On Dec. 21, 2016, 11:40 p.m., Zhitao Li wrote: > > include/mesos/mesos.proto, lines 1107-1108 > > > > > > Why is this newly added field in the middle of the message rather than > > the end? Make sense now sinc

Re: Review Request 54953: Set -pie conditionally on whether hardening is enabled or not.

2016-12-21 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54953/#review159886 --- src/Makefile.am (line 111)

Re: Review Request 54953: Set -pie conditionally on whether hardening is enabled or not.

2016-12-21 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54953/#review159883 --- src/Makefile.am (line 88)

Re: Review Request 54951: Remove the use of FORTIFY_SOURCE from stout.

2016-12-21 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54951/#review159880 --- Ship it! Ship It! - Michael Park On Dec. 21, 2016, 2:06 p.m.

Re: Review Request 54949: Remove the use of FORTIFY_SOURCE from Mesos.

2016-12-21 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54949/#review159882 --- Ship it! Ship It! - Michael Park On Dec. 21, 2016, 2:02 p.m.

Re: Review Request 54950: Remove the use of FORTIFY_SOURCE from libprocess.

2016-12-21 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54950/#review159881 --- Ship it! Ship It! - Michael Park On Dec. 21, 2016, 2:04 p.m.

Re: Review Request 54953: Set -pie conditionally on whether hardening is enabled or not.

2016-12-21 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54953/#review159877 --- Ship it! Ship It! - Michael Park On Dec. 21, 2016, 2:50 p.m.

Re: Review Request 54592: Introduced an `os::lseek` abstraction in stout.

2016-12-21 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54592/ --- (Updated Dec. 21, 2016, 3:42 p.m.) Review request for mesos, Daniel Pravat and

Re: Review Request 54595: Introduced an `os::dup` abstraction in stout.

2016-12-21 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54595/ --- (Updated Dec. 21, 2016, 3:42 p.m.) Review request for mesos, Daniel Pravat and

Re: Review Request 54591: Introduced `WindowsFD` class which is analogous to an `int` in POSIX.

2016-12-21 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54591/ --- (Updated Dec. 21, 2016, 3:41 p.m.) Review request for mesos, Daniel Pravat and

Re: Review Request 54590: Removed unused `peek` function.

2016-12-21 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54590/ --- (Updated Dec. 21, 2016, 3:40 p.m.) Review request for mesos, Daniel Pravat and

Re: Review Request 54693: Add ProtoBuf schema for Blkio cgroup subsystem

2016-12-21 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54693/#review159876 --- include/mesos/mesos.proto (lines 1107 - 1108)

Re: Review Request 54602: Replaced `int` with `int_fd` in libprocess.

2016-12-21 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54602/ --- (Updated Dec. 21, 2016, 3:39 p.m.) Review request for mesos, Daniel Pravat and

Re: Review Request 54603: Replaced `int` with `int_fd` in mesos.

2016-12-21 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54603/ --- (Updated Dec. 21, 2016, 3:40 p.m.) Review request for mesos, Daniel Pravat and

Re: Review Request 54601: Replaced `int` with `int_fd` in stout.

2016-12-21 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54601/ --- (Updated Dec. 21, 2016, 3:38 p.m.) Review request for mesos, Daniel Pravat and

Re: Review Request 54762: Introduced an `os::pipe` abstraction to stout.

2016-12-21 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54762/ --- (Updated Dec. 21, 2016, 3:37 p.m.) Review request for mesos, Daniel Pravat and

Review Request 54954: Improve the warning when failing to find the executor PID.

2016-12-21 Thread James Peach
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54954/ --- Review request for mesos and Anand Mazumdar. Repository: mesos Description --

Re: Review Request 54953: Set -pie conditionally on whether hardening is enabled or not.

2016-12-21 Thread James Peach
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54953/#review159872 --- Ship it! Ship It! - James Peach On Dec. 21, 2016, 10:50 p.m.

Review Request 54953: Set -pie conditionally on whether hardening is enabled or not.

2016-12-21 Thread Aaron Wood
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54953/ --- Review request for mesos and Michael Park. Bugs: MESOS-6830 https://issues.

Review Request 54952: Added EINVAL to the list of expected `getpwnam_r()` errors.

2016-12-21 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54952/ --- Review request for mesos and Alexander Rukletsov. Bugs: MESOS-6826 https://

Re: Review Request 54693: Add ProtoBuf schema for Blkio cgroup subsystem

2016-12-21 Thread Jason Lai
> On Dec. 17, 2016, 1:25 a.m., Jie Yu wrote: > > include/mesos/mesos.proto, line 2188 > > > > > > I'd introduce CgroupInfo::Blkio::CFQ CgroupInfo::Blkio::Throttling > > message here. Looks like for the fields below

Review Request 54951: Remove the use of FORTIFY_SOURCE from stout.

2016-12-21 Thread Aaron Wood
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54951/ --- Review request for mesos and Michael Park. Bugs: MESOS-6829 https://issues.

Review Request 54950: Remove the use of FORTIFY_SOURCE from libprocess.

2016-12-21 Thread Aaron Wood
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54950/ --- Review request for mesos and Michael Park. Bugs: MESOS-6829 https://issues.

Review Request 54949: Remove the use of FORTIFY_SOURCE from Mesos.

2016-12-21 Thread Aaron Wood
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54949/ --- Review request for mesos and Michael Park. Bugs: MESOS-6829 https://issues.

Review Request 54946: Made sure classes deriving from FlagBase use virtual inheritance.

2016-12-21 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54946/ --- Review request for mesos and Michael Park. Repository: mesos Description

Re: Review Request 54693: Add ProtoBuf schema for Blkio cgroup subsystem

2016-12-21 Thread Jason Lai
> On Dec. 17, 2016, 1:25 a.m., Jie Yu wrote: > > include/mesos/mesos.proto, lines 2226-2227 > > > > > > What are these two? They are `blkio.throttle.io_serviced` and `blkio.throttle.io_service_bytes`. I'll add com

Re: Review Request 54693: Add ProtoBuf schema for Blkio cgroup subsystem

2016-12-21 Thread Jason Lai
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54693/ --- (Updated Dec. 21, 2016, 8:06 p.m.) Review request for mesos, Xiaojian Huang, Gi

Re: Review Request 54589: Improved operator HTTP API docs.

2016-12-21 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54589/#review159855 --- Ship it! Ship It! - Vinod Kone On Dec. 13, 2016, 3:02 a.m.,

Re: Review Request 54841: Used `loop` in implementation of io::read and io::write.

2016-12-21 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54841/#review159849 --- Ship it! - Jie Yu On Dec. 18, 2016, 1:04 a.m., Benjamin Hindm

Re: Review Request 54926: Augmented a fault_tolerance_test to cover update of role.

2016-12-21 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54926/#review159851 --- Patch looks great! Reviews applied: [54926] Passed command: expo

Re: Review Request 54821: Refactored Docker::run() to make it only aware of docker cli options.

2016-12-21 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54821/ --- (Updated Dec. 21, 2016, 6:52 p.m.) Review request for mesos, Xiaojian Huang, ha

Re: Review Request 54693: Add ProtoBuf schema for Blkio cgroup subsystem

2016-12-21 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54693/#review159841 --- Patch looks great! Reviews applied: [53546, 54693] Passed comman

Re: Review Request 54896: Fixed copy-template-and-create-symlink make target.

2016-12-21 Thread James Peach
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54896/#review159839 --- Ship it! Ship It! - James Peach On Dec. 21, 2016, 11:20 a.m.

Re: Review Request 53237: Improved slave recovery tests.

2016-12-21 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53237/ --- (Updated Dec. 21, 2016, 5:42 p.m.) Review request for mesos and Vinod Kone. C

Re: Review Request 53237: Improved slave recovery tests.

2016-12-21 Thread Neil Conway
> On Dec. 21, 2016, 4:57 p.m., Vinod Kone wrote: > > src/tests/slave_recovery_tests.cpp, line 800 > > > > > > not yours but can you s/EXPECT/ASSERT/ so that the next line doesn't > > crash. Fixed this and a bunch

Re: Review Request 53237: Improved slave recovery tests.

2016-12-21 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53237/#review159828 --- Fix it, then Ship it! src/tests/slave_recovery_tests.cpp (line

Re: Review Request 54889: Handled all possible offers in test.

2016-12-21 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54889/#review159831 --- Ship it! Ship It! - Neil Conway On Dec. 20, 2016, 10:15 p.m.

Re: Review Request 54889: Handled all possible offers in test.

2016-12-21 Thread Neil Conway
> On Dec. 20, 2016, 4:41 p.m., Neil Conway wrote: > > Can we add an expectation for exactly when we expect to receive the offer? > > i.e., another `WillOnce(FutureSatisfy(...))` and then wait for the future > > at the appropriate time. > > > > The thing I don't like about the `WillRepeatedly(.

Re: Review Request 54880: Added unit-test for dynamic addition/deletion of CNI config.

2016-12-21 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54880/#review159826 --- Patch looks great! Reviews applied: [54716, 54717, 54718, 54880]

Re: Review Request 54889: Handled all possible offers in test.

2016-12-21 Thread Benjamin Bannier
> On Dec. 20, 2016, 5:41 p.m., Neil Conway wrote: > > Can we add an expectation for exactly when we expect to receive the offer? > > i.e., another `WillOnce(FutureSatisfy(...))` and then wait for the future > > at the appropriate time. > > > > The thing I don't like about the `WillRepeatedly(.

Re: Review Request 54889: Handled all possible offers in test.

2016-12-21 Thread Neil Conway
> On Dec. 20, 2016, 4:41 p.m., Neil Conway wrote: > > Can we add an expectation for exactly when we expect to receive the offer? > > i.e., another `WillOnce(FutureSatisfy(...))` and then wait for the future > > at the appropriate time. > > > > The thing I don't like about the `WillRepeatedly(.

Re: Review Request 54603: Replaced `int` with `int_fd` in mesos.

2016-12-21 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54603/#review159812 --- Bad patch! Reviews applied: [54603, 54602, 54601, 54762, 54592, 5

Re: Review Request 54889: Handled all possible offers in test.

2016-12-21 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54889/#review159810 --- Patch looks great! Reviews applied: [54889] Passed command: expo

Re: Review Request 54877: Windows: Stout: Removed dependency on Shell API.

2016-12-21 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54877/#review159803 --- Patch looks great! Reviews applied: [54877] Passed command: expo

Re: Review Request 54896: Fixed copy-template-and-create-symlink make target.

2016-12-21 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54896/ --- (Updated Dec. 21, 2016, 12:20 p.m.) Review request for mesos, James Peach and T

Re: Review Request 54896: Fixed copy-template-and-create-symlink make target.

2016-12-21 Thread Benjamin Bannier
> On Dec. 20, 2016, 9:08 p.m., James Peach wrote: > > src/Makefile.am, line 2428 > > > > > > Should this be a symlink too? Making a copy creates the possibility > > that you could add environment variables in one f

Re: Review Request 53299: Fixed memory leak in implementation of Future::after().

2016-12-21 Thread Alexander Rojas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53299/ --- (Updated Dec. 21, 2016, 12:10 p.m.) Review request for mesos, Benjamin Bannier,

Re: Review Request 54898: Added a CHECK in updateFrameworkInfo.

2016-12-21 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54898/#review159796 --- Patch looks great! Reviews applied: [54898] Passed command: expo

Re: Review Request 53299: Fixed memory leak in implementation of Future::after().

2016-12-21 Thread Alexander Rojas
> On Dec. 14, 2016, 8:45 p.m., Joris Van Remoortere wrote: > > I ran this with `./libprocess-tests --gtest_filter="*FutureTest*After3*" > > --gtest_repeat=1000 --gtest_break_on_failure` and ran into the following: > > > > ``` > > Repeating all tests (iteration 412) . . . > > > > Note: Google T