> On 四月 19, 2016, 11:40 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp, lines 
> > 108-109
> > <https://reviews.apache.org/r/45673/diff/3/?file=1350676#file1350676line108>
> >
> >     To me, this is just an optimization, isn't it? We can still go through 
> > the 'infos' list to see a volume is still begin used, right?
> >     
> >     Can we not introduce this optimizaiton yet?

Yes, as the reference will only be used in `cleanup`, I will move the logic to 
`cleanup`.


> On 四月 19, 2016, 11:40 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp, line 337
> > <https://reviews.apache.org/r/45673/diff/3/?file=1350677#file1350677line337>
> >
> >     We need to serialize calls to 'client' for a given driver and a given 
> > volume name. You can use `Sequence` for each driver+name pair in 
> > DockerVolumeDriverClient to achieve that.

Yes, that's why I was using `mountInfo` in `DockerVolumeClient` `mount` API, as 
I want to keep the relationship between container path and mount point. ok, 
will try to see how `Sequence` can help.


> On 四月 19, 2016, 11:40 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/state.hpp, line 34
> > <https://reviews.apache.org/r/45673/diff/3/?file=1350680#file1350680line34>
> >
> >     I suggest that we only put informations that we can recover in 'Info' 
> > struct. For things like container_path, mount_point that we cannot recover, 
> > try not to put them in 'Info' struct. You can always create a parallel 
> > vector with those additional information, and pass down using parameters in 
> > the lambda.

yes, I was putting it here is because I want to make this shared between 
`DockerVolumeClient` and `Isolator`, but if I use `Sequence`, then there will 
be no need to keep this but just move this to `isolator.hpp`


- Guangya


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


On 四月 19, 2016, 4:45 p.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45673/
> -----------------------------------------------------------
> 
> (Updated 四月 19, 2016, 4:45 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> PoC: Docker Volume Isolator.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt aabb33dfd4b985fe5774f87bb63b4ec1ec853962 
>   src/Makefile.am 5a2b4efa781752863d6751f98614fb78bece73ac 
>   src/cli/execute.cpp 087a73143d739f41638b8335c7e24dfcd14bc7fb 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1e1a36903f4377497bb72b69e4ead63675d453c0 
>   src/slave/containerizer/mesos/isolators/docker/volume/driver.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> 786f917c0ec04b6839bbd524fa7c8de3729f9bdb 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 0ad473dc3bd45e122fba55a670e1a893e61c977a 
>   src/slave/containerizer/mesos/isolators/docker/volume/paths.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/volume/paths.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/volume/state.hpp 
> PRE-CREATION 
>   src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553 
> 
> Diff: https://reviews.apache.org/r/45673/diff/
> 
> 
> Testing
> -------
> 
> state.proto
> 
> message DockerVolumeMount {
>   required string volume_driver = 1;
>   required string volume_name = 2;
> }
> 
> 
> message DockerVolumeMountList {
>   repeated DockerVolumeMount mounts = 1;
> }
> 
> 
> message DockerVolumeInfo {
>   // Docker volume mount info for volume driver and volume name.
>   required DockerVolumeMount dvm = 1;
> 
>   // Container path, this is optional as this is not needed by recover()
>   // but only for prepare() .
>   optional string container_path = 2;
> 
>   // Mount point for the container, this is optiona as this is only needed
>   // by prepare().
>   optional string mount_point = 3;
>   
>   // Volume driver mount options.
>   optional Parameters driver_options = 4;
> }
> 
> DockerVolumeMount will be checkpointed.
> 
> 
> //   /var/run/mesos/isolators/docker/volume
> //    |- <ID of Container1>/
> //    |  |-- DockerVolumeMountList
> //    |-- <ID of Container2>/
> //    |  |-- DockerVolumeMountList
> //    |-<ID of Container3>/
> //    |- ...
> 
> Info structure
> We assume that one container do not use multiple same volume driver and 
> volume name pair.
> struct Info
> {
>   Info (const list<DockerVolumeInfo>& _volumeInfos)
>     : volumeInfos(_volumeInfos) {}
> 
>   list<DockerVolumeInfo> volumeInfos;
> };
> 
> Hashmap in isolator:
> hashmap<ContainerID, process::Owned<Info>> infos;
> hashmap<string, int> volumeReference; // string is 
> volume_driver::volume_name, e.g. convoy:dvd1
> 
> Cleanup()
> When cleanup, check the container’s mount point reference counter, if it is 
> greater than 1, do not remove the mount point; If it is 1, unmount the mount 
> point.
> driver.cpp::mount
> We need to transfer the whole mount info to mount() as parameter to make sure 
> that the container path can get its own related mount point. If the mount() 
> only return a string, then we cannot get the relationship of container path 
> and mount point.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Reply via email to