mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From James Peach <jpe...@apache.org>
Subject Re: Review Request 55795: Fixed `umountAll` to optionally also clean up mtab.
Date Tue, 24 Jan 2017 00:10:44 GMT


> On Jan. 23, 2017, 9:39 p.m., James Peach wrote:
> > src/linux/fs.cpp, line 414
> > <https://reviews.apache.org/r/55795/diff/1/?file=1611031#file1611031line414>
> >
> >     Why do you need `cleanUpMtab`. All the callers pass `true` and it is pretty
hard to imagine a scenario where you *want* the mtab to be out of sync.
> 
> Jiang Yan Xu wrote:
>     Yeah I don't think anyone would want that but if the caller mounts everything exclusively
using the syscall under `target` then they don't *need* to clean up mtab. (There's a minor
cost to it and potential failure factors but admittedly I haven't seen it fail in cases including
non-existent /etc/mtab, non-existent entry, etc.)
>     
>     So maybe it's fine to leave out the boolean argument and fix things later if anything
breaks in any environments.

This is already subject to partial failure, but I'd also be OK with ignoring the return from
executing `umount`, or refactoring this to rewrite `mtab` using `setmntent(3)`. I think that
this patch is an existence proof that no-one really knows they don't need to clean up :)


- James


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


On Jan. 20, 2017, 11:32 p.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55795/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2017, 11:32 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and James Peach.
> 
> 
> Bugs: MESOS-6478
>     https://issues.apache.org/jira/browse/MESOS-6478
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> - Without this, Mesos tests leave garbage in /etc/mtab.
> 
> 
> Diffs
> -----
> 
>   src/linux/fs.hpp da49c9ebfa938d169152ed3b6e4df7378711b013 
>   src/linux/fs.cpp 913e23317291db164fe6bdf77f3eca146dedec9b 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 4040d36d1dbf4968f62fa593024936858f28b4df

>   src/tests/environment.cpp a9e217e8beb5a6aca456ff5379893953cafca135 
>   src/tests/persistent_volume_tests.cpp 468a85b4a6ce09592341afd07ce12a03f5fc4f73 
> 
> Diff: https://reviews.apache.org/r/55795/diff/
> 
> 
> Testing
> -------
> 
> sudo make check.
> 
> Verified that no garbage is left in mtab.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>


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