mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jie Yu <yujie....@gmail.com>
Subject Re: Review Request 45360: Added volume driver client for mount and unmount.
Date Wed, 20 Apr 2016 19:03:20 GMT


> On April 19, 2016, 10:37 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/driver.hpp, line 41
> > <https://reviews.apache.org/r/45360/diff/8/?file=1346544#file1346544line41>
> >
> >     Do you want to use `const Option<string>& dvdcliPath` here? The idea
is that if not specified, the client will just use 'dvdcli' in $PATH. You want to use os::which
to do a check in 'create' to make sure it's there if dvdcliPath is not specified.
> 
> Guangya Liu wrote:
>     So you mean that we still need an agent flag to speicify the dvdcli path as an option?
> 
> Guangya Liu wrote:
>     Can we stick to `const std::string& dvdcliPath`? My thinking is that when `DockerVolumeIsolatorProcess::create`,
I can first get `dvdcliPath` from agent flag, if flag is not specified, get it form `$PATH`,
if no `dvdcliPath`, just return `Error` in `DockerVolumeIsolatorProcess::create`.

OK, makes sense.


> On April 19, 2016, 10:37 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/driver.hpp, line 58
> > <https://reviews.apache.org/r/45360/diff/8/?file=1346544#file1346544line58>
> >
> >     Ditto on removing 'const'.
> 
> Guangya Liu wrote:
>     Why remove the `const` here? I think that we do not expect the `name` be changed
when call `unmount`?

the tailing 'const' has nothing to do with function parameters. It's about object fields.


- Jie


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


On April 20, 2016, 5:44 p.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45360/
> -----------------------------------------------------------
> 
> (Updated April 20, 2016, 5:44 p.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added volume driver client for mount and unmount.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt e4923c4d4a7a9eefcab9267fe5def7f7535e49ed 
>   src/Makefile.am 5a2b4efa781752863d6751f98614fb78bece73ac 
>   src/slave/containerizer/mesos/isolators/docker/volume/driver.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45360/diff/
> 
> 
> Testing
> -------
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


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