Dan Kenigsberg has posted comments on this change. Change subject: fixing race while sampling interfaces ......................................................................
Patch Set 5: Code-Review-1 (5 comments) http://gerrit.ovirt.org/#/c/36138/5/tests/samplingTests.py File tests/samplingTests.py: Line 34: from testlib import permutations, expandPermutations Line 35: from testlib import VdsmTestCase as TestCaseBase Line 36: from monkeypatch import MonkeyPatchScope Line 37: Line 38: NEW_INTERFACE = 'newinterface' no need to expose these on the module level, I believe. Line 39: NEW_VLAN = 'newvlan' Line 40: Line 41: Line 42: class SamplingTests(TestCaseBase): Line 126: Line 127: @contextmanager Line 128: def dummy(dummy_name): Line 129: try: Line 130: ipwrapper.linkAdd(dummy_name, 'dummy') the try-block should begin only after having created the dummy device. then, the finally clause may contain a clean linkDel, instead of _try_remove_link. Line 131: yield Line 132: finally: Line 133: _try_remove_link(dummy_name) Line 134: Line 148: s0 = sampling.InterfaceSample(lo) Line 149: s1 = sampling.InterfaceSample(lo) Line 150: s1.operstate = 'x' Line 151: self.assertEquals('operstate:x', s1.connlog_diff(s0)) Line 152: these new tests @RequiresRoot, I believe. Line 153: def testHostSampleReportsNewInterface(self): Line 154: hs_before = sampling.HostSample(os.getpid()) Line 155: interfaces_before = set(hs_before.interfaces.iterkeys()) Line 156: Line 153: def testHostSampleReportsNewInterface(self): Line 154: hs_before = sampling.HostSample(os.getpid()) Line 155: interfaces_before = set(hs_before.interfaces.iterkeys()) Line 156: Line 157: with dummy(NEW_INTERFACE): Please include a long random string in the auxiliary interface name, as we'd like to be able to run unit tests by two concurrent processes. Line 158: hs_after = sampling.HostSample(os.getpid()) Line 159: interfaces_after = set(hs_after.interfaces.iterkeys()) Line 160: interfaces_diff = interfaces_after - interfaces_before Line 161: self.assertEqual(interfaces_diff, set([NEW_INTERFACE])) http://gerrit.ovirt.org/#/c/36138/5/vdsm/virt/sampling.py File vdsm/virt/sampling.py: Line 220: for link in getLinks(): Line 221: try: Line 222: links_and_samples[link.name] = InterfaceSample(link) Line 223: except IOError as e: Line 224: # this handles a race condition where the device is now no English: drop the "is". Line 225: # longer exists and netlink fails to fetch it Line 226: if e.errno == errno.ENODEV: Line 227: continue Line 228: raise -- To view, visit http://gerrit.ovirt.org/36138 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I059a5bfa4c140d88f512dc9a51d18f02956710de Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Ido Barkan <[email protected]> Gerrit-Reviewer: Ondřej Svoboda <[email protected]> Gerrit-Reviewer: Petr Horáček <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
