Re: Review Request 38329: Ensured that the device containing slave work_dir is a shared mount when LinuxFilesystemIsolator is used.

2015-09-12 Thread Jie Yu 


> On Sept. 12, 2015, 6:38 a.m., Timothy Chen wrote:
> > src/slave/containerizer/isolators/filesystem/linux.cpp, line 91
> > 
> >
> > Should we check we did successfully found one that we can mark shared?
> 
> Jie Yu wrote:
> Good catch! Yeah, let me at that check!
> 
> Jie Yu wrote:
> In fact, that should be a CHECK because we should always find one.
> 
> haosdent huang wrote:
> In fact, we have another way. We could 
> 
> ```
> fs::mount(target, target, None(), MS_BIND, NULL);
> fs::mount(None(), target, None(), MS_SHARED, NULL);
> ```
> 
> to mount self. So that we don't need affect the exist system mount points.
> 
> Jie Yu wrote:
> I thought about that too. But that's more complicated to me because we 
> need to do a recursive self bind mount of the work_dir (in case volumes have 
> already been mounted, think about the recovery case), which sounds more 
> invasive to me.
> 
> haosdent huang wrote:
> Oh, thank you for explanation.
> 
> Jie Yu wrote:
> After some testing, I realized that marking '/' as shared mount is more 
> invasive than I thought.
> 
> I'll discard this patch and instead, self bind mount the sandbox 
> directory and mark it as shared. Expect a patch soon.

Posted https://reviews.apache.org/r/38333/


- Jie


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


On Sept. 12, 2015, 7:26 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38329/
> ---
> 
> (Updated Sept. 12, 2015, 7:26 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Timothy Chen, Vinod Kone, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3349
> https://issues.apache.org/jira/browse/MESOS-3349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensured that the device containing slave work_dir is a shared mount when 
> LinuxFilesystemIsolator is used.
> 
> See the discussion thread in the ticket for motivation (also the NOTE in the 
> code).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 0970b3d48b13d5e9d2e0160df5cf14a3dcd0acc9 
> 
> Diff: https://reviews.apache.org/r/38329/diff/
> 
> 
> Testing
> ---
> 
> sudo make check on CentOS5 and CentOS6
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38329: Ensured that the device containing slave work_dir is a shared mount when LinuxFilesystemIsolator is used.

2015-09-12 Thread Jie Yu 


> On Sept. 12, 2015, 6:38 a.m., Timothy Chen wrote:
> > src/slave/containerizer/isolators/filesystem/linux.cpp, line 91
> > 
> >
> > Should we check we did successfully found one that we can mark shared?
> 
> Jie Yu wrote:
> Good catch! Yeah, let me at that check!
> 
> Jie Yu wrote:
> In fact, that should be a CHECK because we should always find one.
> 
> haosdent huang wrote:
> In fact, we have another way. We could 
> 
> ```
> fs::mount(target, target, None(), MS_BIND, NULL);
> fs::mount(None(), target, None(), MS_SHARED, NULL);
> ```
> 
> to mount self. So that we don't need affect the exist system mount points.
> 
> Jie Yu wrote:
> I thought about that too. But that's more complicated to me because we 
> need to do a recursive self bind mount of the work_dir (in case volumes have 
> already been mounted, think about the recovery case), which sounds more 
> invasive to me.
> 
> haosdent huang wrote:
> Oh, thank you for explanation.

After some testing, I realized that marking '/' as shared mount is more 
invasive than I thought.

I'll discard this patch and instead, self bind mount the sandbox directory and 
mark it as shared. Expect a patch soon.


- Jie


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


On Sept. 12, 2015, 7:26 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38329/
> ---
> 
> (Updated Sept. 12, 2015, 7:26 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Timothy Chen, Vinod Kone, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3349
> https://issues.apache.org/jira/browse/MESOS-3349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensured that the device containing slave work_dir is a shared mount when 
> LinuxFilesystemIsolator is used.
> 
> See the discussion thread in the ticket for motivation (also the NOTE in the 
> code).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 0970b3d48b13d5e9d2e0160df5cf14a3dcd0acc9 
> 
> Diff: https://reviews.apache.org/r/38329/diff/
> 
> 
> Testing
> ---
> 
> sudo make check on CentOS5 and CentOS6
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38329: Ensured that the device containing slave work_dir is a shared mount when LinuxFilesystemIsolator is used.

2015-09-12 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [38328, 38329]

All tests passed.

- Mesos ReviewBot


On Sept. 12, 2015, 7:26 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38329/
> ---
> 
> (Updated Sept. 12, 2015, 7:26 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Timothy Chen, Vinod Kone, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3349
> https://issues.apache.org/jira/browse/MESOS-3349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensured that the device containing slave work_dir is a shared mount when 
> LinuxFilesystemIsolator is used.
> 
> See the discussion thread in the ticket for motivation (also the NOTE in the 
> code).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 0970b3d48b13d5e9d2e0160df5cf14a3dcd0acc9 
> 
> Diff: https://reviews.apache.org/r/38329/diff/
> 
> 
> Testing
> ---
> 
> sudo make check on CentOS5 and CentOS6
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38329: Ensured that the device containing slave work_dir is a shared mount when LinuxFilesystemIsolator is used.

2015-09-12 Thread haosdent huang 

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

Ship it!


Ship It!


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


"parent mount" should be "parent mount point"?


- haosdent huang


On Sept. 12, 2015, 7:26 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38329/
> ---
> 
> (Updated Sept. 12, 2015, 7:26 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Timothy Chen, Vinod Kone, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3349
> https://issues.apache.org/jira/browse/MESOS-3349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensured that the device containing slave work_dir is a shared mount when 
> LinuxFilesystemIsolator is used.
> 
> See the discussion thread in the ticket for motivation (also the NOTE in the 
> code).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 0970b3d48b13d5e9d2e0160df5cf14a3dcd0acc9 
> 
> Diff: https://reviews.apache.org/r/38329/diff/
> 
> 
> Testing
> ---
> 
> sudo make check on CentOS5 and CentOS6
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38329: Ensured that the device containing slave work_dir is a shared mount when LinuxFilesystemIsolator is used.

2015-09-12 Thread Jie Yu 

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

(Updated Sept. 12, 2015, 7:26 a.m.)


Review request for mesos, haosdent huang, Timothy Chen, Vinod Kone, and Jiang 
Yan Xu.


Changes
---

Review comments.

Moved the logics to recover because we want to make sure work_dir exists.


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


Repository: mesos


Description
---

Ensured that the device containing slave work_dir is a shared mount when 
LinuxFilesystemIsolator is used.

See the discussion thread in the ticket for motivation (also the NOTE in the 
code).


Diffs (updated)
-

  src/slave/containerizer/isolators/filesystem/linux.cpp 
0970b3d48b13d5e9d2e0160df5cf14a3dcd0acc9 

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


Testing
---

sudo make check on CentOS5 and CentOS6


Thanks,

Jie Yu

Re: Review Request 38329: Ensured that the device containing slave work_dir is a shared mount when LinuxFilesystemIsolator is used.

2015-09-12 Thread haosdent huang 


> On Sept. 12, 2015, 6:38 a.m., Timothy Chen wrote:
> > src/slave/containerizer/isolators/filesystem/linux.cpp, line 91
> > 
> >
> > Should we check we did successfully found one that we can mark shared?
> 
> Jie Yu wrote:
> Good catch! Yeah, let me at that check!
> 
> Jie Yu wrote:
> In fact, that should be a CHECK because we should always find one.
> 
> haosdent huang wrote:
> In fact, we have another way. We could 
> 
> ```
> fs::mount(target, target, None(), MS_BIND, NULL);
> fs::mount(None(), target, None(), MS_SHARED, NULL);
> ```
> 
> to mount self. So that we don't need affect the exist system mount points.
> 
> Jie Yu wrote:
> I thought about that too. But that's more complicated to me because we 
> need to do a recursive self bind mount of the work_dir (in case volumes have 
> already been mounted, think about the recovery case), which sounds more 
> invasive to me.

Oh, thank you for explanation.


- haosdent


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


On Sept. 12, 2015, 6:36 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38329/
> ---
> 
> (Updated Sept. 12, 2015, 6:36 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Timothy Chen, Vinod Kone, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3349
> https://issues.apache.org/jira/browse/MESOS-3349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensured that the device containing slave work_dir is a shared mount when 
> LinuxFilesystemIsolator is used.
> 
> See the discussion thread in the ticket for motivation (also the NOTE in the 
> code).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 0970b3d48b13d5e9d2e0160df5cf14a3dcd0acc9 
> 
> Diff: https://reviews.apache.org/r/38329/diff/
> 
> 
> Testing
> ---
> 
> sudo make check on CentOS5 and CentOS6
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38329: Ensured that the device containing slave work_dir is a shared mount when LinuxFilesystemIsolator is used.

2015-09-12 Thread Jie Yu 


> On Sept. 12, 2015, 6:38 a.m., Timothy Chen wrote:
> > src/slave/containerizer/isolators/filesystem/linux.cpp, line 91
> > 
> >
> > Should we check we did successfully found one that we can mark shared?
> 
> Jie Yu wrote:
> Good catch! Yeah, let me at that check!
> 
> Jie Yu wrote:
> In fact, that should be a CHECK because we should always find one.
> 
> haosdent huang wrote:
> In fact, we have another way. We could 
> 
> ```
> fs::mount(target, target, None(), MS_BIND, NULL);
> fs::mount(None(), target, None(), MS_SHARED, NULL);
> ```
> 
> to mount self. So that we don't need affect the exist system mount points.

I thought about that too. But that's more complicated to me because we need to 
do a recursive self bind mount of the work_dir (in case volumes have already 
been mounted, think about the recovery case), which sounds more invasive to me.


- Jie


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


On Sept. 12, 2015, 6:36 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38329/
> ---
> 
> (Updated Sept. 12, 2015, 6:36 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Timothy Chen, Vinod Kone, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3349
> https://issues.apache.org/jira/browse/MESOS-3349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensured that the device containing slave work_dir is a shared mount when 
> LinuxFilesystemIsolator is used.
> 
> See the discussion thread in the ticket for motivation (also the NOTE in the 
> code).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 0970b3d48b13d5e9d2e0160df5cf14a3dcd0acc9 
> 
> Diff: https://reviews.apache.org/r/38329/diff/
> 
> 
> Testing
> ---
> 
> sudo make check on CentOS5 and CentOS6
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38329: Ensured that the device containing slave work_dir is a shared mount when LinuxFilesystemIsolator is used.

2015-09-11 Thread haosdent huang 


> On Sept. 12, 2015, 6:38 a.m., Timothy Chen wrote:
> > src/slave/containerizer/isolators/filesystem/linux.cpp, line 91
> > 
> >
> > Should we check we did successfully found one that we can mark shared?
> 
> Jie Yu wrote:
> Good catch! Yeah, let me at that check!
> 
> Jie Yu wrote:
> In fact, that should be a CHECK because we should always find one.

In fact, we have another way. We could 

```
fs::mount(target, target, None(), MS_BIND, NULL);
fs::mount(None(), target, None(), MS_SHARED, NULL);
```

to mount self. So that we don't need affect the exist system mount points.


- haosdent


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


On Sept. 12, 2015, 6:36 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38329/
> ---
> 
> (Updated Sept. 12, 2015, 6:36 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Timothy Chen, Vinod Kone, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3349
> https://issues.apache.org/jira/browse/MESOS-3349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensured that the device containing slave work_dir is a shared mount when 
> LinuxFilesystemIsolator is used.
> 
> See the discussion thread in the ticket for motivation (also the NOTE in the 
> code).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 0970b3d48b13d5e9d2e0160df5cf14a3dcd0acc9 
> 
> Diff: https://reviews.apache.org/r/38329/diff/
> 
> 
> Testing
> ---
> 
> sudo make check on CentOS5 and CentOS6
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38329: Ensured that the device containing slave work_dir is a shared mount when LinuxFilesystemIsolator is used.

2015-09-11 Thread Jie Yu 


> On Sept. 12, 2015, 6:38 a.m., Timothy Chen wrote:
> > src/slave/containerizer/isolators/filesystem/linux.cpp, line 91
> > 
> >
> > Should we check we did successfully found one that we can mark shared?
> 
> Jie Yu wrote:
> Good catch! Yeah, let me at that check!

In fact, that should be a CHECK because we should always find one.


- Jie


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


On Sept. 12, 2015, 6:36 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38329/
> ---
> 
> (Updated Sept. 12, 2015, 6:36 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Timothy Chen, Vinod Kone, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3349
> https://issues.apache.org/jira/browse/MESOS-3349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensured that the device containing slave work_dir is a shared mount when 
> LinuxFilesystemIsolator is used.
> 
> See the discussion thread in the ticket for motivation (also the NOTE in the 
> code).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 0970b3d48b13d5e9d2e0160df5cf14a3dcd0acc9 
> 
> Diff: https://reviews.apache.org/r/38329/diff/
> 
> 
> Testing
> ---
> 
> sudo make check on CentOS5 and CentOS6
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38329: Ensured that the device containing slave work_dir is a shared mount when LinuxFilesystemIsolator is used.

2015-09-11 Thread Jie Yu 


> On Sept. 12, 2015, 6:38 a.m., Timothy Chen wrote:
> > src/slave/containerizer/isolators/filesystem/linux.cpp, line 91
> > 
> >
> > Should we check we did successfully found one that we can mark shared?

Good catch! Yeah, let me at that check!


- Jie


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


On Sept. 12, 2015, 6:36 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38329/
> ---
> 
> (Updated Sept. 12, 2015, 6:36 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Timothy Chen, Vinod Kone, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3349
> https://issues.apache.org/jira/browse/MESOS-3349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensured that the device containing slave work_dir is a shared mount when 
> LinuxFilesystemIsolator is used.
> 
> See the discussion thread in the ticket for motivation (also the NOTE in the 
> code).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 0970b3d48b13d5e9d2e0160df5cf14a3dcd0acc9 
> 
> Diff: https://reviews.apache.org/r/38329/diff/
> 
> 
> Testing
> ---
> 
> sudo make check on CentOS5 and CentOS6
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38329: Ensured that the device containing slave work_dir is a shared mount when LinuxFilesystemIsolator is used.

2015-09-11 Thread Timothy Chen 

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



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


Should we check we did successfully found one that we can mark shared?


- Timothy Chen


On Sept. 12, 2015, 6:36 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38329/
> ---
> 
> (Updated Sept. 12, 2015, 6:36 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Timothy Chen, Vinod Kone, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3349
> https://issues.apache.org/jira/browse/MESOS-3349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensured that the device containing slave work_dir is a shared mount when 
> LinuxFilesystemIsolator is used.
> 
> See the discussion thread in the ticket for motivation (also the NOTE in the 
> code).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 0970b3d48b13d5e9d2e0160df5cf14a3dcd0acc9 
> 
> Diff: https://reviews.apache.org/r/38329/diff/
> 
> 
> Testing
> ---
> 
> sudo make check on CentOS5 and CentOS6
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 38329: Ensured that the device containing slave work_dir is a shared mount when LinuxFilesystemIsolator is used.

2015-09-11 Thread Jie Yu 

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

Review request for mesos, haosdent huang, Timothy Chen, Vinod Kone, and Jiang 
Yan Xu.


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


Repository: mesos


Description
---

Ensured that the device containing slave work_dir is a shared mount when 
LinuxFilesystemIsolator is used.

See the discussion thread in the ticket for motivation (also the NOTE in the 
code).


Diffs
-

  src/slave/containerizer/isolators/filesystem/linux.cpp 
0970b3d48b13d5e9d2e0160df5cf14a3dcd0acc9 

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


Testing
---

sudo make check on CentOS5 and CentOS6


Thanks,

Jie Yu