You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Andy Pang <pa...@huawei.com> on 2016/12/26 12:01:26 UTC
Review Request 55042: Let MesosContainerizer support ramdisk.
-----------------------------------------------------------
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
Re: Review Request 55042: Let MesosContainerizer support ramdisk.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
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.
Posted by haosdent huang <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55042/#review160134
-----------------------------------------------------------
src/linux/fs.cpp (line 801)
<https://reviews.apache.org/r/55042/#comment231166>
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.
Posted by haosdent huang <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55042/#review160133
-----------------------------------------------------------
src/launcher/executor.cpp (line 131)
<https://reviews.apache.org/r/55042/#comment231153>
Nit: `_ramdiskEnable`
src/launcher/executor.cpp (line 152)
<https://reviews.apache.org/r/55042/#comment231154>
Nit: `ramdiskEnable(_ramdiskEnable)`
src/launcher/executor.cpp (line 414)
<https://reviews.apache.org/r/55042/#comment231152>
Nit: `ramdiskEnable`
src/launcher/executor.cpp (line 809)
<https://reviews.apache.org/r/55042/#comment231151>
Nit: `ramdiskEnable`
src/launcher/executor.cpp (line 917)
<https://reviews.apache.org/r/55042/#comment231155>
Nit: `ramdiskEnable`
src/launcher/executor.cpp (line 920)
<https://reviews.apache.org/r/55042/#comment231156>
Nit: `ramdiskEnable`
src/launcher/executor.cpp (line 953)
<https://reviews.apache.org/r/55042/#comment231157>
Nit: `ramdiskEnable`
src/launcher/posix/executor.hpp (line 34)
<https://reviews.apache.org/r/55042/#comment231149>
Nit: `ramdiskEnable`
src/launcher/posix/executor.cpp (line 61)
<https://reviews.apache.org/r/55042/#comment231150>
Nit: `ramdiskEnable`
src/launcher/posix/executor.cpp (line 85)
<https://reviews.apache.org/r/55042/#comment231158>
Nit: `launchFlags.ramdisk_enable = ramdiskEnable`;
src/linux/fs.cpp (line 795)
<https://reviews.apache.org/r/55042/#comment231159>
```
// Chdir to the new root.
```
src/linux/fs.cpp (line 804)
<https://reviews.apache.org/r/55042/#comment231162>
Should be `None()`?
src/linux/fs.cpp (line 806)
<https://reviews.apache.org/r/55042/#comment231160>
Nit: extra line
And should it be `nullptr`?
src/linux/fs.cpp (line 808)
<https://reviews.apache.org/r/55042/#comment231161>
```
return Error("Failed to make move mounts: " + mount.error());
```
src/slave/containerizer/mesos/launch.cpp (line 115)
<https://reviews.apache.org/r/55042/#comment231164>
```
add(&Flags::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)
<https://reviews.apache.org/r/55042/#comment231165>
Is it better to define methods in this form?
```
Try<Nothing> enter(const string& root, const bool ramdisk);
Try<Nothing> enterWithPivotRoot(const string& root);
Try<Nothing> enterWithMoveMount(const string& root);
```
src/slave/flags.cpp (line 541)
<https://reviews.apache.org/r/55042/#comment231163>
```
add(&Flags::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.
Posted by Andy Pang <pa...@huawei.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55042/
-----------------------------------------------------------
(Updated \u5341\u4e8c\u6708 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