Re: Review Request 50125: Added mesos-docker-executor support for device control.

[Guangya Liu]

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




src/docker/executor.hpp (lines 78 - 79)


what about rename it as `devices`?

Ditto for the following.



src/docker/executor.cpp (line 163)


s/vector deviceList/const vector deviceList



src/docker/executor.cpp (line 164)


s/dev/device, we prefer a full name here for easy to understand.



src/docker/executor.cpp (lines 170 - 171)


kill this, the `parse` already logged the message.


- Guangya Liu


On 八月 5, 2016, 9:52 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50125/
> ---
> 
> (Updated 八月 5, 2016, 9:52 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a new flag '--device' to mesos-docker-executor, and gave its
> feature to control device exposition, isolation, and access permission.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.hpp 7b63d784d6b8685912598b77fb38cf6e70646ae3 
>   src/docker/executor.cpp 445628c9164facdd8bd812c5b45e3b2b886ebf0e 
>   src/slave/containerizer/docker.cpp 12bad2db03bcf755317c654f028b628c5c407a62 
> 
> Diff: https://reviews.apache.org/r/50125/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 50125: Added mesos-docker-executor support for device control.

[Yubo Li]

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

(Updated 八月 5, 2016, 9:52 a.m.)


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


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


Repository: mesos


Description
---

Added a new flag '--device' to mesos-docker-executor, and gave its
feature to control device exposition, isolation, and access permission.


Diffs (updated)
-

  src/docker/executor.hpp 7b63d784d6b8685912598b77fb38cf6e70646ae3 
  src/docker/executor.cpp 445628c9164facdd8bd812c5b45e3b2b886ebf0e 
  src/slave/containerizer/docker.cpp 12bad2db03bcf755317c654f028b628c5c407a62 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50125: Added mesos-docker-executor support for device control.

[Guangya Liu]

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



Another comment is that I found it is difficult to review those patches in such 
an order, can you please reverse the order of your patch chain? Implement 
overall logic first, then detail the logic in following patch chain?


src/docker/executor.hpp (line 81)


s/--device/`--device`



src/docker/executor.hpp (line 82)



s/'PathInHost:PathInContainer:Permission'/`PathInHost:PathInContainer:Permission`



src/docker/executor.hpp (line 83)


s/'/dev/tty:/dev/tty:mrw' for '/dev/tty' fully 
exposed./`/dev/tty:/dev/tty:mrw` for `/dev/tty` fully exposed.



src/docker/executor.cpp (line 170)


remove `` at the end



src/docker/executor.cpp (line 171)


remove period at the end



src/docker/executor.cpp (line 827)


the default value of `string device` is already empty, no need to 
initialize it.


- Guangya Liu


On 八月 2, 2016, 2:21 p.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50125/
> ---
> 
> (Updated 八月 2, 2016, 2:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a new flag '--device' to mesos-docker-executor, and gave its
> feature to control device exposition, isolation, and access permission.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.hpp 7b63d784d6b8685912598b77fb38cf6e70646ae3 
>   src/docker/executor.cpp 445628c9164facdd8bd812c5b45e3b2b886ebf0e 
> 
> Diff: https://reviews.apache.org/r/50125/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 50125: Added mesos-docker-executor support for device control.

[Yubo Li]

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

(Updated 八月 2, 2016, 2:21 p.m.)


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


Changes
---

Changed commit order and commit message


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


Repository: mesos


Description (updated)
---

Added a new flag '--device' to mesos-docker-executor, and gave its
feature to control device exposition, isolation, and access permission.


Diffs (updated)
-

  src/docker/executor.hpp 7b63d784d6b8685912598b77fb38cf6e70646ae3 
  src/docker/executor.cpp 445628c9164facdd8bd812c5b45e3b2b886ebf0e 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50125: Added mesos-docker-executor support for device control.

[Yubo Li]

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

(Updated 七月 29, 2016, 9:56 a.m.)


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


Summary (updated)
-

Added mesos-docker-executor support for device control.


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


Repository: mesos


Description (updated)
---

Added a new flag '--device' to mesos-docker-executor to control device
exposition, isolation, and access permission.


Diffs (updated)
-

  src/docker/executor.hpp 7b63d784d6b8685912598b77fb38cf6e70646ae3 
  src/docker/executor.cpp 445628c9164facdd8bd812c5b45e3b2b886ebf0e 
  src/slave/containerizer/docker.cpp 12bad2db03bcf755317c654f028b628c5c407a62 

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


Testing
---

make check


Thanks,

Yubo Li