Re: Review Request 50599: Passed allocated GPUs to 'devices' entry of 'docker::Flags'.

[Yubo Li]

> On 八月 25, 2016, 6:01 p.m., Kevin Klues wrote:
> > src/slave/containerizer/docker.cpp, lines 685-719
> > 
> >
> > If you store `devices` as a vector of type `Device`, and you use the 
> > suggestion from the previous patch to add a static `parse()` function to 
> > `Device`, this whole code block becomes:
> > 
> > ```
> > container->devices.push_back(
> > Device::parse("/dev/nvidiactl:/dev/nvidiactl:rwm"));
> > 
> > container->devices.push_back(
> > Device::parse("/dev/nvidia-uvm:/dev/nvidia-uvm:rwm"));
> > 
> > if (os::exists("/dev/nvidia-uvm-tools")) {
> >   container->devices.push_back(
> >   Device::parse("/dev/nvidia-uvm-tools:/dev/nvidia-uvm-tools:rwm"));
> > }
> > ```
> > 
> > The join as a single comma-separated string should happen only when you 
> > are ready to add it to the flags.

Good suggestion. Changed.


- Yubo


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


On 八月 22, 2016, 10:12 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50599/
> ---
> 
> (Updated 八月 22, 2016, 10:12 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
> ---
> 
> Added a new string entry 'devices' to 'docker::Flags' to
> indicate host devices exposed into docker container, and
> passed allocated GPUs to it.
> 
> 
> 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: Passed allocated GPUs to 'devices' entry of 'docker::Flags'.

[Kevin Klues]

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




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.


- Kevin Klues


On Aug. 22, 2016, 10:12 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50599/
> ---
> 
> (Updated Aug. 22, 2016, 10:12 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
> ---
> 
> Added a new string entry 'devices' to 'docker::Flags' to
> indicate host devices exposed into docker container, and
> passed allocated GPUs to it.
> 
> 
> 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: Passed allocated GPUs to 'devices' entry of 'docker::Flags'.

[Kevin Klues]

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




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


I don't think this should just be a string. It should be an actual vector 
of type `Device`.

Also, the comment should read:
```
// Devices attached to the container.
```



src/slave/containerizer/docker.cpp (lines 685 - 719)


If you store `devices` as a vector of type `Device`, and you use the 
suggestion from the previous patch to add a static `parse()` function to 
`Device`, this whole code block becomes:

```
container->devices.push_back(
Device::parse("/dev/nvidiactl:/dev/nvidiactl:rwm"));

container->devices.push_back(
Device::parse("/dev/nvidia-uvm:/dev/nvidia-uvm:rwm"));

if (os::exists("/dev/nvidia-uvm-tools")) {
  container->devices.push_back(
  Device::parse("/dev/nvidia-uvm-tools:/dev/nvidia-uvm-tools:rwm"));
}
```

The join as a single comma-separated string should happen only when you are 
ready to add it to the flags.



src/slave/containerizer/docker.cpp (lines 685 - 686)


You also need to include `/dev/nvidia-uvm-tools` if it exists.


- Kevin Klues


On Aug. 22, 2016, 10:12 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50599/
> ---
> 
> (Updated Aug. 22, 2016, 10:12 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
> ---
> 
> Added a new string entry 'devices' to 'docker::Flags' to
> indicate host devices exposed into docker container, and
> passed allocated GPUs to it.
> 
> 
> 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: Passed allocated GPUs to 'devices' entry of 'docker::Flags'.

[Yubo Li]

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

(Updated 八月 22, 2016, 10:12 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
---

Added a new string entry 'devices' to 'docker::Flags' to
indicate host devices exposed into docker container, and
passed allocated GPUs to it.


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

Re: Review Request 50599: Passed allocated GPUs to 'devices' entry of 'docker::Flags'.

[Guangya Liu]

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




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


period to the end.

```
// Devices exposition control.
```



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


kill this



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


```
const string nvidiaData =
  nvidiaDataPrefix + boost::lexical_cast(gpu.minor);
```



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


period to the end


- Guangya Liu


On 八月 15, 2016, 7:29 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50599/
> ---
> 
> (Updated 八月 15, 2016, 7: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
> ---
> 
> Added a new string entry 'devices' to 'docker::Flags' to
> indicate host devices exposed into docker container, and
> passed allocated GPUs to it.
> 
> 
> 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: Passed allocated GPUs to 'devices' entry of 'docker::Flags'.

[Yubo Li]

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

(Updated 八月 15, 2016, 7:29 a.m.)


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


Changes
---

Added a new member varible `devices` to `container` and wrapped nvidia GPU 
related logic to `DockerContainerizerProcess::allocateNvidiaGpu()`


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


Repository: mesos


Description
---

Added a new string entry 'devices' to 'docker::Flags' to
indicate host devices exposed into docker container, and
passed allocated GPUs to it.


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

Re: Review Request 50599: Passed allocated GPUs to 'devices' entry of 'docker::Flags'.

[Yubo Li]

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

(Updated 八月 10, 2016, 10:33 a.m.)


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


Summary (updated)
-

Passed allocated GPUs to 'devices' entry of 'docker::Flags'.


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


Repository: mesos


Description (updated)
---

Added a new string entry 'devices' to 'docker::Flags' to
indicate host devices exposed into docker container, and
passed allocated GPUs to it.


Diffs (updated)
-

  src/slave/containerizer/docker.cpp 12bad2db03bcf755317c654f028b628c5c407a62 

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


Testing
---

make check


Thanks,

Yubo Li