Kaitlin Rupert wrote:
-#define VS + status, test_network = create_netpool_conf(options.ip, options.virt) + if status != PASS: + destroy_netpool(options.ip, options.virt, test_network)
Why do we require to call destroy_netpool() when it is not created successfully?
Nope, this is a good point. I'll remove it and send a new patch.
-#Suspend the defined VS - - try: - ret = cxml.run_virsh_cmd(options.ip, "suspend") - if not ret : - Globals.logger.info("Suspending defined VS %s failed, as expected" \ -% test_dom) - status = PASS + status = chk_state(test_dom, options.ip, DEFINE_STATE, options.virt) + if status != PASS: + logger.error("%s not in defined state as expected." % test_dom) + status = FAIL + I think the log stmt here is misleading. We are trying to move from Define to Suspend, aint?
Correct. However, suspending a defined guest is not supported, so we expect the call to RSC() to fail. Because of that, we expect the guest to remain in the defined state. If the guest is in some other state, then we've encountered some unknown behavior and should fail. Yes I agree moving from define to suspend state is not supported state. But, the log message is misleading. It gives me a feeling that domgst is not in defined state and it is expected state for the domgst. Can we chop off the "as expected" part? or have log stmt something like "%s should have been in defined state" ?? If you still feel the log stmt which you have used is correct, then you can ignore my comment.
I think the log stmt should be as below:
logger.error("%s not in suspend state as expected." % test_dom)
The chk_state() call is verifying that the guest is in the defined state, not the suspended state.