----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52596/#review151838 -----------------------------------------------------------
Ship it! The patch LGTM. I would suggest we add some tests in a separate patch to increase the confidence. We could introduce a `MountInfoTable::read` that takes a string. The existing `MountInfoTable::read` can just call that overload once it gets the mount table from the system. Then, in the test, we can construct some mount table entries to simulate situations like this and verify that our code handles it properly. What do you think? - Jie Yu On Oct. 6, 2016, 7:32 a.m., Kevin Klues wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52596/ > ----------------------------------------------------------- > > (Updated Oct. 6, 2016, 7:32 a.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-6118 > https://issues.apache.org/jira/browse/MESOS-6118 > > > Repository: mesos > > > Description > ------- > > It is legal to have entries in a `MountInfoTable` whose `entry.id` is > the same as `entry.parent`. This can happen (for example), if a system > boots from the network and then keeps the original `/` in RAM. > However, to avoid cycles when walking the mount hierarchy, we should > not treat these entries as children of their parent so we skip them. > > This commit adds functionality to handle this case. > > > Diffs > ----- > > src/linux/fs.cpp 4b10141a49dfb3c6defdb07e295eb14cfcdd36ce > > Diff: https://reviews.apache.org/r/52596/diff/ > > > Testing > ------- > > GTEST_FILTER="" make -j40 check > GTEST_FILTER="FsTest.MountInfoTableReadSorted" src/mesos-tests > > > Thanks, > > Kevin Klues > >
