You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Shuai Lin <li...@gmail.com> on 2016/02/18 16:56:47 UTC
Review Request 43718: Added fs::supported() function.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/
-----------------------------------------------------------
Review request for mesos and Jie Yu.
Bugs: MESOS-4707
https://issues.apache.org/jira/browse/MESOS-4707
Repository: mesos
Description
-------
Added fs::supported() function.
Diffs
-----
src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
Diff: https://reviews.apache.org/r/43718/diff/
Testing
-------
make check
Thanks,
Shuai Lin
Re: Review Request 43718: Added fs::supported() function.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review119669
-----------------------------------------------------------
src/linux/fs.hpp (lines 350 - 351)
<https://reviews.apache.org/r/43718/#comment181010>
Can you move this to the top?
src/linux/fs.cpp (line 409)
<https://reviews.apache.org/r/43718/#comment181012>
Ditto on moving this to the top of this file.
src/linux/fs.cpp (lines 411 - 412)
<https://reviews.apache.org/r/43718/#comment181011>
Ditto on using os::read here.
- Jie Yu
On Feb. 18, 2016, 4:01 p.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2016, 4:01 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by Jie Yu <yu...@gmail.com>.
> On Feb. 18, 2016, 7:23 p.m., Benjamin Bannier wrote:
> > src/linux/fs.hpp, line 351
> > <https://reviews.apache.org/r/43718/diff/2/?file=1254441#file1254441line351>
> >
> > Not sure how exactly this will be used later, but I would much prefer a less stringly-typed and more type-safe variant.
> >
> > What about something like this?
> >
> > enum class FileSystem { ext2, ext3, ext4 };
> >
> > bool supported(FileSystem fileSystem);
> >
> > This would minimize weird usage like the `notexistingfs` you have in your test.
One use case for that is: for overlayfs backend, we need to check if overlayfs is supported or not on the box.
in fact, i'm ok with the string type so that we don't have to list all potential fs type that we want to support. It's a simple utility function to see if an entry exists in /proc/filesystem. I would rather keeping it simple and not introducing the enum here.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review119689
-----------------------------------------------------------
On Feb. 18, 2016, 4:01 p.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2016, 4:01 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by Shuai Lin <li...@gmail.com>.
> On Feb. 18, 2016, 7:23 p.m., Benjamin Bannier wrote:
> > src/linux/fs.hpp, line 351
> > <https://reviews.apache.org/r/43718/diff/2/?file=1254441#file1254441line351>
> >
> > Not sure how exactly this will be used later, but I would much prefer a less stringly-typed and more type-safe variant.
> >
> > What about something like this?
> >
> > enum class FileSystem { ext2, ext3, ext4 };
> >
> > bool supported(FileSystem fileSystem);
> >
> > This would minimize weird usage like the `notexistingfs` you have in your test.
>
> Jie Yu wrote:
> One use case for that is: for overlayfs backend, we need to check if overlayfs is supported or not on the box.
>
> in fact, i'm ok with the string type so that we don't have to list all potential fs type that we want to support. It's a simple utility function to see if an entry exists in /proc/filesystem. I would rather keeping it simple and not introducing the enum here.
I agree with Jie on this, let's keep it simple here.
- Shuai
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review119689
-----------------------------------------------------------
On Feb. 19, 2016, 2:02 a.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2016, 2:02 a.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review119689
-----------------------------------------------------------
src/linux/fs.hpp (line 351)
<https://reviews.apache.org/r/43718/#comment181028>
Not sure how exactly this will be used later, but I would much prefer a less stringly-typed and more type-safe variant.
What about something like this?
enum class FileSystem { ext2, ext3, ext4 };
bool supported(FileSystem fileSystem);
This would minimize weird usage like the `notexistingfs` you have in your test.
- Benjamin Bannier
On Feb. 18, 2016, 5:01 p.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2016, 5:01 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by Cong Wang <xi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review119819
-----------------------------------------------------------
Ship it!
Ship It!
- Cong Wang
On Feb. 19, 2016, 2:02 a.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2016, 2:02 a.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by Guangya Liu <gy...@gmail.com>.
> On 二月 19, 2016, 9:28 a.m., haosdent huang wrote:
> > src/tests/containerizer/fs_tests.cpp, line 49
> > <https://reviews.apache.org/r/43718/diff/3/?file=1258902#file1258902line49>
> >
> > I use CentOS 6. But seems don't have `ext2` and `ext3` in `/proc/filesystems`
> >
> > ```
> > at /proc/filesystems
> > nodev sysfs
> > nodev rootfs
> > nodev bdev
> > nodev proc
> > nodev cgroup
> > nodev cpuset
> > nodev tmpfs
> > nodev devtmpfs
> > nodev binfmt_misc
> > nodev debugfs
> > nodev securityfs
> > nodev sockfs
> > nodev usbfs
> > nodev pipefs
> > nodev anon_inodefs
> > nodev inotifyfs
> > nodev devpts
> > nodev ramfs
> > nodev hugetlbfs
> > iso9660
> > nodev pstore
> > nodev mqueue
> > ext4
> > vfat
> > ```
I think that's why using EXPECT_SOME_TRUE here. The ubuntu does have this.
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review119845
-----------------------------------------------------------
On 二月 19, 2016, 2:02 a.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated 二月 19, 2016, 2:02 a.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by Shuai Lin <li...@gmail.com>.
> On Feb. 19, 2016, 9:28 a.m., haosdent huang wrote:
> > src/tests/containerizer/fs_tests.cpp, line 49
> > <https://reviews.apache.org/r/43718/diff/3/?file=1258902#file1258902line49>
> >
> > I use CentOS 6. But seems don't have `ext2` and `ext3` in `/proc/filesystems`
> >
> > ```
> > at /proc/filesystems
> > nodev sysfs
> > nodev rootfs
> > nodev bdev
> > nodev proc
> > nodev cgroup
> > nodev cpuset
> > nodev tmpfs
> > nodev devtmpfs
> > nodev binfmt_misc
> > nodev debugfs
> > nodev securityfs
> > nodev sockfs
> > nodev usbfs
> > nodev pipefs
> > nodev anon_inodefs
> > nodev inotifyfs
> > nodev devpts
> > nodev ramfs
> > nodev hugetlbfs
> > iso9660
> > nodev pstore
> > nodev mqueue
> > ext4
> > vfat
> > ```
>
> Guangya Liu wrote:
> I think that's why using EXPECT_SOME_TRUE here. The ubuntu does have this.
>
> haosdent huang wrote:
> Yes, but this test case would fail in CentOS 6?
Good catch, I think that's because ext2 and ext3 modules are not loaded on your server by default. I removed "ext2" and "ext3" test, and added "rootfs" test.
- Shuai
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review119845
-----------------------------------------------------------
On Feb. 21, 2016, 7:43 a.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2016, 7:43 a.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by haosdent huang <ha...@gmail.com>.
> On Feb. 19, 2016, 9:28 a.m., haosdent huang wrote:
> > src/tests/containerizer/fs_tests.cpp, line 49
> > <https://reviews.apache.org/r/43718/diff/3/?file=1258902#file1258902line49>
> >
> > I use CentOS 6. But seems don't have `ext2` and `ext3` in `/proc/filesystems`
> >
> > ```
> > at /proc/filesystems
> > nodev sysfs
> > nodev rootfs
> > nodev bdev
> > nodev proc
> > nodev cgroup
> > nodev cpuset
> > nodev tmpfs
> > nodev devtmpfs
> > nodev binfmt_misc
> > nodev debugfs
> > nodev securityfs
> > nodev sockfs
> > nodev usbfs
> > nodev pipefs
> > nodev anon_inodefs
> > nodev inotifyfs
> > nodev devpts
> > nodev ramfs
> > nodev hugetlbfs
> > iso9660
> > nodev pstore
> > nodev mqueue
> > ext4
> > vfat
> > ```
>
> Guangya Liu wrote:
> I think that's why using EXPECT_SOME_TRUE here. The ubuntu does have this.
Yes, but this test case would fail in CentOS 6?
- haosdent
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review119845
-----------------------------------------------------------
On Feb. 19, 2016, 2:02 a.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2016, 2:02 a.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by haosdent huang <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review119845
-----------------------------------------------------------
src/tests/containerizer/fs_tests.cpp (line 49)
<https://reviews.apache.org/r/43718/#comment181228>
I use CentOS 6. But seems don't have `ext2` and `ext3` in `/proc/filesystems`
```
at /proc/filesystems
nodev sysfs
nodev rootfs
nodev bdev
nodev proc
nodev cgroup
nodev cpuset
nodev tmpfs
nodev devtmpfs
nodev binfmt_misc
nodev debugfs
nodev securityfs
nodev sockfs
nodev usbfs
nodev pipefs
nodev anon_inodefs
nodev inotifyfs
nodev devpts
nodev ramfs
nodev hugetlbfs
iso9660
nodev pstore
nodev mqueue
ext4
vfat
```
- haosdent huang
On Feb. 19, 2016, 2:02 a.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2016, 2:02 a.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review119874
-----------------------------------------------------------
Patch looks great!
Reviews applied: [43718]
Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On Feb. 19, 2016, 2:02 a.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2016, 2:02 a.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review119840
-----------------------------------------------------------
Ship it!
Ship It!
- Guangya Liu
On 二月 19, 2016, 2:02 a.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated 二月 19, 2016, 2:02 a.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by Shuai Lin <li...@gmail.com>.
> On Feb. 21, 2016, 11:25 a.m., Klaus Ma wrote:
> > src/linux/fs.cpp, line 56
> > <https://reviews.apache.org/r/43718/diff/4/?file=1263675#file1263675line56>
> >
> > Would you add comments on expected format in `/proc/systems`? It'll help other contributors to understand magic number `1` & `2`.
Good suggetstion, updated patch.
- Shuai
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review120067
-----------------------------------------------------------
On Feb. 21, 2016, 12:24 p.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2016, 12:24 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check on ubuntu 14.04 64bit vm
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by Klaus Ma <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review120067
-----------------------------------------------------------
src/linux/fs.cpp (line 56)
<https://reviews.apache.org/r/43718/#comment181438>
Would you add comments on expected format in `/proc/systems`? It'll help other contributors to understand magic number `1` & `2`.
- Klaus Ma
On Feb. 21, 2016, 3:46 p.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2016, 3:46 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check on ubuntu 14.04 64bit vm
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by Shuai Lin <li...@gmail.com>.
> On Feb. 21, 2016, 5:59 p.m., Cong Wang wrote:
> > src/tests/containerizer/fs_tests.cpp, line 48
> > <https://reviews.apache.org/r/43718/diff/5/?file=1263705#file1263705line48>
> >
> > Well, not all kernels compile ext4 module, you can actually test "procfs" and "sysfs" here, they are required by Mesos.
thanks, updated!
- Shuai
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review120076
-----------------------------------------------------------
On Feb. 22, 2016, 2:10 a.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2016, 2:10 a.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check on ubuntu 14.04 64bit vm
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by Cong Wang <xi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review120076
-----------------------------------------------------------
src/tests/containerizer/fs_tests.cpp (line 48)
<https://reviews.apache.org/r/43718/#comment181441>
Well, not all kernels compile ext4 module, you can actually test "procfs" and "sysfs" here, they are required by Mesos.
- Cong Wang
On Feb. 21, 2016, 12:24 p.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2016, 12:24 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check on ubuntu 14.04 64bit vm
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review120072
-----------------------------------------------------------
Patch looks great!
Reviews applied: [43718]
Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On Feb. 21, 2016, 12:24 p.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2016, 12:24 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check on ubuntu 14.04 64bit vm
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by haosdent huang <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review120075
-----------------------------------------------------------
Ship it!
Ship It!
- haosdent huang
On Feb. 21, 2016, 12:24 p.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2016, 12:24 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check on ubuntu 14.04 64bit vm
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by Shuai Lin <li...@gmail.com>.
> On Feb. 25, 2016, 7:53 p.m., Jie Yu wrote:
> > src/linux/fs.cpp, line 60
> > <https://reviews.apache.org/r/43718/diff/6/?file=1263757#file1263757line60>
> >
> > Should we return an Error here?
Updated.
- Shuai
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review120747
-----------------------------------------------------------
On Feb. 22, 2016, 2:10 a.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2016, 2:10 a.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check on ubuntu 14.04 64bit vm
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review120747
-----------------------------------------------------------
Fix it, then Ship it!
Thanks!
src/linux/fs.cpp (line 51)
<https://reviews.apache.org/r/43718/#comment182217>
I would just be more specific:
```
Failed to read /proc/filesystems:
```
src/linux/fs.cpp (line 60)
<https://reviews.apache.org/r/43718/#comment182220>
Should we return an Error here?
- Jie Yu
On Feb. 22, 2016, 2:10 a.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2016, 2:10 a.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check on ubuntu 14.04 64bit vm
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by Klaus Ma <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review121140
-----------------------------------------------------------
Ship it!
Ship It!
- Klaus Ma
On Feb. 28, 2016, 9:08 p.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated Feb. 28, 2016, 9:08 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 6bf87a19a795cf0a1970f160829b477a35cb789a
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check on ubuntu 14.04 64bit vm
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by Shuai Lin <li...@gmail.com>.
> On Feb. 28, 2016, 1:16 p.m., Guangya Liu wrote:
> > src/linux/fs.cpp, line 60
> > <https://reviews.apache.org/r/43718/diff/7/?file=1273083#file1273083line60>
> >
> > I think that should `continue` here, if one line failed, other lines should still be tried to see if the fs is supportted.
> >
> > But it would be good to log some WARNINIG message here before continue.
> >
> > @Jie Yu, @Shuai Lin, what do you say?
>
> Klaus Ma wrote:
> According to the comments, only one or two columns is accepted. If the data format did not math our expectation, we should return error.
>
> > // Each line of /proc/filesystems is "nodev" + "\t" + "fsname", and the
> > // field "nodev" is optional. For the details, check the kernel src code:
I agree with @klaus1982, if the code execution reaches here, it indicates the kernel (very unlikely) changes the format of `/proc/filesystems`, which we should never ignore.
- Shuai
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review121141
-----------------------------------------------------------
On Feb. 28, 2016, 4:56 p.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated Feb. 28, 2016, 4:56 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 6bf87a19a795cf0a1970f160829b477a35cb789a
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check on ubuntu 14.04 64bit vm
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by Klaus Ma <kl...@gmail.com>.
> On Feb. 28, 2016, 9:16 p.m., Guangya Liu wrote:
> > src/linux/fs.cpp, line 60
> > <https://reviews.apache.org/r/43718/diff/7/?file=1273083#file1273083line60>
> >
> > I think that should `continue` here, if one line failed, other lines should still be tried to see if the fs is supportted.
> >
> > But it would be good to log some WARNINIG message here before continue.
> >
> > @Jie Yu, @Shuai Lin, what do you say?
According to the comments, only one or two columns is accepted. If the data format did not math our expectation, we should return error.
> // Each line of /proc/filesystems is "nodev" + "\t" + "fsname", and the
> // field "nodev" is optional. For the details, check the kernel src code:
- Klaus
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review121141
-----------------------------------------------------------
On Feb. 28, 2016, 9:08 p.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated Feb. 28, 2016, 9:08 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 6bf87a19a795cf0a1970f160829b477a35cb789a
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check on ubuntu 14.04 64bit vm
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review121141
-----------------------------------------------------------
src/linux/fs.cpp (line 60)
<https://reviews.apache.org/r/43718/#comment182796>
I think that should `continue` here, if one line failed, other lines should still be tried to see if the fs is supportted.
But it would be good to log some WARNINIG message here before continue.
@Jie Yu, @Shuai Lin, what do you say?
- Guangya Liu
On 二月 28, 2016, 1:08 p.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated 二月 28, 2016, 1:08 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 6bf87a19a795cf0a1970f160829b477a35cb789a
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check on ubuntu 14.04 64bit vm
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by Shuai Lin <li...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/
-----------------------------------------------------------
(Updated Feb. 28, 2016, 4:56 p.m.)
Review request for mesos and Jie Yu.
Bugs: MESOS-4707
https://issues.apache.org/jira/browse/MESOS-4707
Repository: mesos
Description
-------
Added fs::supported() function.
Diffs (updated)
-----
src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
src/linux/fs.cpp 6bf87a19a795cf0a1970f160829b477a35cb789a
src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
Diff: https://reviews.apache.org/r/43718/diff/
Testing
-------
make check on ubuntu 14.04 64bit vm
Thanks,
Shuai Lin
Re: Review Request 43718: Added fs::supported() function.
Posted by Shuai Lin <li...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/
-----------------------------------------------------------
(Updated Feb. 28, 2016, 1:08 p.m.)
Review request for mesos and Jie Yu.
Changes
-------
Address review comments.
Bugs: MESOS-4707
https://issues.apache.org/jira/browse/MESOS-4707
Repository: mesos
Description
-------
Added fs::supported() function.
Diffs (updated)
-----
src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
src/linux/fs.cpp 6bf87a19a795cf0a1970f160829b477a35cb789a
src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
Diff: https://reviews.apache.org/r/43718/diff/
Testing
-------
make check on ubuntu 14.04 64bit vm
Thanks,
Shuai Lin
Re: Review Request 43718: Added fs::supported() function.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review120106
-----------------------------------------------------------
Patch looks great!
Reviews applied: [43718]
Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On Feb. 22, 2016, 2:10 a.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2016, 2:10 a.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check on ubuntu 14.04 64bit vm
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by Shuai Lin <li...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/
-----------------------------------------------------------
(Updated Feb. 22, 2016, 2:10 a.m.)
Review request for mesos and Jie Yu.
Changes
-------
use "proc" and "sysfs" in the fs::supported test
Bugs: MESOS-4707
https://issues.apache.org/jira/browse/MESOS-4707
Repository: mesos
Description
-------
Added fs::supported() function.
Diffs (updated)
-----
src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
Diff: https://reviews.apache.org/r/43718/diff/
Testing
-------
make check on ubuntu 14.04 64bit vm
Thanks,
Shuai Lin
Re: Review Request 43718: Added fs::supported() function.
Posted by Shuai Lin <li...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/
-----------------------------------------------------------
(Updated Feb. 21, 2016, 12:24 p.m.)
Review request for mesos and Jie Yu.
Changes
-------
Added comments explaining the format of `/proc/filesystems`
Bugs: MESOS-4707
https://issues.apache.org/jira/browse/MESOS-4707
Repository: mesos
Description
-------
Added fs::supported() function.
Diffs (updated)
-----
src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
Diff: https://reviews.apache.org/r/43718/diff/
Testing
-------
make check on ubuntu 14.04 64bit vm
Thanks,
Shuai Lin
Re: Review Request 43718: Added fs::supported() function.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review120066
-----------------------------------------------------------
Patch looks great!
Reviews applied: [43718]
Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On Feb. 21, 2016, 7:46 a.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2016, 7:46 a.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check on ubuntu 14.04 64bit vm
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by Shuai Lin <li...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/
-----------------------------------------------------------
(Updated Feb. 21, 2016, 7:46 a.m.)
Review request for mesos and Jie Yu.
Bugs: MESOS-4707
https://issues.apache.org/jira/browse/MESOS-4707
Repository: mesos
Description
-------
Added fs::supported() function.
Diffs
-----
src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
Diff: https://reviews.apache.org/r/43718/diff/
Testing (updated)
-------
make check on ubuntu 14.04 64bit vm
Thanks,
Shuai Lin
Re: Review Request 43718: Added fs::supported() function.
Posted by Shuai Lin <li...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/
-----------------------------------------------------------
(Updated Feb. 21, 2016, 7:43 a.m.)
Review request for mesos and Jie Yu.
Changes
-------
CentOS 6 doesn't have ext2/ext3 modules loaded by default.
Bugs: MESOS-4707
https://issues.apache.org/jira/browse/MESOS-4707
Repository: mesos
Description
-------
Added fs::supported() function.
Diffs (updated)
-----
src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
Diff: https://reviews.apache.org/r/43718/diff/
Testing
-------
make check
Thanks,
Shuai Lin
Re: Review Request 43718: Added fs::supported() function.
Posted by Shuai Lin <li...@gmail.com>.
> On Feb. 19, 2016, 9:34 a.m., haosdent huang wrote:
> > src/linux/fs.cpp, line 59
> > <https://reviews.apache.org/r/43718/diff/3/?file=1258901#file1258901line59>
> >
> > `tokens.back()` instead of `tokens[tokens.size() - 1]`?
Good suggestion, updated.
- Shuai
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review119847
-----------------------------------------------------------
On Feb. 21, 2016, 7:43 a.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2016, 7:43 a.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by haosdent huang <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review119847
-----------------------------------------------------------
src/linux/fs.cpp (line 59)
<https://reviews.apache.org/r/43718/#comment181233>
`tokens.back()` instead of `tokens[tokens.size() - 1]`?
- haosdent huang
On Feb. 19, 2016, 2:02 a.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2016, 2:02 a.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by Shuai Lin <li...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/
-----------------------------------------------------------
(Updated Feb. 19, 2016, 2:02 a.m.)
Review request for mesos and Jie Yu.
Changes
-------
Address review comments
Bugs: MESOS-4707
https://issues.apache.org/jira/browse/MESOS-4707
Repository: mesos
Description
-------
Added fs::supported() function.
Diffs (updated)
-----
src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
Diff: https://reviews.apache.org/r/43718/diff/
Testing
-------
make check
Thanks,
Shuai Lin
Re: Review Request 43718: Added fs::supported() function.
Posted by Cong Wang <xi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review119641
-----------------------------------------------------------
src/linux/fs.cpp (line 411)
<https://reviews.apache.org/r/43718/#comment180989>
Use os::read() which handles error nicely.
src/linux/fs.cpp (line 413)
<https://reviews.apache.org/r/43718/#comment180990>
Use tokenize("\n")
- Cong Wang
On Feb. 18, 2016, 4:01 p.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2016, 4:01 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/#review119703
-----------------------------------------------------------
Patch looks great!
Reviews applied: [43718]
Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On Feb. 18, 2016, 4:01 p.m., Shuai Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2016, 4:01 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added fs::supported() function.
>
>
> Diffs
> -----
>
> src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
> src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
> src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
>
> Diff: https://reviews.apache.org/r/43718/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Shuai Lin
>
>
Re: Review Request 43718: Added fs::supported() function.
Posted by Shuai Lin <li...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43718/
-----------------------------------------------------------
(Updated Feb. 18, 2016, 4:01 p.m.)
Review request for mesos and Jie Yu.
Bugs: MESOS-4707
https://issues.apache.org/jira/browse/MESOS-4707
Repository: mesos
Description
-------
Added fs::supported() function.
Diffs (updated)
-----
src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595
src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e
src/tests/containerizer/fs_tests.cpp 29e43877612fa151e6f6d79268a7411272a7bfeb
Diff: https://reviews.apache.org/r/43718/diff/
Testing
-------
make check
Thanks,
Shuai Lin