> 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 > >
