mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Guangya Liu <gyliu...@gmail.com>
Subject Re: Review Request 47396: Added aufs provisioning backend.
Date Mon, 13 Jun 2016 09:35:29 GMT


> On 六月 7, 2016, 2:09 a.m., Guangya Liu wrote:
> > src/tests/environment.cpp, lines 492-495
> > <https://reviews.apache.org/r/47396/diff/4/?file=1407798#file1407798line492>
> >
> >     I think that here also needs to be updated to use your new function `fs::aufs::supported()`
to check `aufs`?
> 
> Jie Yu wrote:
>     Hum... this looks weird to me. I am wondering if we can make `fs::supported` understand
the sutble details related to overlayfs? Can you guys follow up with a patch. Basically, here,
it should be `Try<bool> check = fs::supported(fsname);` and inside `fs::supported`,
we check special case for overlayfs.
> 
> Guangya Liu wrote:
>     I'm now thinking that we may not need the API of `fs::aufs::supported`, the reason
that we introduced `fs::overlay::supported` is becuase of keyword issue, the overlay fs keyword
in `/proc/filesystems` can be `overlay` or `overlayfs`.
>     
>     But for `aufs`, I think we do not need to introduce the new API of `fs::aufs::supported`
but call `fs::supported` directly, as the API of `fs::aufs::supported` is just a wrapper of
`fs::supported`. Comments?
> 
> Jie Yu wrote:
>     Yeah, I agree with you Guangya. I think we should make `fs::supported(fsname)` understand
both overlayfs and overlay and get rid of all fs::xxx::supported logics. Do you have time
to follow up with a patch?
> 
> Guangya Liu wrote:
>     Hi Yu Jie, it may be difficult to make `fs::supported(fsname)` to understand both
overlayfs and overlay, so my thinking is that we only keep `fs::overlay::supported` but remove
the `fs::aufs::supported`, comments?
> 
> Jie Yu wrote:
>     Why it's difficult to make fs::supported(fsname) to understand both overlayfs and
overlay?
> 
> Guangya Liu wrote:
>     My first thinking is as https://reviews.apache.org/r/44421/diff/1/
>     
>     But considering this will change the parameter of `fs::supported`, so we decide to
add `fs::overlay::supported`.  Comments?
> 
> Jie Yu wrote:
>     Let's not change the parameter of fs::supported, instead, change the logic inside
fs::supported so that it handles overlay/overlayfs well.
> 
> Guangya Liu wrote:
>     Got it, then the `fs::supported` may have some special logic for `overlay` and `overlayfs`,
I will post a patch later for this.

I posted two patches here

https://reviews.apache.org/r/48618/ Refactored overlay, overlayfs and aufs check to fs::supported.
https://reviews.apache.org/r/48619/ Removed fs::aufs::supported and fs::overlay::supported.


- Guangya


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


On 六月 6, 2016, 11:57 p.m., Shuai Lin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47396/
> -----------------------------------------------------------
> 
> (Updated 六月 6, 2016, 11:57 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, and Jie Yu.
> 
> 
> Bugs: MESOS-4672
>     https://issues.apache.org/jira/browse/MESOS-4672
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added aufs provisioning backend.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am a08ea407d631f6ae56ac36b122bfdf0e849e8b56 
>   src/slave/containerizer/mesos/provisioner/backend.cpp b2a20b7c08fa790da09ba05b725248e42b8d3bc4

>   src/slave/containerizer/mesos/provisioner/backends/aufs.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/backends/aufs.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_backend_tests.cpp bc04760c9504ea55cd4bb72c7e9012e43a5911aa

>   src/tests/environment.cpp 849e9ce05864f6ec1a736dfc1a7a31d2364c84bf 
> 
> Diff: https://reviews.apache.org/r/47396/diff/
> 
> 
> Testing
> -------
> 
> - Make check on ubuntu 14.04 64bit
> - Test manually: start slave with aufs backend, and write a simple script that launches
tasks with alpine/ubuntu images
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>


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