Change in vdsm[master]: mom: add mom balloon functional tests for running vms

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 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Mei Liu 
Gerrit-Reviewer: oVirt Jenkins CI Server