Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Yubo Li]

> On 十一月 12, 2016, 12:14 p.m., Guangya Liu wrote:
> > We do not have `external containerizer` now, so you can remove it from the 
> > `Descripiton`.
> > 
> > Summary
> > ```
> > Only check `gpu/nvidia` isolator for mesos containerizer.
> > ```
> > 
> > Description
> > ```
> > Mesos containerizer uses isolator `gpu/nvidia` for GPU isolation while
> > docker containerizers do not need it. This patch is making the isolator
> > `gpu/nvidia` check only available for mesos containerizer but not docker
> > containerizer in GPU allocator.
> > ```

fixed. thanks


- Yubo


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


On 十一月 10, 2016, 8:13 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50947/
> ---
> 
> (Updated 十一月 10, 2016, 8:13 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5975
> https://issues.apache.org/jira/browse/MESOS-5975
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
> docker and external containerizers do not need it anymore. This removed
> the 'gpu/nvidia' isolator check for docker and external containerizers.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
> 2e722691475c84afae14009014ea70cc0fdd0e65 
> 
> Diff: https://reviews.apache.org/r/50947/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Guangya Liu]

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



We do not have `external containerizer` now, so you can remove it from the 
`Descripiton`.

Summary
```
Only check `gpu/nvidia` isolator for mesos containerizer.
```

Description
```
Mesos containerizer uses isolator `gpu/nvidia` for GPU isolation while
docker containerizers do not need it. This patch is making the isolator
`gpu/nvidia` check only available for mesos containerizer but not docker
containerizer in GPU allocator.
```

- Guangya Liu


On 十一月 10, 2016, 8:13 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50947/
> ---
> 
> (Updated 十一月 10, 2016, 8:13 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5975
> https://issues.apache.org/jira/browse/MESOS-5975
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
> docker and external containerizers do not need it anymore. This removed
> the 'gpu/nvidia' isolator check for docker and external containerizers.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
> 2e722691475c84afae14009014ea70cc0fdd0e65 
> 
> Diff: https://reviews.apache.org/r/50947/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Yubo Li]

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

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


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


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


Repository: mesos


Description
---

Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
docker and external containerizers do not need it anymore. This removed
the 'gpu/nvidia' isolator check for docker and external containerizers.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
2e722691475c84afae14009014ea70cc0fdd0e65 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [50947]

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

- Mesos ReviewBot


On Nov. 7, 2016, 1:51 p.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50947/
> ---
> 
> (Updated Nov. 7, 2016, 1:51 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5975
> https://issues.apache.org/jira/browse/MESOS-5975
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
> docker and external containerizers do not need it anymore. This removed
> the 'gpu/nvidia' isolator check for docker and external containerizers.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
> 2e722691475c84afae14009014ea70cc0fdd0e65 
> 
> Diff: https://reviews.apache.org/r/50947/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Yubo Li]

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

(Updated 十一月 7, 2016, 1:51 p.m.)


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


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


Repository: mesos


Description
---

Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
docker and external containerizers do not need it anymore. This removed
the 'gpu/nvidia' isolator check for docker and external containerizers.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
2e722691475c84afae14009014ea70cc0fdd0e65 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Yubo Li]

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

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


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


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


Repository: mesos


Description
---

Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
docker and external containerizers do not need it anymore. This removed
the 'gpu/nvidia' isolator check for docker and external containerizers.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
2e722691475c84afae14009014ea70cc0fdd0e65 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Yubo Li]

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

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


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


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


Repository: mesos


Description
---

Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
docker and external containerizers do not need it anymore. This removed
the 'gpu/nvidia' isolator check for docker and external containerizers.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
2e722691475c84afae14009014ea70cc0fdd0e65 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Yubo Li]

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

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


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


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


Repository: mesos


Description
---

Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
docker and external containerizers do not need it anymore. This removed
the 'gpu/nvidia' isolator check for docker and external containerizers.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
2e722691475c84afae14009014ea70cc0fdd0e65 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Yubo Li]

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

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


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


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


Repository: mesos


Description
---

Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
docker and external containerizers do not need it anymore. This removed
the 'gpu/nvidia' isolator check for docker and external containerizers.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
2e722691475c84afae14009014ea70cc0fdd0e65 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Kevin Klues]

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




src/slave/containerizer/mesos/isolators/gpu/allocator.cpp (lines 149 - 151)


Can we cahge the order of these conditionals:

i.e. change:
```
if (strings::contains(flags.containerizers, "mesos") &&
  flags.nvidia_gpu_devices.isSome() &&
  isolators.count("gpu/nvidia") == 0) {
```
to:
```
if (strings::contains(flags.containerizers, "mesos") &&
  isolators.count("gpu/nvidia") == 0 &&
  flags.nvidia_gpu_devices.isSome()) {
```


- Kevin Klues


On Oct. 24, 2016, 5:07 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50947/
> ---
> 
> (Updated Oct. 24, 2016, 5:07 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5975
> https://issues.apache.org/jira/browse/MESOS-5975
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
> docker and external containerizers do not need it anymore. This removed
> the 'gpu/nvidia' isolator check for docker and external containerizers.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
> 2e722691475c84afae14009014ea70cc0fdd0e65 
> 
> Diff: https://reviews.apache.org/r/50947/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Yubo Li]

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

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


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


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


Repository: mesos


Description
---

Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
docker and external containerizers do not need it anymore. This removed
the 'gpu/nvidia' isolator check for docker and external containerizers.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
2e722691475c84afae14009014ea70cc0fdd0e65 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Yubo Li]

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

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


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


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


Repository: mesos


Description
---

Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
docker and external containerizers do not need it anymore. This removed
the 'gpu/nvidia' isolator check for docker and external containerizers.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
2e722691475c84afae14009014ea70cc0fdd0e65 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Yubo Li]

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

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


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


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


Repository: mesos


Description
---

Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
docker and external containerizers do not need it anymore. This removed
the 'gpu/nvidia' isolator check for docker and external containerizers.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
2e722691475c84afae14009014ea70cc0fdd0e65 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Guangya Liu]

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




src/slave/containerizer/mesos/isolators/gpu/allocator.cpp (lines 170 - 174)


Here should also check if using `mesos` containerizer, otherwise, the 
`docker` containerizer will always return here and there may be GPU overcommit 
if we specify more GPU resources than the actual GPU resources.

```
// Pass the GPU resources through if we're not going to do any
// isolation for mesos conainerizer or we cannot validate the
// resources using NVML.
if ((strings::contains(flags.containerizers, "mesos") &&
 isolators.count("gpu/nvidia") == 0) ||
!nvml::isAvailable()) {
  return resources;
}
```


- Guangya Liu


On 十月 20, 2016, 10:01 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50947/
> ---
> 
> (Updated 十月 20, 2016, 10:01 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5975
> https://issues.apache.org/jira/browse/MESOS-5975
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
> docker and external containerizers do not need it anymore. This removed
> the 'gpu/nvidia' isolator check for docker and external containerizers.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
> 2e722691475c84afae14009014ea70cc0fdd0e65 
> 
> Diff: https://reviews.apache.org/r/50947/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Yubo Li]

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

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


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


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


Repository: mesos


Description
---

Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
docker and external containerizers do not need it anymore. This removed
the 'gpu/nvidia' isolator check for docker and external containerizers.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
2e722691475c84afae14009014ea70cc0fdd0e65 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Yubo Li]

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

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


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


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


Repository: mesos


Description
---

Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
docker and external containerizers do not need it anymore. This removed
the 'gpu/nvidia' isolator check for docker and external containerizers.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
2e722691475c84afae14009014ea70cc0fdd0e65 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Yubo Li]

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

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


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


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


Repository: mesos


Description
---

Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
docker and external containerizers do not need it anymore. This removed
the 'gpu/nvidia' isolator check for docker and external containerizers.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
2e722691475c84afae14009014ea70cc0fdd0e65 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Guangya Liu]

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




src/slave/containerizer/mesos/isolators/gpu/allocator.cpp (line 146)


s/--nvidia-gpu_devices/--nvidia_gpu_devices


- Guangya Liu


On 九月 22, 2016, 6:21 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50947/
> ---
> 
> (Updated 九月 22, 2016, 6:21 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5975
> https://issues.apache.org/jira/browse/MESOS-5975
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
> docker and external containerizers do not need it anymore. This removed
> the 'gpu/nvidia' isolator check for docker and external containerizers.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
> c1a87e9e5c07529bc1d077f68477108a40506806 
> 
> Diff: https://reviews.apache.org/r/50947/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Yubo Li]

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

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


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


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


Repository: mesos


Description
---

Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
docker and external containerizers do not need it anymore. This removed
the 'gpu/nvidia' isolator check for docker and external containerizers.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
c1a87e9e5c07529bc1d077f68477108a40506806 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Yubo Li]

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

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


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


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


Repository: mesos


Description
---

Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
docker and external containerizers do not need it anymore. This removed
the 'gpu/nvidia' isolator check for docker and external containerizers.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
c1a87e9e5c07529bc1d077f68477108a40506806 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Yubo Li]

> On 八月 25, 2016, 7:17 p.m., Kevin Klues wrote:
> > src/slave/containerizer/mesos/isolators/gpu/allocator.cpp, lines 146-149
> > 
> >
> > This comment is confusing. I'd probably just leave it as it was, but 
> > say that this is only required for the mesos containerizer, i.e.:
> > 
> > `` Don't allow the `--nvidia_gpu_devices` flag without the GPU isolator 
> > when using the mesos containerizer. For the docker containerizers, this is 
> > not required.```

Fixed comments as you suggested.


- Yubo


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


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/50947/
> ---
> 
> (Updated 八月 15, 2016, 7:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5975
> https://issues.apache.org/jira/browse/MESOS-5975
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
> docker and external containerizers do not need it anymore. This removed
> the 'gpu/nvidia' isolator check for docker and external containerizers.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
> c1a87e9e5c07529bc1d077f68477108a40506806 
> 
> Diff: https://reviews.apache.org/r/50947/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Kevin Klues]

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




src/slave/containerizer/mesos/isolators/gpu/allocator.cpp (lines 146 - 149)


This comment is confusing. I'd probably just leave it as it was, but say 
that this is only required for the mesos containerizer, i.e.:

`` Don't allow the `--nvidia_gpu_devices` flag without the GPU isolator 
when using the mesos containerizer. For the docker containerizers, this is not 
required.```


- Kevin Klues


On Aug. 15, 2016, 7:29 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50947/
> ---
> 
> (Updated Aug. 15, 2016, 7:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5975
> https://issues.apache.org/jira/browse/MESOS-5975
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
> docker and external containerizers do not need it anymore. This removed
> the 'gpu/nvidia' isolator check for docker and external containerizers.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
> c1a87e9e5c07529bc1d077f68477108a40506806 
> 
> Diff: https://reviews.apache.org/r/50947/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Yubo Li]

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

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


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


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


Repository: mesos


Description
---

Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
docker and external containerizers do not need it anymore. This removed
the 'gpu/nvidia' isolator check for docker and external containerizers.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
c1a87e9e5c07529bc1d077f68477108a40506806 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Yubo Li]

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

(Updated 八月 11, 2016, 9:09 a.m.)


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


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


Repository: mesos


Description
---

Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
docker and external containerizers do not need it anymore. This removed
the 'gpu/nvidia' isolator check for docker and external containerizers.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
c1a87e9e5c07529bc1d077f68477108a40506806 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Guangya Liu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/gpu/allocator.cpp (lines 146 - 149)


```
// Mesos containerizer needs to enable isolator flag "gpu/nvidia" to use
// GPUs, we don't allow the `--nvidia-gpu_devices` flag without the GPU
// isolator; For docker containerizers, this limitation is not set.
```



src/slave/containerizer/mesos/isolators/gpu/allocator.cpp (line 147)


s/For docker and external containerizers/For docker containerizers

The external containerizer was already removed.


- Guangya Liu


On 八月 11, 2016, 8:38 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50947/
> ---
> 
> (Updated 八月 11, 2016, 8:38 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5975
> https://issues.apache.org/jira/browse/MESOS-5975
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
> docker and external containerizers do not need it anymore. This removed
> the 'gpu/nvidia' isolator check for docker and external containerizers.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
> c1a87e9e5c07529bc1d077f68477108a40506806 
> 
> Diff: https://reviews.apache.org/r/50947/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Yubo Li]

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

(Updated 八月 11, 2016, 8:38 a.m.)


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


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


Repository: mesos


Description
---

Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
docker and external containerizers do not need it anymore. This removed
the 'gpu/nvidia' isolator check for docker and external containerizers.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
c1a87e9e5c07529bc1d077f68477108a40506806 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Guangya Liu]

> On 八月 10, 2016, 3:08 p.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/isolators/gpu/allocator.cpp, line 153
> > 
> >
> > What about update the message here as well by highlighting `mesos 
> > containerizer`?
> > 
> > Such as 
> > 
> > ```
> > return Error("'--nvidia_gpus_devices' can only be specified if the "
> >  "`--isolation` flag contains 'gpu/nvidia' with `mesos` "
> >  "containerizer");
> > ```

Correct a typo here: I prefer that we use `'` but not ``` for the error message.

```
return Error("'--nvidia_gpus_devices' can only be specified if the "
 "'--isolation' flag contains 'gpu/nvidia' with 'mesos' "
 "containerizer");
```


- Guangya


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


On 八月 10, 2016, 10:33 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50947/
> ---
> 
> (Updated 八月 10, 2016, 10:33 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5975
> https://issues.apache.org/jira/browse/MESOS-5975
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
> docker and external containerizers do not need it anymore. This removed
> the 'gpu/nvidia' isolator check for docker and external containerizers.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
> c1a87e9e5c07529bc1d077f68477108a40506806 
> 
> Diff: https://reviews.apache.org/r/50947/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Guangya Liu]

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




src/slave/containerizer/mesos/isolators/gpu/allocator.cpp (line 147)


The `external containerizers` was already removed in 
https://reviews.apache.org/r/50914/



src/slave/containerizer/mesos/isolators/gpu/allocator.cpp (lines 148 - 155)


What about 

```
// Don't allow the `--nvidia-gpu_devices` flag without the GPU isolator
// when using `mesos` containerizer.
if (strings::contains(flags.containerizers, "mesos") &&
flags.nvidia_gpu_devices.isSome() &&
isolators.count("gpu/nvidia") == 0) {
  return Error("'--nvidia_gpus_devices' can only be specified if the"
   " `--isolation` flag contains 'gpu/nvidia'");
}
```



src/slave/containerizer/mesos/isolators/gpu/allocator.cpp (line 152)


Not yours, but we prefer adding the space in the first line, which is 

```
return Error("'--nvidia_gpus_devices' can only be specified if the "
```



src/slave/containerizer/mesos/isolators/gpu/allocator.cpp (line 153)


What about update the message here as well by highlighting `mesos 
containerizer`?

Such as 

```
return Error("'--nvidia_gpus_devices' can only be specified if the "
 "`--isolation` flag contains 'gpu/nvidia' with `mesos` "
 "containerizer");
```


- Guangya Liu


On 八月 10, 2016, 10:33 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50947/
> ---
> 
> (Updated 八月 10, 2016, 10:33 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5975
> https://issues.apache.org/jira/browse/MESOS-5975
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
> docker and external containerizers do not need it anymore. This removed
> the 'gpu/nvidia' isolator check for docker and external containerizers.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
> c1a87e9e5c07529bc1d077f68477108a40506806 
> 
> Diff: https://reviews.apache.org/r/50947/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Yubo Li]

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

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


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


Repository: mesos


Description
---

Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
docker and external containerizers do not need it anymore. This removed
the 'gpu/nvidia' isolator check for docker and external containerizers.


Diffs
-

  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
c1a87e9e5c07529bc1d077f68477108a40506806 

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


Testing
---

make check


Thanks,

Yubo Li