Re: Review Request 45674: Implemented recover() for dvd isolator.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 23, 2016, 8:20 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45674/
> ---
> 
> (Updated April 23, 2016, 8:20 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5104
> https://issues.apache.org/jira/browse/MESOS-5104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented recover() for dvd isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> bedc687cc280d0b721fb84801039fd3614364cca 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 915e5ae755a55a02b7dfcda88165f27346cad955 
> 
> Diff: https://reviews.apache.org/r/45674/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45674: Implemented recover() for dvd isolator.

[Jie Yu]

> On April 24, 2016, 5:52 p.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp, lines 
> > 133-134
> > 
> >
> > Let's quote the containerId.

We don't quote containerId because it's generated by Mesos.


- Jie


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


On April 23, 2016, 8:20 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45674/
> ---
> 
> (Updated April 23, 2016, 8:20 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5104
> https://issues.apache.org/jira/browse/MESOS-5104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented recover() for dvd isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> bedc687cc280d0b721fb84801039fd3614364cca 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 915e5ae755a55a02b7dfcda88165f27346cad955 
> 
> Diff: https://reviews.apache.org/r/45674/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45674: Implemented recover() for dvd isolator.

[Gilbert Song]

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




src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (lines 133 - 
134)


Let's quote the containerId.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (lines 157 - 
158)


ditto.


- Gilbert Song


On April 23, 2016, 1:20 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45674/
> ---
> 
> (Updated April 23, 2016, 1:20 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5104
> https://issues.apache.org/jira/browse/MESOS-5104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented recover() for dvd isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> bedc687cc280d0b721fb84801039fd3614364cca 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 915e5ae755a55a02b7dfcda88165f27346cad955 
> 
> Diff: https://reviews.apache.org/r/45674/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45674: Implemented recover() for dvd isolator.

[Guangya Liu]

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

(Updated 四月 23, 2016, 8:20 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Implemented recover() for dvd isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
bedc687cc280d0b721fb84801039fd3614364cca 
  src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
915e5ae755a55a02b7dfcda88165f27346cad955 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 45674: Implemented recover() for dvd isolator.

[Guangya Liu]

> On 四月 22, 2016, 10:20 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp, lines 
> > 181-185
> > 
> >
> > Please do not copy the comments blindly. The comments here does not 
> > apply to this isolator. Please remove it.

Seems this comment also fit to the `_recover` here well, as the `_recover` is 
also adding `Info` to `infos` and will not add it if the checkpoint directory 
was deleted.


- Guangya


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


On 四月 22, 2016, 7:15 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45674/
> ---
> 
> (Updated 四月 22, 2016, 7:15 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5104
> https://issues.apache.org/jira/browse/MESOS-5104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented recover() for dvd isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt f991743f1a467f24a786e925983390a411792327 
>   src/Makefile.am e024c6d65608a55765e527a8668c415723dcfcca 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> bedc687cc280d0b721fb84801039fd3614364cca 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 915e5ae755a55a02b7dfcda88165f27346cad955 
>   src/slave/containerizer/mesos/isolators/docker/volume/state.hpp 
> bc0c3299f9f3e351aaa20652b784bccbfd40c1a5 
>   src/slave/containerizer/mesos/isolators/docker/volume/state.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45674/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45674: Implemented recover() for dvd isolator.

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (lines 120 - 
121)


Copy paste error? Please remove the comments here.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (lines 181 - 
185)


Please do not copy the comments blindly. The comments here does not apply 
to this isolator. Please remove it.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 218)


no need to introduce a separate file and parse funcition. Simply put the 
parsing logic here.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 225)


s/dockerVolumes/volumes/



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 227)


This is no longer needed per my comments in ealier patches.


- Jie Yu


On April 22, 2016, 7:15 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45674/
> ---
> 
> (Updated April 22, 2016, 7:15 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5104
> https://issues.apache.org/jira/browse/MESOS-5104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented recover() for dvd isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt f991743f1a467f24a786e925983390a411792327 
>   src/Makefile.am e024c6d65608a55765e527a8668c415723dcfcca 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> bedc687cc280d0b721fb84801039fd3614364cca 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 915e5ae755a55a02b7dfcda88165f27346cad955 
>   src/slave/containerizer/mesos/isolators/docker/volume/state.hpp 
> bc0c3299f9f3e351aaa20652b784bccbfd40c1a5 
>   src/slave/containerizer/mesos/isolators/docker/volume/state.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45674/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45674: Implemented recover() for dvd isolator.

[Guangya Liu]

> On 四月 22, 2016, 7:41 a.m., haosdent huang wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp, line 171
> > 
> >
> > Should wait for cleanup before return like:
> > ```
> >   list futures;
> >   foreach (const ContainerID& containerId, unknownOrphans) {
> > futures.push_back(cleanup(containerId));
> >   }
> > 
> >   return collect(futures)
> > .then([]() -> Future { return Nothing(); });
> > ```

What would be the problem if did not wait for clean up here? The `cleanup()` 
already have some logic handling wait logic for `unmount`.


- Guangya


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


On 四月 22, 2016, 7:15 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45674/
> ---
> 
> (Updated 四月 22, 2016, 7:15 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5104
> https://issues.apache.org/jira/browse/MESOS-5104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented recover() for dvd isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt f991743f1a467f24a786e925983390a411792327 
>   src/Makefile.am e024c6d65608a55765e527a8668c415723dcfcca 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> bedc687cc280d0b721fb84801039fd3614364cca 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 915e5ae755a55a02b7dfcda88165f27346cad955 
>   src/slave/containerizer/mesos/isolators/docker/volume/state.hpp 
> bc0c3299f9f3e351aaa20652b784bccbfd40c1a5 
>   src/slave/containerizer/mesos/isolators/docker/volume/state.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45674/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45674: Implemented recover() for dvd isolator.

[haosdent huang]

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




src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 171)


Should wait for cleanup before return like:
```
  list futures;
  foreach (const ContainerID& containerId, unknownOrphans) {
futures.push_back(cleanup(containerId));
  }

  return collect(futures)
.then([]() -> Future { return Nothing(); });
```


- haosdent huang


On April 22, 2016, 7:15 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45674/
> ---
> 
> (Updated April 22, 2016, 7:15 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5104
> https://issues.apache.org/jira/browse/MESOS-5104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented recover() for dvd isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt f991743f1a467f24a786e925983390a411792327 
>   src/Makefile.am e024c6d65608a55765e527a8668c415723dcfcca 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> bedc687cc280d0b721fb84801039fd3614364cca 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 915e5ae755a55a02b7dfcda88165f27346cad955 
>   src/slave/containerizer/mesos/isolators/docker/volume/state.hpp 
> bc0c3299f9f3e351aaa20652b784bccbfd40c1a5 
>   src/slave/containerizer/mesos/isolators/docker/volume/state.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45674/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45674: Implemented recover() for dvd isolator.

[Guangya Liu]

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

(Updated 四月 22, 2016, 7:15 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Implemented recover() for dvd isolator.


Diffs (updated)
-

  src/CMakeLists.txt f991743f1a467f24a786e925983390a411792327 
  src/Makefile.am e024c6d65608a55765e527a8668c415723dcfcca 
  src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
bedc687cc280d0b721fb84801039fd3614364cca 
  src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
915e5ae755a55a02b7dfcda88165f27346cad955 
  src/slave/containerizer/mesos/isolators/docker/volume/state.hpp 
bc0c3299f9f3e351aaa20652b784bccbfd40c1a5 
  src/slave/containerizer/mesos/isolators/docker/volume/state.cpp PRE-CREATION 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 45674: Implemented recover() for dvd isolator.

[Gilbert Song]

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




src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 121)


add apace before `should`



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (lines 122 - 
124)


Let's add a VLOG(1).



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 223)


ditto.


- Gilbert Song


On April 21, 2016, 2:13 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45674/
> ---
> 
> (Updated April 21, 2016, 2:13 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5104
> https://issues.apache.org/jira/browse/MESOS-5104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented recover() for dvd isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt f991743f1a467f24a786e925983390a411792327 
>   src/Makefile.am 922b5984b67ace97c142b52aaa8221323147a7ff 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> bedc687cc280d0b721fb84801039fd3614364cca 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 915e5ae755a55a02b7dfcda88165f27346cad955 
>   src/slave/containerizer/mesos/isolators/docker/volume/state.hpp 
> bc0c3299f9f3e351aaa20652b784bccbfd40c1a5 
>   src/slave/containerizer/mesos/isolators/docker/volume/state.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45674/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45674: Implemented recover() for dvd isolator.

[Guangya Liu]

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

(Updated 四月 21, 2016, 9:13 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Implemented recover() for dvd isolator.


Diffs (updated)
-

  src/CMakeLists.txt f991743f1a467f24a786e925983390a411792327 
  src/Makefile.am 922b5984b67ace97c142b52aaa8221323147a7ff 
  src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
bedc687cc280d0b721fb84801039fd3614364cca 
  src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
915e5ae755a55a02b7dfcda88165f27346cad955 
  src/slave/containerizer/mesos/isolators/docker/volume/state.hpp 
bc0c3299f9f3e351aaa20652b784bccbfd40c1a5 
  src/slave/containerizer/mesos/isolators/docker/volume/state.cpp PRE-CREATION 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 45674: Implemented recover() for dvd isolator.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45214, 45217, 45265, 45270, 45360, 45373, 45370, 45375, 
45377, 45326, 44454, 45671, 45674]

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

- Mesos ReviewBot


On April 4, 2016, 11:05 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45674/
> ---
> 
> (Updated April 4, 2016, 11:05 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5104
> https://issues.apache.org/jira/browse/MESOS-5104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented recover() for dvd isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45674/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Review Request 45674: Implemented recover() for dvd isolator.

[Guangya Liu]

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

Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Implemented recover() for dvd isolator.


Diffs
-

  src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp PRE-CREATION 

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


Testing
---

make
make check


Thanks,

Guangya Liu