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 53115: Implemented handling AUFS whiteouts for copy backend.
Date Fri, 28 Oct 2016 02:03:12 GMT

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




src/slave/containerizer/mesos/provisioner/backends/copy.cpp (line 131)
<https://reviews.apache.org/r/53115/#comment223590>

    I am not sure if we need to pass in 'image' to backend. If we added OCI support, do we
need to add another check here?
    
    I think we can simply assume the layer's whiteout is in aufs whiteout format, no matter
what image type it is. I think APPC, we probably needs to do the same, right?
    
    You probably add a NOTE there saying that we assume all image type uses aufs whiteout
format.
    
    In that way, no need to pass 'image' around to backend.



src/slave/containerizer/mesos/provisioner/backends/copy.cpp (line 150)
<https://reviews.apache.org/r/53115/#comment223593>

    not yours, but can you fix the indentation here.



src/slave/containerizer/mesos/provisioner/backends/copy.cpp (line 211)
<https://reviews.apache.org/r/53115/#comment223594>

    Hum, IIUC, looks like this is not entirely correct. According to this:
    https://github.com/opencontainers/image-spec/blob/master/layer.md#whiteouts
    
    "Files that are present in the same layer as a whiteout file can only be hidden by whiteout
files in subsequent layers."
    
    So we cannot cp first and then process whiteouts. Looks like before cp each layer, we
need to do a pre-processing to handle whiteouts first (and remove the whiteouts), and then
do the cp.



src/slave/containerizer/mesos/provisioner/backends/copy.cpp (lines 215 - 218)
<https://reviews.apache.org/r/53115/#comment223591>

    Ditto on doing this in a less nesting way.



src/slave/containerizer/mesos/provisioner/backends/copy.cpp (line 218)
<https://reviews.apache.org/r/53115/#comment223592>

    Ditto. string is not needed here.


- Jie Yu


On Oct. 25, 2016, 3:47 a.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53115/
> -----------------------------------------------------------
> 
> (Updated Oct. 25, 2016, 3:47 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6360
>     https://issues.apache.org/jira/browse/MESOS-6360
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented handling AUFS whiteouts for copy backend.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/provisioner/backend.hpp 7257d3a962ecdf87fe9d52facbd6a2619311a018

>   src/slave/containerizer/mesos/provisioner/backends/aufs.hpp a3c924195ae5eecc1caca9cd6fc0f6dc0df0a741

>   src/slave/containerizer/mesos/provisioner/backends/aufs.cpp c92c6c7174421158b9190ed0bfb00c1c97ef0f7b

>   src/slave/containerizer/mesos/provisioner/backends/bind.hpp 54c88d940aa64d13114fc5d79ecbc1d474d169a6

>   src/slave/containerizer/mesos/provisioner/backends/bind.cpp 94dac40a12b6fd2e7d9733444d84763c77785402

>   src/slave/containerizer/mesos/provisioner/backends/copy.hpp 131d75521ca38afae651e8d885ebedad77d86a3e

>   src/slave/containerizer/mesos/provisioner/backends/copy.cpp 9c5354e5fea488618ebdecf0aef9dd2fce555d20

>   src/slave/containerizer/mesos/provisioner/backends/overlay.hpp 4d591c5f988d87e0e905f973df5ab15a3386d676

>   src/slave/containerizer/mesos/provisioner/backends/overlay.cpp b71a31323aef376c9a28e1d52322a1802fb212ad

>   src/slave/containerizer/mesos/provisioner/provisioner.cpp de2c12140652244bd3de9763ced203b144688ab2

>   src/tests/containerizer/provisioner_backend_tests.cpp 727bf645dd9337a94f098ab0a816b7e0e0651083

> 
> Diff: https://reviews.apache.org/r/53115/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


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