> On April 1, 2016, 2:55 a.m., Qian Zhang wrote: > > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines 418-424 > > <https://reviews.apache.org/r/45571/diff/1/?file=1321722#file1321722line418> > > > > I do not think we need this. If agent crashes after removing the > > interface directory in `_detach`, then we should not get into this > > `foreach` loop because `networkNames` returned by `paths::getNetworkNames` > > should be empty. > > Jie Yu wrote: > Hum, I don't think we delete network dir, do we? We delete ifdir first, > if all are successful, we delete containerDir. But it's possible that we > delete the ifdir, but agent crashes before the containerDir is deleted, right? > > Qian Zhang wrote: > Yes, you are right. However, since there is only one ifdir in a network > dir, I am thinking we may change `_detach` to remove the network dir rather > than remove the ifdir and leave an empty network dir there, and then here we > will not need this code: > if (interfaces->empty()) { > continue; > }
We will support adding more than one interface to a network dir, so I think the approach in my patch is better. - Jie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45571/#review126511 ----------------------------------------------------------- On April 1, 2016, 1:08 a.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45571/ > ----------------------------------------------------------- > > (Updated April 1, 2016, 1:08 a.m.) > > > Review request for mesos, Avinash sridharan and Qian Zhang. > > > Bugs: MESOS-4759 > https://issues.apache.org/jira/browse/MESOS-4759 > > > Repository: mesos > > > Description > ------- > > A few cleanups and simplifications in CNI isolator. > > > Diffs > ----- > > src/slave/containerizer/mesos/isolators/network/cni/cni.hpp > 3a07540909ed771d1bd3b22888e04d5fb451710d > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp > 486c382365d5293cd9d53b8b239f70a543c46792 > > Diff: https://reviews.apache.org/r/45571/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
