Mark Wu has posted comments on this change. Change subject: mom: add mom balloon functional tests for running vms ......................................................................
Patch Set 2: I would prefer that you didn't submit this (5 inline comments) .................................................... File tests/functional/momTests.py Line 65: self.assertEqual(pages_to_scan, hostStats['ksmPages']) Line 66: Line 67: def _filterVmsStats(self, vmsStats, filteredStats): Line 68: # Filter all vms' statistics to get balloon operation candidates. Line 69: for Stats in vmsStats: states is better than States Line 70: try: Line 71: if Stats['status'] == 'Running' and Stats['balloonInfo'] \ Line 72: and Stats['memoryStats']: Line 73: filteredStats.append(Stats) Line 69: for Stats in vmsStats: Line 70: try: Line 71: if Stats['status'] == 'Running' and Stats['balloonInfo'] \ Line 72: and Stats['memoryStats']: Line 73: filteredStats.append(Stats) filteredStats is a little bit confusing. Any reason using a pass in argument instead of return value? Line 74: except KeyError: Line 75: pass Line 76: return Line 77: Line 134: Host.mem_available)) Line 135: (if (<= host_free_percent pressure_threshold) Line 136: (with Guests guest (shrink_guest guest)) Line 137: 0)""" Line 138: You could save the policy string in a separate file Line 139: r = self.s.setMOMPolicy(testPolicyStr) Line 140: self.assertOK(r) Line 141: Line 142: # Wait for the policy taking effect Line 214: (defvar host_free_percent (/ (Host.StatAvg "mem_free") Line 215: Host.mem_available)) Line 216: (if (> host_free_percent pressure_threshold) Line 217: (with Guests guest (grow_guest guest)) 0)""" Line 218: the same here as line 138 Line 219: r = self.s.setMOMPolicy(testPolicyStr) Line 220: self.assertOK(r) Line 221: Line 222: # Wait for the policy taking effect Line 236: self.assertTrue(vmNewStats['balloonInfo']['balloon_cur'] Line 237: <= ceil(balloonMax)) Line 238: self.assertTrue(vmNewStats['balloonInfo']['balloon_cur'] Line 239: >= floor(0.99225 * balloonMax)) Line 240: break I believe there's a lot of code could be shared between testBalloonGrow and testBalloonShrink. So it's better to exact some functions to reduce the duplicated code. -- To view, visit http://gerrit.ovirt.org/13156 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I922568233dc769d83e2fdffe1c24439d13d03d7e Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mei Liu <[email protected]> Gerrit-Reviewer: Mark Wu <[email protected]> Gerrit-Reviewer: Mei Liu <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
