----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50763/#review144686 -----------------------------------------------------------
src/linux/fs.hpp (lines 222 - 223) <https://reviews.apache.org/r/50763/#comment210725> I'd prefer putting parameters in the next line to be less jagged: ``` static Try<MountInfoTable> read( const Option<pid_t>& pid = None(), bool hierarchicalSort = true); ``` src/linux/fs.cpp (lines 86 - 87) <https://reviews.apache.org/r/50763/#comment210726> Ditto on parameters on the next line. src/linux/fs.cpp (line 104) <https://reviews.apache.org/r/50763/#comment210740> space before `==` src/linux/fs.cpp (lines 123 - 124) <https://reviews.apache.org/r/50763/#comment210900> There is a problem here. You don't update the hashmap `parentToFirstChild` if an entry already exists for the key. But with the new insertion, it's likely that the invariant `first child entry that lists another entry as its parent` might already be broken. You still have to update it. For instance, think about the following mount table <id, parent_id>: ``` <2, 5> <7, 10> <5, 10> <10, 11> ``` src/linux/fs.cpp (line 124) <https://reviews.apache.org/r/50763/#comment210744> indentation - Jie Yu On Aug. 3, 2016, 7:10 p.m., Kevin Klues wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50763/ > ----------------------------------------------------------- > > (Updated Aug. 3, 2016, 7:10 p.m.) > > > Review request for mesos, Benjamin Mahler and Jie Yu. > > > Bugs: MESOS-5969 > https://issues.apache.org/jira/browse/MESOS-5969 > > > Repository: mesos > > > Description > ------- > > Many places in the codebase assume that the mountinfo table is sorted > according to the order: 'parent mount point < child mount point'. > > However, in some cases this may not be true if (for example), a parent > mount point (say '/') is remounted to add some extra flags to it. > When this happens, the remounted file system will appear in the > mountinfo table at the point where it was remounted. > > We actually encountered this problem in the wild for the case of '/' > being remounted after '/run' was mounted -- causing problems in the > 'NvidiaVolume' which assumes the 'parent < child' ordering. > > This commit fixes this problem by building the list of MountInfoTable > entries in sorted order when 'read()' is called. An optional flag can > be used to disable sorting produce the the original ordering. > > > Diffs > ----- > > src/linux/fs.hpp ec3b5b8cd6926b1f69ad499de1c13b989766a84e > src/linux/fs.cpp f57db80ad0d7235d47910e05d663c77e233f8228 > > Diff: https://reviews.apache.org/r/50763/diff/ > > > Testing > ------- > > GTEST_FILTER="" make -j check > src/mesos-tests > sudo src/mesos-tests > > Appeared to have one unrelated flaky test fail: > `ResourceOffersTest.ResourcesGetReofferedAfterTaskInfoError` > Rerunning the tests a second time passed. > > > Thanks, > > Kevin Klues > >
