mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilbert Song <songzihao1...@gmail.com>
Subject Re: Review Request 45673: PoC: Docker Volume Isolator.
Date Tue, 19 Apr 2016 00:02:23 GMT

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




src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (lines 315 - 322)
<https://reviews.apache.org/r/45673/#comment192854>

    Why `xxxx`?



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 350)
<https://reviews.apache.org/r/45673/#comment192863>

    Where do you set the mount point?


- Gilbert Song


On April 17, 2016, 9:10 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45673/
> -----------------------------------------------------------
> 
> (Updated April 17, 2016, 9:10 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> PoC: Docker Volumt Isolator.
> 
> Will split this to several patches after review discussion.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 06f58c4a88e3c527df727df1efe11ed3ab77efa8 
>   src/Makefile.am ec855263d620e4723c8ba9cd056c40a3a2e9ca99 
>   src/cli/execute.cpp f70d9e1c4badb7d4342e90ce4d4f8114f27a7dff 
>   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/containerizer/mesos/isolators/docker/volume/state.proto PRE-CREATION 
>   src/slave/slave.cpp de99e9eb5cc812b2e07deb749b98b4f4db363728 
> 
> Diff: https://reviews.apache.org/r/45673/diff/
> 
> 
> Testing
> -------
> 
> Some issues want to discuss:
> 1) One container can have multiple volumes, and the prepare() will generate all of the
mount point for all of the volumes in one container. Each volume driver and volume name need
to map to one mount point, so in driver.cpp::mount(), if only adding volume_driver, volume_name,
options as paramter, then once the mount point was generated, in prepare(), we will not able
to  know the relationship of volume_driver, volume_name pair and mount point. So it is better
transfer a structure including volume_driver, volume_name and mount point to driver.cpp::mount()
so that we can keep the relationship of  volume_driver, volume_name and mount point
> 2) Reference counter when cleanup. My thinking is that we can use volume driver and volume
name generate an volume ID, if one volume driver and volume name pair was used by more than
one container, then the reference couter of the volume ID will be greater than 1 and we should
not unmounts for such case, the unmount was only called when the volume ID reference count
was 1.
> 3) I was using “multihasmap” for infos in volume isolator, the reason is because
one container can have multiple volumes. If do not want to use “multihashmap”, then I
may need to update Info structure as:
> 
> Info (const hashmap<VolumeMountId, tuple<DockerVolumeMount, Option<string>
mountPoint>>& _dockerVolumeMountInfo
>       const Option<std::string>& _containerPath = None(),
>       const Option<hashmap<std::string, std::string>>& _driverOptions
= None())
>     : dockerVolumeMountInfo (_dockerVolumeMountInfo),
>       containerPath(_containerPath),
>       driverOptions(_driverOptions){}
> 
>   // The driver.cpp::mount() will fill in the value of mountPoint for each DockerVolumeMount
(driver and name)
>   hashmap<VolumeMountId, tuple<DockerVolumeMount, Option<string> mountPoint>>
dockerVolumeMountInfo;
> 
>   Option<std::string> containerPath;
> 
>   Option<hashmap<std::string, std::string>> driverOptions;
> };
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


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