On 05/24/2012 06:07 AM, Alex Jia wrote:
> On 05/24/2012 03:46 PM, tangchen wrote:
>> Signed-off-by: Tang Chen<[email protected]>
>> ---
>> client/virt/libvirt_vm.py | 63
>> ++++++++++++++++++++++++++++++++++++++++----
>> 1 files changed, 57 insertions(+), 6 deletions(-)
>>
>> diff --git a/client/virt/libvirt_vm.py b/client/virt/libvirt_vm.py
>> index 5d35888..ce594af 100644
>> --- a/client/virt/libvirt_vm.py
>> +++ b/client/virt/libvirt_vm.py
>> @@ -424,6 +424,7 @@ def virsh_migrate(options, name, dest_uri, extra,
>> uri = ""):
>> # Rely on test-code to verify guest state on receiving-end
>> migrate_cmd = "migrate %s %s %s %s" %\
>> (options, name, dest_uri, extra)
>> + logging.debug("Migration command: %s" % migrate_cmd)
>> try:
>> virsh_cmd(migrate_cmd, uri)
>> except error.CmdError, detail:
>> @@ -432,6 +433,44 @@ def virsh_migrate(options, name, dest_uri, extra,
>> uri = ""):
>> return True
>>
>>
>> +def virsh_migrate_error(options="", vm_ref="", dest_uri="", extra="",
>> uri=""):
>> + """
>> + Migrate a guest with wrong options.
>> + This function is for error testing. So it doesn't handle any fault.
>> +
>> + @param: options: Free-form string of options to virsh migrate
>> + @param: vm_ref: How to refer a guest (invalid name, invalid uuid and
>> so on)
>> + @param: dest_uri: libvirt uri to send guest to
>> + @param: extra: Free-form string of options to follow<domain> <desturi>
>> + @return: True if migration command was successful
>> + """
>> + cmd = ""
>> + if uri:
>> + cmd += "virsh -c %s migrate " % uri
>> + else:
>> + cmd += "virsh migrate "
>> + if options:
>> + cmd += options
>> + if vm_ref:
>> + cmd += " --domain %s " % vm_ref
>> + if dest_uri:
>> + cmd += " --desturi %s " % dest_uri
>> + if extra:
>> + cmd += extra
>> + logging.debug("Migration command: %s" % cmd)
>> +
>> + cmd_result = utils.run(cmd, ignore_status=True)
>> + logging.info("Output: %s", cmd_result.stdout.strip())
>> + logging.error("Error: %s", cmd_result.stderr.strip())
>> + logging.info("Status: %d", cmd_result.exit_status)
This is good, I like that you're logging the status and output here.
This is good to keep users informed of what is happening. The only
thing I'd suggest is maybe not use logging.error().
logging.error is for when the test code breaks, not for when
test-subject (i.e. virsh or libvirt) breaks. When test-subject breaks,
that's not always a test error (i.e. when you're doing error testing).
>> + # Here we should keep the return value type the same as do_migration().
>> + if cmd_result.exit_status == 0:
>> + return True
>> + else:
>> + return False
>> +
>> +
>> def _check_interface_type(type):
>> if not type:
>> logging.error("Interface type missing.")
>> @@ -1280,20 +1319,32 @@ class VM(virt_vm.BaseVM):
>> lockfile.close()
>>
>>
>> -
>> - def migrate(self, dest_uri, options="--live --timeout 60", extra=""):
>> + def migrate(self, dest_uri, options="--live --timeout 60", extra="",
>> + error_test=False, vm_ref=""):
>> """
>> Migrate a VM to a remote host.
>>
>> @param: dest_uri: Destination libvirt URI
>> @param: options: Migration options before<domain> <desturi>
>> @param: extra: Migration options after<domain> <desturi>
>> + @param: error_test: If we are doing error test.
>> + @param: vm_ref: How to refer a guest when doing error test.
>> @return: True if command succeeded
>> """
>> - logging.info("Migrating VM %s from %s to %s" %
>> - (self.name, self.connect_uri, dest_uri))
>> - result = virsh_migrate(options, self.name, dest_uri,
>> - extra, self.connect_uri)
>> + if not error_test:
>> + if self.connect_uri == dest_uri:
>> + logging.error("Destination uri and source uri are the same.")
>> + return False
>> + logging.info("Migrating VM %s from %s to %s" %
>> + (self.name, self.connect_uri, dest_uri))
>> + result = virsh_migrate(options, self.name, dest_uri,
>> + extra, self.connect_uri)
>> +
>> + else:
>> + logging.info("Doing migration with wrong options.")
>> + result = virsh_migrate_error(options, vm_ref, dest_uri,
>> + extra, self.connect_uri)
> Is virsh_migrate_error() a necessary function? whether we may use
> 'try...except' to catch
> virsh_migrate() exception.
I too would prefer to see it all in one function. This will make
maintenance easier. One idea is maybe move the exception handling up
the stack into the vm.migrate() and check the error_test bool there?
I think I'm guilty of putting too much validation/checking in
virsh_function, so this is partly my fault. Perhaps instead of
returning True/False, virsh_migrate() should just return the cmd_result
up the stack to vm.migrate() and we stick try...except there also.
This way we can handle error testing all w/in one place and it will have
side-effect of making lower level virsh_migrate() function more generic
and flexible.
>> +
>> # On successful migration, point to guests new hypervisor
>> if result == True:
>> self.connect_uri = dest_uri
>
--
Chris Evich, RHCA, RHCE, RHCDS, RHCSS
Quality Assurance Engineer
e-mail: cevich + `@' + redhat.com o: 1-888-RED-HAT1 x44214
_______________________________________________
Autotest mailing list
[email protected]
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
