> On Jan. 23, 2017, 1: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. > > James Peach wrote: > 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 :)
re `setmntent(3)`: sounds worthy of a TODO but given that they are currently used mainly for tests I would take this short cut for now :) re partial failure: I guess it's unavoidable. I don't think this method itself knows if it should ignore the failure, right now it returns an error when `fs::unmount` fails on a mount entry so I guess returning an error when `umount --fake` fails is basically treating the two as two equal steps taken for each mount. - Jiang Yan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55795/#review162706 ----------------------------------------------------------- On Jan. 20, 2017, 3: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, 3: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 > >
