Change in vdsm[master]: adding tx/rx statistic to getInterfacesStat

danken at redhat.com danken at redhat.com
Thu Dec 11 21:55:12 UTC 2014


Dan Kenigsberg has posted comments on this change.

Change subject: adding tx/rx statistic to getInterfacesStat
......................................................................


Patch Set 7: Code-Review-1

(3 comments)

http://gerrit.ovirt.org/#/c/35061/7/tests/functional/networkTests.py
File tests/functional/networkTests.py:

Line 293:         if nics is None:
Line 294:             self.assertNotIn(bondName, netinfo.bondings,
Line 295:                              '%s found unexpectedly' % bondName)
Line 296:         else:
Line 297:             self.assertTrue(
again: please avoid these unsolicited formatting.
Line 298:                 bondName not in netinfo.bondings
Line 299:                 or (set(nics) !=
Line 300:                     set(netinfo.bondings[bondName]['slaves'])),
Line 301:                 '%s found unexpectedly' % bondName)


Line 399:             prevTxStat = hostStats['network'][left]['tx']
Line 400:             # running ARP from the interface
Line 401:             rc, out, err = execCmd([_ARPPING_COMMAND.cmd, '-I', left,
Line 402:                                     '-c', '1', IP_ADDRESS_IN_NETWORK])
Line 403:             time.sleep(2)
arbitrary sleeps  like this must include an explanation: what are we waiting for?
Line 404:             status, msg, hostStats = self.vdsm_net.getVdsStats()
Line 405:             self.assertIn('tx', hostStats)
Line 406:             self.assertIn('rx', hostStats)
Line 407:             # get the tx stat,it should increment by 1 ARP request i.e 42 bytes


Line 415:             disableIpv6Interface(right)
Line 416:             veth.setIP(left, IP_ADDRESS, IP_CIDR)
Line 417:             veth.setIP(right, IP_ADDRESS_IN_NETWORK, IP_CIDR)
Line 418:             veth.setLinkUp(left)
Line 419:             self.retryAssert(assertTestDevStatsReported, timeout=20)
Why do you need to retry 20 times?

I fathomed that you need a single ARPING done here, and then wait a bit to read the new stats of the interface.
Line 420: 
Line 421:     @cleanupNet
Line 422:     @permutations([[True], [False]])
Line 423:     @RequireDummyMod


-- 
To view, visit http://gerrit.ovirt.org/35061
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I314a04570dec56677168e0fdb4a7824d299dbd65
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amir Shladovsky <a.shlado at gmail.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list