From reviews-return-36483-apmail-mesos-reviews-archive=mesos.apache.org@mesos.apache.org Mon Jun 13 03:17:26 2016 Return-Path: X-Original-To: apmail-mesos-reviews-archive@minotaur.apache.org Delivered-To: apmail-mesos-reviews-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 682F01946F for ; Mon, 13 Jun 2016 03:17:26 +0000 (UTC) Received: (qmail 66492 invoked by uid 500); 13 Jun 2016 03:17:26 -0000 Delivered-To: apmail-mesos-reviews-archive@mesos.apache.org Received: (qmail 66464 invoked by uid 500); 13 Jun 2016 03:17:26 -0000 Mailing-List: contact reviews-help@mesos.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: reviews@mesos.apache.org Delivered-To: mailing list reviews@mesos.apache.org Received: (qmail 66449 invoked by uid 99); 13 Jun 2016 03:17:25 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 13 Jun 2016 03:17:25 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id 67CA42C54CF; Mon, 13 Jun 2016 03:17:25 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============2276472257165755671==" MIME-Version: 1.0 Subject: Re: Review Request 47396: Added aufs provisioning backend. From: Guangya Liu To: Jie Yu , Guangya Liu , Gilbert Song Cc: Mesos ReviewBot , Shuai Lin , mesos Date: Mon, 13 Jun 2016 03:17:25 -0000 Message-ID: <20160613031725.30691.76214@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org/ Auto-Submitted: auto-generated Sender: Guangya Liu X-ReviewGroup: mesos X-Auto-Response-Suppress: DR, RN, OOF, AutoReply X-ReviewRequest-URL: https://reviews.apache.org/r/47396/ X-Sender: Guangya Liu References: <20160607020918.8185.39710@reviews.apache.org> In-Reply-To: <20160607020918.8185.39710@reviews.apache.org> X-ReviewBoard-Diff-For: src/slave/containerizer/mesos/provisioner/backends/aufs.hpp X-ReviewBoard-Diff-For: src/slave/containerizer/mesos/provisioner/backends/aufs.cpp Reply-To: Guangya Liu X-ReviewRequest-Repository: mesos --===============2276472257165755671== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit > On 六月 7, 2016, 2:09 a.m., Guangya Liu wrote: > > src/tests/environment.cpp, lines 492-495 > > > > > > 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 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? 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? - 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 > > --===============2276472257165755671==--