mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Guangya Liu <gyliu...@gmail.com>
Subject Re: Review Request 50599: Added a new 'device' entry to 'docker::Flags'.
Date Mon, 01 Aug 2016 10:12:44 GMT

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




src/slave/containerizer/docker.cpp (line 229)
<https://reviews.apache.org/r/50599/#comment210328>

    You are losing the logic of handling the `device` here: 
    
    ```
    // Exposed devices to this docker container.
    dockerFlags.device = device;
    ```
    
    I'd prefer that you add the `device` flags first before implementing `dockerFlags`.



src/slave/containerizer/docker.cpp (lines 1353 - 1354)
<https://reviews.apache.org/r/50599/#comment210329>

    ```
    const Docker::Device nvidiaCtlDevice = Docker::Device(
        nvidiaCtl, nvidiaCtl, permission);
    ```



src/slave/containerizer/docker.cpp (lines 1354 - 1355)
<https://reviews.apache.org/r/50599/#comment210330>

    new line here



src/slave/containerizer/docker.cpp (lines 1357 - 1358)
<https://reviews.apache.org/r/50599/#comment210331>

    ```
    const Docker::Device nvidiaUvmDevice = Docker::Device(
        nvidiaUvm, nvidiaUvm, permission);
    ```



src/slave/containerizer/docker.cpp (lines 1358 - 1359)
<https://reviews.apache.org/r/50599/#comment210332>

    new line here



src/slave/containerizer/docker.cpp (lines 1362 - 1366)
<https://reviews.apache.org/r/50599/#comment210333>

    ```
    const string nvidiaData = nvidiaDataPrefix +
      boost::lexical_cast<string>(gpu.minor);
    
    Docker::Device nvidiaDataDevice = Docker::Device(
        nvidiaData, nvidiaData, permission);
    
    argv.push_back(nvidiaDataDevice.serialize());
    ```



src/slave/containerizer/docker.cpp (line 1371)
<https://reviews.apache.org/r/50599/#comment210338>

    I'd prefer that you show an example here as the comment for the `nvidiaGpuDevices`.


- Guangya Liu


On 七月 29, 2016, 9:54 a.m., Yubo Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50599/
> -----------------------------------------------------------
> 
> (Updated 七月 29, 2016, 9:54 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 'device' to 'docker::Flags' to
> indicate host devices exposed into docker container, and
> passed allocated GPUs to it.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/docker.cpp 12bad2db03bcf755317c654f028b628c5c407a62 
> 
> Diff: https://reviews.apache.org/r/50599/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message