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.