On 05/24/2012 03:47 AM, tangchen wrote:
> Signed-off-by: Tang Chen<[email protected]>
> ---
> client/tests/libvirt/tests/virsh_migrate.py | 278
> +++++++++++++++++++++++----
> 1 files changed, 236 insertions(+), 42 deletions(-)
>
> diff --git a/client/tests/libvirt/tests/virsh_migrate.py
> b/client/tests/libvirt/tests/virsh_migrate.py
> index b342987..2fcd045 100644
> --- a/client/tests/libvirt/tests/virsh_migrate.py
> +++ b/client/tests/libvirt/tests/virsh_migrate.py
> @@ -1,5 +1,6 @@
> -import logging, time
> -from autotest.client.shared import error
> +import logging, os, re, time, shutil, codecs, tempfile
> +from autotest.client.shared import utils, error
> +from autotest.client.virt import libvirt_vm, virt_utils, virt_test_utils
>
> def run_virsh_migrate(test, params, env):
> """
> @@ -16,51 +17,82 @@ def run_virsh_migrate(test, params, env):
> else:
> return False
>
> - def cleanup_dest(vm, src_uri = ""):
> + def cleanup_dest(vm, src_uri=""):
Good catch, IIRC no spaces is proper form. I missed this change earlier.
> """
> Clean up the destination host environment
> when doing the uni-direction migration.
> """
> - vm_state = vm.state()
> - if vm_state == "running":
> - vm.destroy()
> - elif vm_state == "paused":
> - vm.resume()
> - vm.destroy()
> + logging.info("Cleaning up VMs on %s" % vm.connect_uri)
> + try:
> + if libvirt_vm.virsh_domain_exists(vm.name, vm.connect_uri):
> + vm_state = vm.state()
> + if vm_state == "paused":
> + vm.resume()
> + elif vm_state == "shut off":
> + vm.start()
> + vm.destroy()
> +
> +
> + if vm.is_persistent():
> + vm.undefine()
>
> - if vm.is_persistent():
> - vm.undefine()
> + except Exception, detail:
> + logging.error("Cleaning up destination failed.\n%s" % detail)
>
> - vm.connect_uri = src_uri
> + if src_uri:
> + vm.connect_uri = src_uri
Another good catch, I like this way better.
>
> - def do_migration(dly, vm, dest_uri, options, extra):
> - logging.info("Sleeping %d seconds before migration" % dly)
> - time.sleep(dly)
> + def do_error_migration(vm_ref="", dest_uri="", options="", extra=""):
> + logging.info("Sleeping %d seconds before migration with"
> + " invalid options" % delay)
> + time.sleep(delay)
> + successful = vm.migrate(dest_uri, options, extra, True, vm_ref)
> + # We have to keep the return value coincident to do_migration(),
> + # so that we can share the result check procedure.
> + if not successful:
> + logging.info("Migration failed with invalid options.")
> + return False
> + else:
> + logging.error("Migration succeeded with invalid options.")
> + return True
> +
> + def do_migration(delay, vm, dest_uri, options, extra):
> + logging.info("Sleeping %d seconds before migration" % delay)
> + time.sleep(delay)
I think having two functions, do_migration() and do_error_migration()
within test module is fine. This code is not intended to be re-used
elsewhere, so standard is much more relaxed. Unless you feel it would
be better as one function, don't sweat it. It doesn't bother me to have
two functions here.
> # Migrate the guest.
> successful = vm.migrate(dest_uri, options, extra)
> if not successful:
> - raise error.TestFail("Migration failed for %s." % vm_name)
> + logging.error("Migration failed for %s." % vm_name)
> + return False
> +
> + if options.count("dname") or extra.count("dname"):
> + vm.name = extra.split()[1].strip()
>
> if vm.is_alive(): # vm.connect_uri was updated
> logging.info("Alive guest found on destination %s." % dest_uri)
> else:
> - raise error.TestFail("VM not running on destination %s" %
> dest_uri)
> -
> - # Migration may fail, but VM is alive on destination.
> - dest_state = params.get("virsh_migrate_dest_state")
> - ret = check_vm_state(vm, dest_state)
> - logging.info("Supposed state: %s" % dest_state)
> - logging.info("Actual state: %s" % vm.state())
> - if not ret:
> - raise error.TestFail("VM is not in the supposed state.")
> + logging.error("VM not alive on destination %s" % dest_uri)
> + return False
>
> # FIXME: This needs to be tested, but won't work currently
> # vm.verify_kernel_crash()
> + logging.debug("vm.verify_kernel_crash() needs to be tested, "
> + "but won't work currently.")
> + return True
> +
I looked into this more and it appears mainly a problem with older
libvirt. When using newer, I was able to attach to serial console with
'virsh console' command at remote system with no problem. On older
libvirt it just gives error message, which is okay, it's not the test's
fault.
The root problem here is actually in the libvirt_vm code being "too
fancy". I have it on my "to do" list to fix this up so we just use
'virsh console' all the time. I already cleared this with lmr before
and he was okay with it.
For now, it's good to just print something. In fact, if you want to
make this a logging.info or logging.warning that would motivate me more
to fix console problem lol :D
>
> vm_name = params.get("main_vm")
> vm = env.get_vm(params["main_vm"])
> vm.verify_alive()
>
> + # For safety reasons, we'd better back up xmlfile.
> + vm_xmlfile_bak = tempfile.mktemp(dir="/tmp")
> + if not vm.backup_xml(vm_xmlfile_bak):
> + logging.error("Backing up xmlfile failed.")
> +
> + vm.connect_uri = params.get("connect_uri", "default")
> + if vm.connect_uri == 'default':
> + vm.connect_uri = libvirt_vm.virsh_uri()
> +
> src_uri = vm.connect_uri
> dest_uri = params.get("virsh_migrate_desturi")
> # Identify easy config. mistakes early
> @@ -77,22 +109,184 @@ def run_virsh_migrate(test, params, env):
>
> options = params.get("virsh_migrate_options")
> extra = params.get("virsh_migrate_extra")
> - dly = int(params.get("virsh_migrate_delay", 10))
> -
> -
> - do_migration(dly, vm, dest_uri, options, extra)
> - # Repeat the migration with a recursive call and guaranteed exit
> - if params.get("virsh_migrate_back", "no") == 'yes':
> - back_dest_uri = params.get("virsh_migrate_back_desturi", 'default')
> - back_options = params.get("virsh_migrate_back_options", 'default')
> - back_extra = params.get("virsh_migrate_back_extra", 'default')
> - if back_dest_uri == 'default':
> - back_dest_uri = src_uri
> - if back_options == 'default':
> - back_options = options
> - if back_extra == 'default':
> - back_extra = extra
> - do_migration(dly, vm, back_dest_uri, back_options, back_extra)
> - # Do the uni-direction migration here.
> + delay = int(params.get("virsh_migrate_delay", 10))
> + status_error = params.get("status_error", 'no')
> + libvirtd_state = params.get("virsh_migrate_libvirtd_state", 'on')
> + src_state = params.get("virsh_migrate_src_state", "running")
> + new_nic_mac = ""
> + dest_xmlfile = ""
> +
> + exception = False
> + try:
> + # Confirm VM can be accessed through network.
> + time.sleep(delay)
> + vm_ip = vm.get_address()
> + s_ping, o_ping = virt_test_utils.ping(vm_ip, count=2, timeout=delay)
> + logging.info(o_ping)
> + if s_ping != 0:
> + raise error.TestError("%s did not respond after %d sec." %
> (vm.name, delay))
> +
> + # Prepare for --xml.
> + logging.debug("Preparing new xml file for --xml option.")
> + if options.count("xml") or extra.count("xml"):
> + new_nic_mac = virt_utils.generate_mac_address(vm, 1)
> + logging.info("New mac address: %s" % new_nic_mac)
> + dest_xmlfile = params.get("virsh_migrate_xml", "")
> + if dest_xmlfile:
> + ret_attach = vm.attach_interface("bridge", "--source virbr0
> --mac %s" % new_nic_mac)
> + if not ret_attach:
> + exception = True
> + raise error.TestError("Attaching nic to %s failed." %
> vm.name)
> + vm_xml_new = vm.get_xml()
> + logging.debug("Xml file on source: %s" % vm_xml_new)
> + f = codecs.open(dest_xmlfile, 'wb', encoding='utf-8')
> + f.write(vm_xml_new)
> + f.close()
> + if not os.path.exists(dest_xmlfile):
> + exception = True
> + raise error.TestError("Creating %s failed." %
> dest_xmlfile)
> +
> + # Turn VM into certain state.
> + logging.debug("Turning %s into certain state." % vm.name)
> + if src_state == "paused":
> + if vm.is_alive():
> + vm.pause()
> + elif src_state == "shut off":
> + if vm.is_alive():
> + if not vm.shutdown():
> + vm.destroy()
> +
> + # Turn libvirtd into certain state.
> + logging.debug("Turning libvirtd into certain status.")
> + if libvirtd_state == "off":
> + libvirt_vm.libvirtd_stop()
> +
> + # Test uni-direction migration.
> + logging.debug("Doing migration test.")
> + if status_error == 'no':
> + ret_migrate = do_migration(delay, vm, dest_uri, options, extra)
> + else:
> + vm_ref = params.get("vm_ref", vm.name)
> + dest_uri_ref = params.get("dest_uri_ref", dest_uri)
> + ret_migrate = do_error_migration(vm_ref, dest_uri_ref, options,
> extra)
> +
> + # Recover libvirtd state.
> + logging.debug("Recovering libvirtd status.")
> + if libvirtd_state == "off":
> + libvirt_vm.libvirtd_start()
> +
> + # Check vm state on destination.
> + logging.debug("Checking %s state on %s." % (vm.name, vm.connect_uri))
> + if options.count("dname") or extra.count("dname"):
> + vm.name = extra.split()[1].strip()
> + check_dest_state = True
> + dest_state = params.get("virsh_migrate_dest_state", "running")
> + check_dest_state = check_vm_state(vm, dest_state)
> + logging.info("Supposed state: %s" % dest_state)
> + logging.info("Actual state: %s" % vm.state())
> +
> + # Recover VM state.
> + logging.debug("Recovering %s state." % vm.name)
> + if src_state == "paused":
> + vm.resume()
> + elif src_state == "shut off":
> + vm.start()
> +
> + # Checking for --persistent.
> + logging.debug("Checking for --persistent option.")
> + check_dest_persistent = True
> + if options.count("persistent") or extra.count("persistent"):
> + if not vm.is_persistent():
> + check_dest_persistent = False
> +
> + # Checking for --undefinesource.
> + logging.debug("Checking for --undefinesource option.")
> + check_src_undefine = True
> + if options.count("undefinesource") or extra.count("undefinesource"):
> + logging.info("Verifying<virsh domstate> DOES return an error."
> + "%s should not exist on %s." % (vm_name, src_uri))
> + if libvirt_vm.virsh_domain_exists(vm_name, src_uri):
> + check_src_undefine = False
> +
> + # Checking for --dname.
> + logging.debug("Checking for --dname option.")
> + check_dest_dname = True
> + if options.count("dname") or extra.count("dname"):
> + dname = extra.split()[1].strip()
> + if not libvirt_vm.virsh_domain_exists(dname, dest_uri):
> + check_dest_dname = False
> +
> + # Checking for --xml.
> + logging.debug("Checking for --xml option.")
> + check_dest_xml = True
> + if options.count("xml") or extra.count("xml"):
> + if dest_xmlfile:
> + vm_dest_xml = vm.get_xml()
> + logging.info("Xml file on destination: %s" % vm_dest_xml)
> + if not re.search(new_nic_mac, vm_dest_xml):
> + check_dest_xml = False
> +
> + # Repeat the migration from destination to source.
> + if params.get("virsh_migrate_back", "no") == 'yes':
> + back_dest_uri = params.get("virsh_migrate_back_desturi",
> 'default')
> + back_options = params.get("virsh_migrate_back_options",
> 'default')
> + back_extra = params.get("virsh_migrate_back_extra", 'default')
> + if back_dest_uri == 'default':
> + back_dest_uri = src_uri
> + if back_options == 'default':
> + back_options = options
> + if back_extra == 'default':
> + back_extra = extra
> + ret_migrate = do_migration(delay, vm, back_dest_uri,
> back_options, back_extra)
> +
> + except Exception, detail:
> + exception = True
> + logging.error("%s: %s" % (detail.__class__, detail))
> +
> +
> + # Whatever error occurs, we have to clean up all environment.
> + # Make sure vm.connect_uri is the destination uri.
> + vm.connect_uri = dest_uri
> + if options.count("dname") or extra.count("dname"):
> + # Use the VM object to remove
> + vm.name = extra.split()[1].strip()
> + cleanup_dest(vm, src_uri)
> + vm.name = vm_name
> else:
> cleanup_dest(vm, src_uri)
I don't think we want to always call cleanup (which will vm.destroy())
here. Part of the postprocess function that runs in virt_env_process.py
after the test code finishes, will take care of shutting down the vm if
the test calls for it.
Especially in the case of the 'there_and_back_a_lot' test, we use the
'iterations' parameter along with 'kill_vm', 'kill_vm_gracefully', and
'kill_unresponsive_vms' set to 'no', to control what VM state should be
after testing.
The 'iterations' parameter runs through the full preprocess, test code,
and postprocess each time, so code should not shutdown the guest when
virsh_migrate_back == 'yes'. i.e. the guest should still be running on
dest. after the test code finishes.
> + # Recover source (just in case).
> + # vm.connect_uri has been set back to src_uri in cleanup_dest().
> + if not libvirt_vm.virsh_domain_exists(vm_name, src_uri):
> + vm.define(vm_xmlfile_bak)
> + else:
> + #if not vm.shutdown():
> + vm.destroy()
> +
> + # Cleanup source.
> + if os.path.exists(vm_xmlfile_bak):
> + os.remove(vm_xmlfile_bak)
> + logging.info("%s removed." % vm_xmlfile_bak)
> + if os.path.exists(dest_xmlfile):
> + os.remove(dest_xmlfile)
> +
> + if exception:
> + raise error.TestError("Error occurred. \n%s: %s" %
> (detail.__class__, detail))
> +
> + # Check test result.
> + if status_error == 'yes':
> + if ret_migrate:
> + raise error.TestFail("Migration finished with unexpected
> status.")
> + else:
> + if not ret_migrate:
> + raise error.TestFail("Migration finished with unexpected
> status.")
> + if not check_dest_state:
> + raise error.TestFail("Wrong VM state on destination.")
> + if not check_dest_persistent:
> + raise error.TestFail("VM is not persistent on destination.")
> + if not check_src_undefine:
> + raise error.TestFail("VM is not undefined on source.")
> + if not check_dest_dname:
> + raise error.TestFail("Wrong VM name %s on destination." % dname)
> + if not check_dest_xml:
> + raise error.TestFail("Wrong xml configuration on destination.")
I like the use of explicit exception strings here that explain what
condition was detected. Doing this makes debugging so much easier when
the result is not expected.
Overall this is looking good. It's quickly taking shape into a very
nice test module and suite of tests. Thanks!
--
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
