Re: [Libvirt-cim] [PATCH] [TEST]Fixing 06_paused_active_suspend.py tc failure
+ try:
+
+ for i in range(1, (timeout + 1)):
+ sleep(1)
+ cs = computersystem.get_cs_class(virt)(server, test_dom)
+ if cs.Name == test_dom:
+ to_RequestedState = cs.RequestedState
+ enabledState = cs.EnabledState
+ else:
+ logger.error("VS %s not found" % test_dom)
+ return status
You'll want to break here, otherwise the guest doesn't get destroyed
properly.
+ if enabledState == expstate:
+ status = PASS
+ break
Instead of breaking here, you can just return since we've found the
value we're looking for.
-
+
+ status, to_RequestedState, enabledState = poll_for_status(options.ip, options.virt,
cxml, 2)
+ if status != PASS or enabledState != 2:
No need to verify enabledState == 2, the poll_for_status function do
this already.
+ return status
+
#Suspend the VS
- rc = call_request_state_change(test_dom, options.ip, REQUESTED_STATE,
- TIME, options.virt)
+ rc = call_request_state_change(test_dom, options.ip, REQUESTED_STATE, TIME,
options.virt)
This change causes the test to span multiple lines. I'll work on
sending out some suggested style guidelines and post them to the list to
see if people agree.
- except Exception, detail:
- logger.error("Exception variable: %s" % detail)
- return status
-
+ status, to_RequestedState, enabledState = poll_for_status(options.ip, options.virt,
cxml,
+
FINAL_STATE)
Unusual formatting here. Usually, the second line of arguments for a
function will line up with the first argument. Something like:
status, to_RequestedState, enabledState = poll_for_status(options.ip,
options.virt, cxml,
FINAL_STATE)
Hopefully the formatting of the email doesn't kill the illustration.
if enabledState != FINAL_STATE:
logger.error("EnabledState has %i instead of %i", enabledState,
FINAL_STATE)
logger.error("Try to increase the timeout and run the test again")
if status != PASS:
- ret = cxml.destroy(options.ip)
- cxml.undefine(options.ip)
Why remove the destroy and undefine here? The guest won't get cleaned
up properly.
return status
# Success:
@@ -134,9 +144,8 @@ def main():
# To state == 2
# Enabled_state == RequestedState
- if from_State == START_STATE and \
- to_RequestedState == FINAL_STATE and \
- enabledState == to_RequestedState:
+ if from_State == START_STATE and to_RequestedState == FINAL_STATE and \
+ enabledState == to_RequestedState:
This is unusual indention style. Usually, the new line starts just
below the if or the first part of the statement. So you'd have:
if from_State == START_STATE and to_RequestedState == FINAL_STATE and \
enabledState == to_RequestedState:
or
if from_State == START_STATE and to_RequestedState == FINAL_STATE and \
enabledState == to_RequestedState:
My preference is the latter. You can hold off on changing this patch
until we agree on a coding style.
--
Kaitlin Rupert
IBM Linux Technology Center
kaitlin(a)linux.vnet.ibm.com