----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51124/#review146732 -----------------------------------------------------------
src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (line 155) <https://reviews.apache.org/r/51124/#comment213356> s/temp dir/temporary directory/g src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (line 158) <https://reviews.apache.org/r/51124/#comment213351> Just style nits. newline above. src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (line 159) <https://reviews.apache.org/r/51124/#comment213352> kill this line. src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (line 161) <https://reviews.apache.org/r/51124/#comment213355> newline above. src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (line 165) <https://reviews.apache.org/r/51124/#comment213357> ditto. src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (line 167) <https://reviews.apache.org/r/51124/#comment213358> Newline above. s/tempLink/symlink/g src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (line 172) <https://reviews.apache.org/r/51124/#comment213359> Could you add comments for the index? so people know that the symlinks are named as 0, 1, ..., N. src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (line 174) <https://reviews.apache.org/r/51124/#comment213361> we can use stringify, but both look good to me. src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (line 262) <https://reviews.apache.org/r/51124/#comment213369> Could you address it here? We should `getBackendDir` in provisioner destroy and pass the backendDir to backend::destroy(). src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (line 267) <https://reviews.apache.org/r/51124/#comment213373> no space, please fix the indentation. s/migration/deprecation/g src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (line 273) <https://reviews.apache.org/r/51124/#comment213376> sounds a little weird. Maybe: ``` "Invalid symlink '" + tempLink + "'" ``` src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (line 275) <https://reviews.apache.org/r/51124/#comment213377> s/resolved/realpath/g src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (lines 285 - 286) <https://reviews.apache.org/r/51124/#comment213378> You should remove the symlink as well, right? - Gilbert Song On Aug. 15, 2016, 6:09 p.m., Zhitao Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51124/ > ----------------------------------------------------------- > > (Updated Aug. 15, 2016, 6:09 p.m.) > > > Review request for mesos, Xiaojian Huang, Gilbert Song, and Jie Yu. > > > Bugs: MESOS-6000 > https://issues.apache.org/jira/browse/MESOS-6000 > > > Repository: mesos > > > Description > ------- > > Support more layers through symlink for overlay backend. > > > Diffs > ----- > > src/slave/containerizer/mesos/provisioner/backends/overlay.hpp > 387f28a331813c75a509b4a31dbbdc764080b8c1 > src/slave/containerizer/mesos/provisioner/backends/overlay.cpp > e20cd48463a78ecd8a652a4d5ac923dc02cca4d9 > > Diff: https://reviews.apache.org/r/51124/diff/ > > > Testing > ------- > > 1. Make sure `OverlayBackendTest.*` passes; > 2. Use mesos-execute to provision a container using overlay backend. Observed > following log lines: > > ``` > I0816 01:04:27.823420 46584 overlay.cpp:167] Created tempLink at > '/var/lib/mesos/provisioner/containers/fd574bbf-4bc0-4538-9ce5-8c2cc93b94c7/backends/overlay/links' > to '/tmp/NcmRZt' > I0816 01:04:27.824834 46584 overlay.cpp:191] Provisioning image rootfs with > overlayfs: > 'lowerdir=/tmp/NcmRZt/42:/tmp/NcmRZt/41:/tmp/NcmRZt/40:/tmp/NcmRZt/39:/tmp/NcmRZt/38:/tmp/NcmRZt/37:/tmp/NcmRZt/36:/tmp/NcmRZt/35:/tmp/NcmRZt/34:/tmp/NcmRZt/33:/tmp/NcmRZt/32:/tmp/NcmRZt/31:/tmp/NcmRZt/30:/tmp/NcmRZt/29:/tmp/NcmRZt/28:/tmp/NcmRZt/27:/tmp/NcmRZt/26:/tmp/NcmRZt/25:/tmp/NcmRZt/24:/tmp/NcmRZt/23:/tmp/NcmRZt/22:/tmp/NcmRZt/21:/tmp/NcmRZt/20:/tmp/NcmRZt/19:/tmp/NcmRZt/18:/tmp/NcmRZt/17:/tmp/NcmRZt/16:/tmp/NcmRZt/15:/tmp/NcmRZt/14:/tmp/NcmRZt/13:/tmp/NcmRZt/12:/tmp/NcmRZt/11:/tmp/NcmRZt/10:/tmp/NcmRZt/9:/tmp/NcmRZt/8:/tmp/NcmRZt/7:/tmp/NcmRZt/6:/tmp/NcmRZt/5:/tmp/NcmRZt/4:/tmp/NcmRZt/3:/tmp/NcmRZt/2:/tmp/NcmRZt/1:/tmp/NcmRZt/0,upperdir=/var/lib/mesos/provisioner/containers/fd574bbf-4bc0-4538-9ce5-8c2cc93b94c7/backends/overlay/scratch/25d9b6e4-fd0e-401e-bb9e-754dbaec4f55/upperdir,workdir=/var/lib/mesos/provisioner/containers/fd574bbf-4bc0-4538-9ce5-8c2cc93b94c7/backends/overlay/scratch/2 5d9b6e4-fd0e-401e-bb9e-754dbaec4f55/workdir' > ... > (after executor exited) > I0816 01:04:34.859851 46584 overlay.cpp:281] Removed temporary directory > '/tmp/NcmRZt' pointed by > '/var/lib/mesos/provisioner/containers/fd574bbf-4bc0-4538-9ce5-8c2cc93b94c7/backends/overlay/links' > ``` > > > Thanks, > > Zhitao Li > >
