Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2017-03-01 Thread Joris Van Remoortere

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



Closing this review due to inactivity. Please see our 
[guidelines](https://github.com/apache/mesos/blob/master/docs/reopening-reviews.md)
 for reopening reviews.

- Joris Van Remoortere


On Nov. 16, 2016, 3:29 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50599/
> ---
> 
> (Updated Nov. 16, 2016, 3:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Assigned Nvidia GPU devices to docker container based on
> GPUs allocated by Nvidia GPU allocator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 006f929eca0e0a6b1de941821ac72869ba393d2d 
>   src/slave/containerizer/docker.cpp c2ed5240aab9ea9d1a386c44c94e5ae7e98d313c 
> 
> 
> Diff: https://reviews.apache.org/r/50599/diff/22/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-11-15 Thread Yubo Li

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

(Updated Nov. 16, 2016, 3:29 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Assigned Nvidia GPU devices to docker container based on
GPUs allocated by Nvidia GPU allocator.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 006f929eca0e0a6b1de941821ac72869ba393d2d 
  src/slave/containerizer/docker.cpp c2ed5240aab9ea9d1a386c44c94e5ae7e98d313c 

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


Testing
---

make check


Thanks,

Yubo Li



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-11-10 Thread Yubo Li

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

(Updated Nov. 10, 2016, 8:14 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Assigned Nvidia GPU devices to docker container based on
GPUs allocated by Nvidia GPU allocator.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 006f929eca0e0a6b1de941821ac72869ba393d2d 
  src/slave/containerizer/docker.cpp c2ed5240aab9ea9d1a386c44c94e5ae7e98d313c 

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


Testing
---

make check


Thanks,

Yubo Li



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-11-07 Thread Yubo Li

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

(Updated 十一月 7, 2016, 2:02 p.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Assigned Nvidia GPU devices to docker container based on
GPUs allocated by Nvidia GPU allocator.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 006f929eca0e0a6b1de941821ac72869ba393d2d 
  src/slave/containerizer/docker.cpp c2ed5240aab9ea9d1a386c44c94e5ae7e98d313c 

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


Testing
---

make check


Thanks,

Yubo Li



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-11-04 Thread Guangya Liu

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



What about split this to two patches, one is for adding helper function to 
parse a string to a 'Docker::Device', another is assign Nvidia GPU devices to 
docker container.

- Guangya Liu


On 十一月 2, 2016, 7:28 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50599/
> ---
> 
> (Updated 十一月 2, 2016, 7:28 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Assigned Nvidia GPU devices to docker container based on
> GPUs allocated by Nvidia GPU allocator.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
>   src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
>   src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 
> 
> Diff: https://reviews.apache.org/r/50599/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-11-02 Thread Kevin Klues


> On Oct. 26, 2016, 3:22 a.m., Kevin Klues wrote:
> > src/slave/containerizer/docker.cpp, line 723
> > 
> >
> > Can you just use `stringify()` here? Also, can you use `Path().join()` 
> > instead of just `+`.
> 
> Yubo Li wrote:
> I changed the code to `path::join(nvidiaDataPrefix, 
> stringify(gpu.minor));`, but the output path becomes `/dev/nvidia/0`, not 
> `/dev/nvidia0` we are expected. So that I keeped `+` instead of 
> `path::join()`.

Yeah, that makes sense. A '+' is actually the correct thing here afterall.


- Kevin


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


On Nov. 2, 2016, 7:28 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50599/
> ---
> 
> (Updated Nov. 2, 2016, 7:28 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Assigned Nvidia GPU devices to docker container based on
> GPUs allocated by Nvidia GPU allocator.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
>   src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
>   src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 
> 
> Diff: https://reviews.apache.org/r/50599/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-11-02 Thread Yubo Li

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

(Updated Nov. 2, 2016, 7:28 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Assigned Nvidia GPU devices to docker container based on
GPUs allocated by Nvidia GPU allocator.


Diffs (updated)
-

  src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
  src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
  src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 

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


Testing
---

make check


Thanks,

Yubo Li



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-11-01 Thread Yubo Li


> On 十一月 1, 2016, 3:04 p.m., Guangya Liu wrote:
> > src/slave/containerizer/docker.cpp, line 701
> > 
> >
> > You need to keep the comments from Kevin as open here 
> > https://reviews.apache.org/r/50599/#comment223322

OK, I re-opened Kevin's comment.


- Yubo


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


On 十一月 1, 2016, 9:11 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50599/
> ---
> 
> (Updated 十一月 1, 2016, 9:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Assigned Nvidia GPU devices to docker container based on
> GPUs allocated by Nvidia GPU allocator.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
>   src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
>   src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 
> 
> Diff: https://reviews.apache.org/r/50599/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-11-01 Thread Guangya Liu

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


Fix it, then Ship it!




Ship It!


src/slave/containerizer/docker.hpp (line 509)


You also need to `include ` at the beginning of this file.



src/slave/containerizer/docker.cpp (line 701)


You need to keep the comments from Kevin as open here 
https://reviews.apache.org/r/50599/#comment223322



src/slave/containerizer/docker.cpp (lines 705 - 710)


What about update this a bit to make it less jagged?

```
  // NOTE: GPU devices permissions are required to be `rmw` by default,
  // that is because GPU tasks launched in the container may need to
  // read/write/mknod to GPU devices in their lifecycle.
  //
  // `container->devices` records Nvidia GPU devices to be attached to
  // the docker container.
```


- Guangya Liu


On 十一月 1, 2016, 9:11 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50599/
> ---
> 
> (Updated 十一月 1, 2016, 9:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Assigned Nvidia GPU devices to docker container based on
> GPUs allocated by Nvidia GPU allocator.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
>   src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
>   src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 
> 
> Diff: https://reviews.apache.org/r/50599/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-11-01 Thread Yubo Li

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

(Updated Nov. 1, 2016, 9:11 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Assigned Nvidia GPU devices to docker container based on
GPUs allocated by Nvidia GPU allocator.


Diffs (updated)
-

  src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
  src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
  src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 

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


Testing
---

make check


Thanks,

Yubo Li



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-11-01 Thread Guangya Liu


> On 十月 26, 2016, 3:22 a.m., Kevin Klues wrote:
> > src/slave/containerizer/docker.cpp, line 691
> > 
> >
> > This is OK since we do this the same in other functions for this class, 
> > but we should really wrap pointers like this in Owned<> or Shared<> 
> > depending on their semantics.

I think that you did not resolve this, please keep this open.


- Guangya


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


On 十一月 1, 2016, 6:11 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50599/
> ---
> 
> (Updated 十一月 1, 2016, 6:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Assigned Nvidia GPU devices to docker container based on
> GPUs allocated by Nvidia GPU allocator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
>   src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 
> 
> Diff: https://reviews.apache.org/r/50599/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-11-01 Thread Guangya Liu

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




src/slave/containerizer/docker.cpp (lines 705 - 707)


kill this, there is no constant variable now



src/slave/containerizer/docker.cpp (line 712)


Why not kill this and use "/dev/nvidia" directly in #729?


- Guangya Liu


On 十一月 1, 2016, 6:11 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50599/
> ---
> 
> (Updated 十一月 1, 2016, 6:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Assigned Nvidia GPU devices to docker container based on
> GPUs allocated by Nvidia GPU allocator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
>   src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 
> 
> Diff: https://reviews.apache.org/r/50599/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-11-01 Thread Yubo Li

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

(Updated Nov. 1, 2016, 6:11 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Assigned Nvidia GPU devices to docker container based on
GPUs allocated by Nvidia GPU allocator.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
  src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 

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


Testing
---

make check


Thanks,

Yubo Li



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-10-31 Thread Yubo Li

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

(Updated Oct. 31, 2016, 7:10 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Assigned Nvidia GPU devices to docker container based on
GPUs allocated by Nvidia GPU allocator.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
  src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 

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


Testing
---

make check


Thanks,

Yubo Li



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-10-29 Thread Yubo Li


> On 十月 26, 2016, 3:22 a.m., Kevin Klues wrote:
> > src/slave/containerizer/docker.cpp, line 723
> > 
> >
> > Can you just use `stringify()` here? Also, can you use `Path().join()` 
> > instead of just `+`.

I changed the code to `path::join(nvidiaDataPrefix, stringify(gpu.minor));`, 
but the output path becomes `/dev/nvidia/0`, not `/dev/nvidia0` we are 
expected. So that I keeped `+` instead of `path::join()`.


- Yubo


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


On 十月 24, 2016, 5 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50599/
> ---
> 
> (Updated 十月 24, 2016, 5 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Assigned Nvidia GPU devices to docker container based on
> GPUs allocated by Nvidia GPU allocator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
>   src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 
> 
> Diff: https://reviews.apache.org/r/50599/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-10-27 Thread Yubo Li


> On 十月 26, 2016, 3:22 a.m., Kevin Klues wrote:
> > src/slave/containerizer/docker.cpp, line 700
> > 
> >
> > I would reword this to explicitly mention that tasks launched with the 
> > docker containerizer require these permissions in order to operate.

Added comments as follow:
```
// NOTE: GPU devices permissions are required to be `rmw` by default, that
// is because GPU tasks launched in the container may need to read/write/mknod
// to GPU devices in their lifecycle.
```


- Yubo


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


On 十月 24, 2016, 5 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50599/
> ---
> 
> (Updated 十月 24, 2016, 5 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Assigned Nvidia GPU devices to docker container based on
> GPUs allocated by Nvidia GPU allocator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
>   src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 
> 
> Diff: https://reviews.apache.org/r/50599/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-10-25 Thread Kevin Klues

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




src/slave/containerizer/docker.hpp (line 509)


I dont think we need an Option here. We can just use a vector alone, and 
then check if there are any devices in it with a `.empty()` instead of a 
`.isSome()`.



src/slave/containerizer/docker.cpp (line 691)


This is OK since we do this the same in other functions for this class, but 
we should really wrap pointers like this in Owned<> or Shared<> depending on 
their semantics.



src/slave/containerizer/docker.cpp (line 694)


This is unnecessary, given the comment above.

Doing this here would have been problematic anyway, because it's possible 
that future code would want to add things to this vector outside of this 
function.



src/slave/containerizer/docker.cpp (lines 696 - 698)


We should reword this to:
```
  // TODO(Yubo): Nvidia devices are hard-coded here. We should move these 
  // constants to a common file shared by both the mesos containerizer and
  // the docker containerizer.
```



src/slave/containerizer/docker.cpp (line 700)


I would reword this to explicitly mention that tasks launched with the 
docker containerizer require these permissions in order to operate.



src/slave/containerizer/docker.cpp (lines 701 - 726)


It might be better to just set the Device fields explicity.

Otherwise, building a string and passing it to the parse function should be 
OK, but we need to use the new version of the parse function, which *only* 
takes a string, rather than the constructor like one used here.



src/slave/containerizer/docker.cpp (line 705)


This should be "rmw", not "mrw"



src/slave/containerizer/docker.cpp (line 722)


Can we rename this variable: `devicePath`.



src/slave/containerizer/docker.cpp (line 723)


Can you just use `stringify()` here? Also, can you use `Path().join()` 
instead of just `+`.


- Kevin Klues


On Oct. 24, 2016, 5 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50599/
> ---
> 
> (Updated Oct. 24, 2016, 5 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Assigned Nvidia GPU devices to docker container based on
> GPUs allocated by Nvidia GPU allocator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
>   src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 
> 
> Diff: https://reviews.apache.org/r/50599/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-10-23 Thread Yubo Li

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

(Updated Oct. 24, 2016, 5 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Assigned Nvidia GPU devices to docker container based on
GPUs allocated by Nvidia GPU allocator.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
  src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 

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


Testing
---

make check


Thanks,

Yubo Li



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-10-21 Thread Yubo Li

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

(Updated 十月 21, 2016, 9:11 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Assigned Nvidia GPU devices to docker container based on
GPUs allocated by Nvidia GPU allocator.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
  src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 

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


Testing
---

make check


Thanks,

Yubo Li



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-10-21 Thread Yubo Li

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

(Updated 十月 21, 2016, 6:29 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Assigned Nvidia GPU devices to docker container based on
GPUs allocated by Nvidia GPU allocator.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
  src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 

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


Testing
---

make check


Thanks,

Yubo Li



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-10-20 Thread Yubo Li

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

(Updated 十月 20, 2016, 10:01 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Assigned Nvidia GPU devices to docker container based on
GPUs allocated by Nvidia GPU allocator.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
  src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 

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


Testing
---

make check


Thanks,

Yubo Li



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-10-14 Thread Yubo Li

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

(Updated 十月 14, 2016, 9:57 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Assigned Nvidia GPU devices to docker container based on
GPUs allocated by Nvidia GPU allocator.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
  src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 

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


Testing
---

make check


Thanks,

Yubo Li



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-10-13 Thread Guangya Liu

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



You need a rebase for this patch.


src/slave/containerizer/docker.cpp (line 691)


s/"container->devices"/`container->devices`



src/slave/containerizer/docker.cpp (line 707)


two spaces


- Guangya Liu


On 十月 11, 2016, 8:18 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50599/
> ---
> 
> (Updated 十月 11, 2016, 8:18 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Assigned Nvidia GPU devices to docker container based on
> GPUs allocated by Nvidia GPU allocator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
>   src/slave/containerizer/docker.cpp 1d27761fcb3f310cf954d45ed41f4c89ecbd5982 
> 
> Diff: https://reviews.apache.org/r/50599/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-10-11 Thread Yubo Li

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

(Updated 十月 11, 2016, 8:18 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Assigned Nvidia GPU devices to docker container based on
GPUs allocated by Nvidia GPU allocator.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
  src/slave/containerizer/docker.cpp 1d27761fcb3f310cf954d45ed41f4c89ecbd5982 

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


Testing
---

make check


Thanks,

Yubo Li



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-10-09 Thread Guangya Liu

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




src/slave/containerizer/docker.cpp (line 677)


```
Container* container = containers_.at(containerId);
```


- Guangya Liu


On 九月 22, 2016, 6:20 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50599/
> ---
> 
> (Updated 九月 22, 2016, 6:20 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Assigned Nvidia GPU devices to docker container based on
> GPUs allocated by Nvidia GPU allocator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
>   src/slave/containerizer/docker.cpp 1d27761fcb3f310cf954d45ed41f4c89ecbd5982 
> 
> Diff: https://reviews.apache.org/r/50599/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-09-22 Thread Yubo Li

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

(Updated 九月 22, 2016, 6:20 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Assigned Nvidia GPU devices to docker container based on
GPUs allocated by Nvidia GPU allocator.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
  src/slave/containerizer/docker.cpp 1d27761fcb3f310cf954d45ed41f4c89ecbd5982 

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


Testing
---

make check


Thanks,

Yubo Li



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-09-20 Thread Yubo Li


> On 八月 25, 2016, 6:08 p.m., Kevin Klues wrote:
> > src/slave/containerizer/docker.cpp, lines 230-249
> > 
> >
> > I would probably move this into the next patch alongside the other 
> > changes to the flags and not even bother with the comment.

Moved to next patch.


- Yubo


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


On 九月 20, 2016, 9:25 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50599/
> ---
> 
> (Updated 九月 20, 2016, 9:25 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Assigned Nvidia GPU devices to docker container based on
> GPUs allocated by Nvidia GPU allocator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp f2a06065cf99fed934c2c1ffc47461ec8a97f50d 
>   src/slave/containerizer/docker.cpp 5c1ee8e467d1c54c60b67dc5275ef71e1bb90723 
> 
> Diff: https://reviews.apache.org/r/50599/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>



Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

2016-09-20 Thread Yubo Li

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

(Updated 九月 20, 2016, 9:25 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


Summary (updated)
-

Assigned Nvidia GPU devices to docker container.


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


Repository: mesos


Description (updated)
---

Assigned Nvidia GPU devices to docker container based on
GPUs allocated by Nvidia GPU allocator.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp f2a06065cf99fed934c2c1ffc47461ec8a97f50d 
  src/slave/containerizer/docker.cpp 5c1ee8e467d1c54c60b67dc5275ef71e1bb90723 

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


Testing
---

make check


Thanks,

Yubo Li