Re: Review Request 55042: Let MesosContainerizer support ramdisk.

2016-12-28 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [55042]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Dec. 26, 2016, 12:23 p.m., Andy Pang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55042/
> ---
> 
> (Updated Dec. 26, 2016, 12:23 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add slave startup flags ramdisk_enable environment variable to make Mesos 
> work when the root is on a ramdisk, because 'pivot_root' don't support 
> on ramdisk FS.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp cc9adfe 
>   src/launcher/posix/executor.hpp d057ff6 
>   src/launcher/posix/executor.cpp a29b31c 
>   src/linux/fs.hpp da49c9e 
>   src/linux/fs.cpp 913e233 
>   src/slave/containerizer/mesos/launch.hpp 5bba139 
>   src/slave/containerizer/mesos/launch.cpp e482ab8 
>   src/slave/flags.hpp 6ac0d45 
>   src/slave/flags.cpp 1eccea9 
>   src/slave/slave.cpp f8f2ccf 
> 
> Diff: https://reviews.apache.org/r/55042/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Andy Pang
> 
>

Re: Review Request 55042: Let MesosContainerizer support ramdisk.

2016-12-26 Thread haosdent huang 

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




src/linux/fs.cpp (line 801)


Note that `MS_MOVE` would not work if the rootfs is shared.


https://github.com/GNOME/linux-user-chroot/blob/v2015.1/src/linux-user-chroot.c#L363-L370


- haosdent huang


On Dec. 26, 2016, 12:23 p.m., Andy Pang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55042/
> ---
> 
> (Updated Dec. 26, 2016, 12:23 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add slave startup flags ramdisk_enable environment variable to make Mesos 
> work when the root is on a ramdisk, because 'pivot_root' don't support 
> on ramdisk FS.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp cc9adfe 
>   src/launcher/posix/executor.hpp d057ff6 
>   src/launcher/posix/executor.cpp a29b31c 
>   src/linux/fs.hpp da49c9e 
>   src/linux/fs.cpp 913e233 
>   src/slave/containerizer/mesos/launch.hpp 5bba139 
>   src/slave/containerizer/mesos/launch.cpp e482ab8 
>   src/slave/flags.hpp 6ac0d45 
>   src/slave/flags.cpp 1eccea9 
>   src/slave/slave.cpp f8f2ccf 
> 
> Diff: https://reviews.apache.org/r/55042/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Andy Pang
> 
>

Re: Review Request 55042: Let MesosContainerizer support ramdisk.

2016-12-26 Thread haosdent huang 

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




src/launcher/executor.cpp (line 131)


Nit: `_ramdiskEnable`



src/launcher/executor.cpp (line 152)


Nit: `ramdiskEnable(_ramdiskEnable)`



src/launcher/executor.cpp (line 414)


Nit: `ramdiskEnable`



src/launcher/executor.cpp (line 809)


Nit: `ramdiskEnable`



src/launcher/executor.cpp (line 917)


Nit: `ramdiskEnable`



src/launcher/executor.cpp (line 920)


Nit: `ramdiskEnable`



src/launcher/executor.cpp (line 953)


Nit: `ramdiskEnable`



src/launcher/posix/executor.hpp (line 34)


Nit: `ramdiskEnable`



src/launcher/posix/executor.cpp (line 61)


Nit: `ramdiskEnable`



src/launcher/posix/executor.cpp (line 85)


Nit: `launchFlags.ramdisk_enable = ramdiskEnable`;



src/linux/fs.cpp (line 795)


```
// Chdir to the new root.
```



src/linux/fs.cpp (line 804)


Should be `None()`?



src/linux/fs.cpp (line 806)


Nit: extra line

And should it be `nullptr`?



src/linux/fs.cpp (line 808)


```
return Error("Failed to make move mounts: " + mount.error());
```



src/slave/containerizer/mesos/launch.cpp (line 115)


```
  add(::ramdisk_enable,
  "ramdisk_enable",
  "Top level control of ramdisk support. When enabled, move\n"
  "mount would be used to `chroot` because ramdisk don't support\n"
  "`pivot_root`.\n",
  false);
```



src/slave/containerizer/mesos/launch.cpp (lines 539 - 543)


Is it better to define methods in this form?

```
Try enter(const string& root, const bool ramdisk);
Try enterWithPivotRoot(const string& root);
Try enterWithMoveMount(const string& root);
```



src/slave/flags.cpp (line 541)


```
  add(::ramdisk_enable,
  "ramdisk_enable",
  "Top level control of ramdisk support. When enabled, move\n"
  "mount would be used to `chroot` because ramdisk don't support\n"
  "`pivot_root`.\n",
  false);


- haosdent huang


On Dec. 26, 2016, 12:23 p.m., Andy Pang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55042/
> ---
> 
> (Updated Dec. 26, 2016, 12:23 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add slave startup flags ramdisk_enable environment variable to make Mesos 
> work when the root is on a ramdisk, because 'pivot_root' don't support 
> on ramdisk FS.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp cc9adfe 
>   src/launcher/posix/executor.hpp d057ff6 
>   src/launcher/posix/executor.cpp a29b31c 
>   src/linux/fs.hpp da49c9e 
>   src/linux/fs.cpp 913e233 
>   src/slave/containerizer/mesos/launch.hpp 5bba139 
>   src/slave/containerizer/mesos/launch.cpp e482ab8 
>   src/slave/flags.hpp 6ac0d45 
>   src/slave/flags.cpp 1eccea9 
>   src/slave/slave.cpp f8f2ccf 
> 
> Diff: https://reviews.apache.org/r/55042/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Andy Pang
> 
>

Re: Review Request 55042: Let MesosContainerizer support ramdisk.

2016-12-26 Thread Andy Pang 

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

(Updated 十二月 26, 2016, 12:23 p.m.)


Review request for mesos, Jie Yu and Vinod Kone.


Repository: mesos


Description
---

Add slave startup flags ramdisk_enable environment variable to make Mesos 
work when the root is on a ramdisk, because 'pivot_root' don't support 
on ramdisk FS.


Diffs (updated)
-

  src/launcher/executor.cpp cc9adfe 
  src/launcher/posix/executor.hpp d057ff6 
  src/launcher/posix/executor.cpp a29b31c 
  src/linux/fs.hpp da49c9e 
  src/linux/fs.cpp 913e233 
  src/slave/containerizer/mesos/launch.hpp 5bba139 
  src/slave/containerizer/mesos/launch.cpp e482ab8 
  src/slave/flags.hpp 6ac0d45 
  src/slave/flags.cpp 1eccea9 
  src/slave/slave.cpp f8f2ccf 

Diff: https://reviews.apache.org/r/55042/diff/


Testing
---

make check


Thanks,

Andy Pang

Review Request 55042: Let MesosContainerizer support ramdisk.

2016-12-26 Thread Andy Pang 

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

Review request for mesos, Jie Yu and Vinod Kone.


Repository: mesos


Description
---

Add slave startup flags ramdisk_enable environment variable to make Mesos 
work when the root is on a ramdisk, because 'pivot_root' don't support 
on ramdisk FS.


Diffs
-

  src/launcher/executor.cpp cc9adfe 
  src/launcher/posix/executor.hpp d057ff6 
  src/launcher/posix/executor.cpp a29b31c 
  src/linux/fs.hpp da49c9e 
  src/linux/fs.cpp 913e233 
  src/slave/containerizer/mesos/launch.hpp 5bba139 
  src/slave/containerizer/mesos/launch.cpp e482ab8 
  src/slave/flags.hpp 6ac0d45 
  src/slave/flags.cpp 1eccea9 
  src/slave/slave.cpp f8f2ccf 

Diff: https://reviews.apache.org/r/55042/diff/


Testing
---

make check


Thanks,

Andy Pang