Re: Review Request 39719: Fixed marking mounts as slave in ubuntu.

2015-11-04 Thread Jie Yu 

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

Ship it!



src/slave/containerizer/mesos/mount.cpp (line 72)


One extra blank line above.


- Jie Yu


On Nov. 4, 2015, 8:57 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39719/
> ---
> 
> (Updated Nov. 4, 2015, 8:57 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-3806
> https://issues.apache.org/jira/browse/MESOS-3806
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed marking mounts as slave in ubuntu. More details please see MESOS-3806
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt d107e329cc6887cd9d4ce3706dfc6ce6080d0289 
>   src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> a126cd6ee15adb8d92e85ab562e998e3647f999a 
>   src/slave/containerizer/mesos/main.cpp 
> 0e1793128b56ab2e0aa2d263383c9de47ffe25d1 
>   src/slave/containerizer/mesos/mount.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/mount.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39719/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 39719: Fixed marking mounts as slave in ubuntu.

2015-11-04 Thread Timothy Chen 

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

(Updated Nov. 4, 2015, 8:57 p.m.)


Review request for mesos and Jie Yu.


Bugs: MESOS-3806
https://issues.apache.org/jira/browse/MESOS-3806


Repository: mesos


Description
---

Fixed marking mounts as slave in ubuntu. More details please see MESOS-3806


Diffs (updated)
-

  src/CMakeLists.txt d107e329cc6887cd9d4ce3706dfc6ce6080d0289 
  src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
a126cd6ee15adb8d92e85ab562e998e3647f999a 
  src/slave/containerizer/mesos/main.cpp 
0e1793128b56ab2e0aa2d263383c9de47ffe25d1 
  src/slave/containerizer/mesos/mount.hpp PRE-CREATION 
  src/slave/containerizer/mesos/mount.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 39719: Fixed marking mounts as slave in ubuntu.

2015-11-03 Thread Timothy Chen 


> On Nov. 3, 2015, 1:01 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/mark_mounts_rslave.hpp, line 36
> > 
> >
> > I am thinking about making this more general and can be extended in 
> > case we need to (e.g., supporting make-slave, make-rshared, or other mount 
> > operation).
> > 
> > How about calling this Subcommand:
> > `MesosContainerizerMount`
> > 
> > ```
> > class MesosContainerizerMount : public Subcommand
> > {
> > public:
> >   static constexpr char NAME[] = ""
> >   static constexpr char MAKE_RSLAVE[] = "make-rslave";
> >   
> >   struct Flags : public flags:: FlagsBase
> >   {
> > Flags();
> > 
> > std::string operation;
> > Option path;
> >   };
> > };
> > 
> > int MesosContainerizerMount::execute()
> > {
> >   if (flags.operation == MAKE_RSLAVE) {
> > if (flags.path.isNone()) {
> >   ..
> > }
> > 
> > fs::mount(...);
> >   } else {
> > cerr << "Unknown operation: " << flags.operation;
> > return 1;
> >   }
> > }
> > ```

Sure, I wasn't sure if we need to support all kinds of mounts but I'll leave 
that option open by putting the code like you suggested.


- Timothy


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


On Nov. 2, 2015, 6:52 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39719/
> ---
> 
> (Updated Nov. 2, 2015, 6:52 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-3806
> https://issues.apache.org/jira/browse/MESOS-3806
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed marking mounts as slave in ubuntu. More details please see MESOS-3806
> 
> 
> Diffs
> -
> 
>   src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> a126cd6ee15adb8d92e85ab562e998e3647f999a 
>   src/slave/containerizer/mesos/main.cpp 
> 0e1793128b56ab2e0aa2d263383c9de47ffe25d1 
>   src/slave/containerizer/mesos/mark_mounts_rslave.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/mark_mounts_rslave.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39719/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 39719: Fixed marking mounts as slave in ubuntu.

2015-11-02 Thread Jie Yu 

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


As Alex suggested, please update cmakelist as well.


src/Makefile.am (line 860)


Probably call this mount.hpp|cpp. See comments below.



src/slave/containerizer/mesos/isolators/filesystem/linux.cpp (lines 469 - 470)


```
MesosContainerizerMount::Flags mountFlags;
mountFlags.operation = MesosContainerizerMount::MAKE_RSLAVE;
mountFlags.path = "/";

...
```



src/slave/containerizer/mesos/mark_mounts_rslave.hpp (line 36)


I am thinking about making this more general and can be extended in case we 
need to (e.g., supporting make-slave, make-rshared, or other mount operation).

How about calling this Subcommand:
`MesosContainerizerMount`

```
class MesosContainerizerMount : public Subcommand
{
public:
  static constexpr char NAME[] = ""
  static constexpr char MAKE_RSLAVE[] = "make-rslave";
  
  struct Flags : public flags:: FlagsBase
  {
Flags();

std::string operation;
Option path;
  };
};

int MesosContainerizerMount::execute()
{
  if (flags.operation == MAKE_RSLAVE) {
if (flags.path.isNone()) {
  ..
}

fs::mount(...);
  } else {
cerr << "Unknown operation: " << flags.operation;
return 1;
  }
}
```


- Jie Yu


On Nov. 2, 2015, 6:52 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39719/
> ---
> 
> (Updated Nov. 2, 2015, 6:52 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-3806
> https://issues.apache.org/jira/browse/MESOS-3806
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed marking mounts as slave in ubuntu. More details please see MESOS-3806
> 
> 
> Diffs
> -
> 
>   src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> a126cd6ee15adb8d92e85ab562e998e3647f999a 
>   src/slave/containerizer/mesos/main.cpp 
> 0e1793128b56ab2e0aa2d263383c9de47ffe25d1 
>   src/slave/containerizer/mesos/mark_mounts_rslave.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/mark_mounts_rslave.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39719/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 39719: Fixed marking mounts as slave in ubuntu.

2015-11-02 Thread Alex Clemmer 

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



src/Makefile.am (line 735)


It looks like it might make sense to add this to `src/CMakeLists.txt` also?


- Alex Clemmer


On Nov. 2, 2015, 6:52 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39719/
> ---
> 
> (Updated Nov. 2, 2015, 6:52 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-3806
> https://issues.apache.org/jira/browse/MESOS-3806
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed marking mounts as slave in ubuntu. More details please see MESOS-3806
> 
> 
> Diffs
> -
> 
>   src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> a126cd6ee15adb8d92e85ab562e998e3647f999a 
>   src/slave/containerizer/mesos/main.cpp 
> 0e1793128b56ab2e0aa2d263383c9de47ffe25d1 
>   src/slave/containerizer/mesos/mark_mounts_rslave.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/mark_mounts_rslave.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39719/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 39719: Fixed marking mounts as slave in ubuntu.

2015-11-02 Thread Timothy Chen 

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

(Updated Nov. 2, 2015, 6:52 p.m.)


Review request for mesos and Jie Yu.


Bugs: MESOS-3806
https://issues.apache.org/jira/browse/MESOS-3806


Repository: mesos


Description
---

Fixed marking mounts as slave in ubuntu. More details please see MESOS-3806


Diffs (updated)
-

  src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
a126cd6ee15adb8d92e85ab562e998e3647f999a 
  src/slave/containerizer/mesos/main.cpp 
0e1793128b56ab2e0aa2d263383c9de47ffe25d1 
  src/slave/containerizer/mesos/mark_mounts_rslave.hpp PRE-CREATION 
  src/slave/containerizer/mesos/mark_mounts_rslave.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 39719: Fixed marking mounts as slave in ubuntu.

2015-10-31 Thread Jie Yu 

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



src/Makefile.am (lines 1165 - 1170)


You don't have to add a new binary. You can just add a subcommand for 
mesos-containerizer binary. See the example in 
src/containerizer/mesos/launch.hpp|cpp


- Jie Yu


On Oct. 31, 2015, 5:55 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39719/
> ---
> 
> (Updated Oct. 31, 2015, 5:55 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-3806
> https://issues.apache.org/jira/browse/MESOS-3806
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed marking mounts as slave in ubuntu. More details please see MESOS-3806
> 
> 
> Diffs
> -
> 
>   src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> a126cd6ee15adb8d92e85ab562e998e3647f999a 
>   src/slave/containerizer/mesos/mark_mounts_rslave.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39719/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 39719: Fixed marking mounts as slave in ubuntu.

2015-10-31 Thread Timothy Chen 

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

(Updated Oct. 31, 2015, 5:55 p.m.)


Review request for mesos and Jie Yu.


Bugs: MESOS-3806
https://issues.apache.org/jira/browse/MESOS-3806


Repository: mesos


Description
---

Fixed marking mounts as slave in ubuntu. More details please see MESOS-3806


Diffs (updated)
-

  src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
a126cd6ee15adb8d92e85ab562e998e3647f999a 
  src/slave/containerizer/mesos/mark_mounts_rslave.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 39719: Fixed marking mounts as slave in ubuntu.

2015-10-29 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [39719]

All tests passed.

- Mesos ReviewBot


On Oct. 29, 2015, 3:44 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39719/
> ---
> 
> (Updated Oct. 29, 2015, 3:44 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-3806
> https://issues.apache.org/jira/browse/MESOS-3806
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed marking mounts as slave in ubuntu. More details please see MESOS-3806
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> a126cd6ee15adb8d92e85ab562e998e3647f999a 
> 
> Diff: https://reviews.apache.org/r/39719/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 39719: Fixed marking mounts as slave in ubuntu.

2015-10-29 Thread Timothy Chen 

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

(Updated Oct. 29, 2015, 3:44 p.m.)


Review request for mesos and Jie Yu.


Bugs: MESOS-3806
https://issues.apache.org/jira/browse/MESOS-3806


Repository: mesos


Description
---

Fixed marking mounts as slave in ubuntu. More details please see MESOS-3806


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
a126cd6ee15adb8d92e85ab562e998e3647f999a 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 39719: Fixed marking mounts as slave in ubuntu.

2015-10-28 Thread Jie Yu 

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

Ship it!


Thanks Tim!


src/slave/containerizer/mesos/isolators/filesystem/linux.cpp (line 465)


Please add a `// NOTE:` header.



src/slave/containerizer/mesos/isolators/filesystem/linux.cpp (line 468)


Can you follow the syntax below: each command one line?
```
out << "cat /proc/self/mountinfo | "
<< "cut -d' ' -f5 | " // '-f5' is the mount target.
<< "xargs --no-run-if-empty mount --make-slave\n";
```

I suggest we don't mask errors for this case because if this command does 
not work, we could end up with in a bad situation (the one you saw in the test).


- Jie Yu


On Oct. 28, 2015, 12:55 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39719/
> ---
> 
> (Updated Oct. 28, 2015, 12:55 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-3806
> https://issues.apache.org/jira/browse/MESOS-3806
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed marking mounts as slave in ubuntu. More details please see MESOS-3806
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> a126cd6ee15adb8d92e85ab562e998e3647f999a 
> 
> Diff: https://reviews.apache.org/r/39719/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 39719: Fixed marking mounts as slave in ubuntu.

2015-10-28 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [39719]

All tests passed.

- Mesos ReviewBot


On Oct. 28, 2015, 12:55 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39719/
> ---
> 
> (Updated Oct. 28, 2015, 12:55 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-3806
> https://issues.apache.org/jira/browse/MESOS-3806
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed marking mounts as slave in ubuntu. More details please see MESOS-3806
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> a126cd6ee15adb8d92e85ab562e998e3647f999a 
> 
> Diff: https://reviews.apache.org/r/39719/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Review Request 39719: Fixed marking mounts as slave in ubuntu.

2015-10-28 Thread Timothy Chen 

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

Review request for mesos and Jie Yu.


Bugs: MESOS-3806
https://issues.apache.org/jira/browse/MESOS-3806


Repository: mesos


Description
---

Fixed marking mounts as slave in ubuntu.


Diffs
-

  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
a126cd6ee15adb8d92e85ab562e998e3647f999a 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 39719: Fixed marking mounts as slave in ubuntu.

2015-10-28 Thread Timothy Chen 

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

(Updated Oct. 28, 2015, 12:55 p.m.)


Review request for mesos and Jie Yu.


Bugs: MESOS-3806
https://issues.apache.org/jira/browse/MESOS-3806


Repository: mesos


Description (updated)
---

Fixed marking mounts as slave in ubuntu. More details please see MESOS-3806


Diffs
-

  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
a126cd6ee15adb8d92e85ab562e998e3647f999a 

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


Testing
---

make check


Thanks,

Timothy Chen