mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From haosdent huang <haosd...@gmail.com>
Subject Re: Review Request 45370: Implemented prepare() for dvd isolator.
Date Tue, 05 Apr 2016 17:47:27 GMT

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




src/slave/containerizer/mesos/isolators/docker/dvd/dvd.hpp (line 21)
<https://reviews.apache.org/r/45370/#comment190281>

    Why need `<tuple>` and `<subprocess.hpp>` here?



src/slave/containerizer/mesos/isolators/docker/dvd/dvd.hpp (line 74)
<https://reviews.apache.org/r/45370/#comment190279>

    A bit confusing why add `using` here? Because of line limit? How about
    ```
      multihashmap<ContainerID, process::Owned<dvd::spec::ExternalMount>>
        containermountmap infos;
    ```



src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp (line 62)
<https://reviews.apache.org/r/45370/#comment190283>

    How about use `which` when it avaiable? https://issues.apache.org/jira/browse/MESOS-4576



src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp (line 98)
<https://reviews.apache.org/r/45370/#comment190285>

    `vector`? And add `using std::vector` above? And I think `vector<ExternalMount>`
without `spec` would be better



src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp (line 110)
<https://reviews.apache.org/r/45370/#comment190288>

    ```
        foreach (const string& driverOption,
                 volume.source().docker_volume().driver_options()) {
          driverOptions += " --volumeopts=" + driverOption;
        }
    ```



src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp (line 133)
<https://reviews.apache.org/r/45370/#comment190300>

    Add a line above.



src/slave/containerizer/mesos/isolators/docker/dvd/spec.hpp (line 29)
<https://reviews.apache.org/r/45370/#comment190293>

    I think a method which accept these parameters are enough? This way looks more like Java.


- haosdent huang


On April 4, 2016, 9:21 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45370/
> -----------------------------------------------------------
> 
> (Updated April 4, 2016, 9:21 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5013
>     https://issues.apache.org/jira/browse/MESOS-5013
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented prepare() for dvd isolator.
> 
> 
> Diffs
> -----
> 
>   src/cli/execute.cpp af62f41622e1c43acd8f257c54f8754162c433b8 
>   src/slave/containerizer/mesos/isolators/docker/dvd/dvd.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/dvd/spec.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45370/diff/
> 
> 
> Testing
> -------
> 
> The execute.cpp is mainly for test, will remove this file once test finished.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


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