We'll need to rebase this again for pull/533 but we need to get the
libvirt_xml stuff stable also. Anyway, here's a few comments for the
next version...
On 08/30/2012 05:47 AM, Yu Mingfei wrote:
Signed-off-by: Yu Mingfei<[email protected]>
---
client/tests/libvirt/tests/virt_edit.py | 230
+++++++++++++++++++++++++++++++
1 files changed, 230 insertions(+), 0 deletions(-)
create mode 100644 client/tests/libvirt/tests/virt_edit.py
diff --git a/client/tests/libvirt/tests/virt_edit.py
b/client/tests/libvirt/tests/virt_edit.py
new file mode 100644
index 0000000..15dc61e
--- /dev/null
+++ b/client/tests/libvirt/tests/virt_edit.py
@@ -0,0 +1,230 @@
+import logging, time, re
+from autotest.client.shared import utils, error
+from autotest.client.virt import remote, libvirt_vm, aexpect, libvirt_xml
+
+
+def cmd_virt_edit(option_vm="", option_file="", prefix_option="",
suffix_option=""):
+ """
+ Create command of virt-edit.
+
+ @param option_dom_disk:virt-edit object,a dom name or a disk path
+ @param option_file:the file to edit
+ @param prefix_option:additional prefix option
+ @param suffix_option:additional suffix option
+ @return:return created cmd string
+ """
+ cmd = "virt-edit"
+ if prefix_option != "":
+ cmd += " %s" % prefix_option
+ if option_vm != "":
+ cmd += " %s" % option_vm
+ if option_file != "":
+ cmd += " %s" % option_file
+ if suffix_option != "":
+ cmd += " %s" % suffix_option
+ return cmd
I'm wondering if the need for this function is a sign that the virt-edit
interface isn't so good. How far are you thinking of going with
virt-edit? Maybe we need a new virt_edit module? Otherwise, if it's
just this, then having this function is fine.
+
+
+def exec_virt_edit(cmd):
+ cmd_result = utils.run(cmd, timeout=30, ignore_status=True)
+ logging.info("Status:%s", cmd_result.exit_status)
+ logging.info("Output:%s", cmd_result.stdout.strip())
+ logging.info("Error:%s", cmd_result.stderr.strip())
+ return cmd_result.exit_status
What do you think about making this logging.debug()
+
+
+def add_foo_line(cmd, foo_line):
Hehehe, nice name :D
+ """
+ Execute virt-edit command and add a foo_line to the end of file.
+ """
+ session = aexpect.ShellSession(cmd)
+ time.sleep(15)
Arbitrary sleeps, Eeeeek! These are ripe for causing race-conditions
and can be a big pain to maintain over the log-term (i.e. computers
generally get faster). aexpect.ShellSession is intended to support
subclassing, which is probably what we need here to avoid sleeping.
If you subclass it, then you can rely on it's in-built prompt checking
which will allow you to avoid sleeps. Check out VirshSession class in
virsh module for an example, it's easier than you think.
+ logging.info("Output: %s", session.get_output())
+ session.send('Go')
+ time.sleep(5)
+ session.send('%s' % foo_line)
+ time.sleep(5)
+ session.send('\x1b')
+ time.sleep(5)
+ session.send('ZZ')
+ time.sleep(5)
+ logging.info("virt-edit to add %s success", foo_line)
+ return True
Consider logging.debug here or at least some kind of control so verbose
output can be switched on/off.
+
+
+def run_virt_edit(test, params, env):
+ """
+ Test of virt-edit.
+
+ 1)Start a domain and check its IP according domain's os type.
+ 2)According case in subtest.cfg, rename vm or not.
+ 3)Check domain's state,shutdown vm or not.
+ 4)Get virt-edit's object and create cmd to virt-edit.
+ 5)Execute command virt-edit
+ 6)Check result.
+ 7)According result to PASS or FAIL this case.
+
+ @param test: kvm test object
+ @param params: Dictionary with the test parameters
+ @param env: Dictionary with test environment.
+ """
+ os_type = params.get("windows", "linux")
+ vm_name = params.get("main_vm", "vm1")
+ vm = env.get_vm(params["main_vm"])
+ wait_time = int(params.get("wait_time", "30"))
+
+ #Rename vm if it is needed.
+ #virt-edit can edit a guest's disk by its name,
+ #It is necessary to verify different type name.
Good comment.
+ new_vm_name = params.get("new_vm_name", "")
+ escape_name = params.get("escape_name", "no")
+ if new_vm_name != "":
Just a Tip: In python, empty strings evaluate to False, so you can
actually write this as:
'if not new_vm_name'
Not critical change though, just FYI.
+ libvirt_xml.vm_rename(vm_name, new_vm_name)
This will probably break since libvirt_xml API isn't totally figured out
yet.
+ if escape_name == "yes":
+ test_vm_name = "\\" + new_vm_name
+ else:
+ test_vm_name = new_vm_name
+ vm.name = test_vm_name
+ else:
+ test_vm_name = vm_name
+
+ #check domain ip
+ #if domain is linux and can not be connected ,end test
+ #because test will log in to check command's result.
Another good comment, explains 'why'.
+ if os_type != "windows":
+ if not vm.is_alive():
+ vm.start()
+ vm.wait_for_login(timeout=wait_time)
wait_time is possibly ambiguous param name. Pls fix to make it more
descriptive.
+ vm_ip = vm.get_address(index = 0)
Suggestion: Put comment in config. explaining that only VM's with one
interface are usable with this test. Then it can be future enhancement
If you want to make test support more than one nic.
+ logging.info("VM IP: %s" % vm_ip)
+ ping_result = utils.run("ping -c 2 %s" % vm_ip, ignore_status =
True)
+ if ping_result.exit_status != 0:
+ if new_vm_name != "":
+ libvirt_xml.vm_rename(test_vm_name, vm_name)
+ vm.name = vm_name
+ raise error.TestError("Pinging VM %s has no response." %
vm_name)
Hmmmm, I seem to remember a similar ping-check happening in another test
or in a utils module somewhere. Are you sure there isn't already a
function/method for doing this check?
If not, since I swear I've seen it twice now, we probably should have it
as general code.
+
+ #All failures are stored for envionment's cleanup.
s/envionment's/environment's
good comment
+ fail_flag = 0
+
+ vm_uuid = vm.get_uuid()
+
+ #if we need to test during vm is runing,vm_running is yes.
+ vm_running = params.get("vm_running", "no")
Can't 'start_vm' be re-used here? If not, it's another parameter name
to consider changing so it's not confused.
+ if vm_running != "yes":
+ vm.destroy()
+ vm.wait_for_shutdown(count=30)
+
+ vm_ref = params.get("vm_ref", "")
+ if vm_ref == "vm_disk":
+ get_disk_cmd = "domblklist %s" % test_vm_name
+ disk_list = libvirt_vm.virsh_cmd(get_disk_cmd).stdout.split('\n')
+ logging.info("Disks on %s:\n%s", test_vm_name, disk_list)
+ disk_path = ""
+ for disk in disk_list:
+ disk_name = disk.split(' ')[0]
+ if re.search("[v|h|s]da", disk_name):
+ disk_path = disk.split(' ')[-1]
+ break
+ vm_ref = disk_path.strip()
+ logging.info("disk to edit:%s", vm_ref)
+ elif vm_ref == "vm_name":
+ vm_ref = test_vm_name
+ elif vm_ref == "vm_uuid":
+ vm_ref = vm_uuid
+ elif vm_ref == "created_img":
+ temp_file = "%s/foo.img" % params.get("image_dir", "/tmp")
+ created_img = params.get("created_img", temp_file)
+ vm_ref = created_img
+ utils.run("dd if=/dev/zero of=%s bs=512M count=2" % created_img)
+
+ #get file to edit in domain or disk
+ file_ref = params.get("exist_file", "exist_file")
+ addition_edit = params.get("addition_edit", "no")
+ foo_line = params.get("foo_line", "#foo")
+ if file_ref == "exist_file":
+ if os_type == "windows":
+ file_ref = params.get("windows_file", "")
+ else:
+ file_ref = params.get("linux_file", "")
+
+ #Stop libvirtd if libvirtd is off.
+ libvirtd = params.get("libvirtd", "on")
+ if libvirtd == "off":
+ libvirt_vm.libvirtd_stop()
+
+ #try to edit files, and add a foo_line as you need.
+ prefix_option = params.get("prefix_option", "")
+ suffix_option = params.get("suffix_option", "")
+ vm_status_error = params.get("vm_status_error", "no")
+ file_status_error = params.get("file_status_error", "no")
+ addition_status_error = params.get("addition_status_error", "no")
+ result_good = (vm_status_error == "no") and (file_status_error ==
"no") \
+ and (addition_status_error == "no")
+ cmd = cmd_virt_edit(vm_ref, file_ref, prefix_option, suffix_option)
+ if addition_edit == "yes" and result_good is True:
+ try:
+ logging.info("Virt-edit command:%s", cmd)
+ add_foo_line(cmd, foo_line)
+ except Exception, e:
+ fail_flag = 1
+ logging.info("virt-edit returned invalid, add %s
failed:\n%s", foo_line, e)
+ else:
+ status = exec_virt_edit(cmd)
+ if status:
+ logging.info("virt-edit test pass, result is expected.")
+ else:
+ fail_flag = 1
+ logging.info("virt-edit returned is not expected.")
+
+ #Recover libvirtd if libvirtd is off.
+ if libvirtd == "off":
+ libvirt_vm.libvirtd_start()
+
+ if params.get("vm_ref") == "created_img":
+ utils.run("rm -f %s" % created_img)
+
+ if fail_flag != 0:
+ #Recover modified vm
+ if new_vm_name != "":
+ libvirt_xml.vm_rename(test_vm_name, vm_name)
+ vm.name = vm_name
+ raise error.TestFail("Command executed failed, Test Failed.")
+ else:
+ #login vm to check whether foo_line has been added if os is linux
+ if os_type == "linux" and addition_edit == "yes" and
result_good is True:
+ vm.start()
+ cat_file = ""
+ if not vm.is_alive():
+ fail_flag = 1
+ logging.info("vm can not be started when check
result.Test Fail.")
+ else:
+ try:
+ vm.wait_for_login(timeout=wait_time)
+ passwd = params.get("password")
+ session = remote.remote_login("ssh", vm_ip,
+ "22", "root",
passwd, "#")
+ time.sleep(5)
+ cat_file = session.cmd_output('cat /etc/hosts',
internal_timeout=10)
+ logging.info("\n%s", cat_file)
+ time.sleep(5)
+ session.cmd('cp -f /etc/hosts /etc/hosts.bk')
+ time.sleep(5)
+ session.cmd('sed -e \'/%s/d\' /etc/hosts.bk>
/etc/hosts' % foo_line)
+ time.sleep(5)
+ session.cmd('rm -f /etc/hosts.bk')
+ time.sleep(5)
+ session.close()
+ except Exception, e:
+ fail_flag = 1
+ logging.info("vm guest can not be accessed.Test
Fail:%s", e)
+ vm.destroy()
+ if not re.search(foo_line, cat_file):
+ fail_flag = 1
+ logging.info("Edit file fail, do not find %s.Test
Fail.", foo_line)
+
+ if new_vm_name != "":
+ libvirt_xml.vm_rename(test_vm_name, vm_name)
+ vm.name = vm_name
+ if fail_flag != 0:
+ raise error.TestFail("Check result failed,Test Failed.")
-- 1.7.1
--
Best Regards
Yu Mingfei
--
Chris Evich, RHCA, RHCE, RHCDS, RHCSS
Quality Assurance Engineer
e-mail: cevich + `@' + redhat.com o: 1-888-RED-HAT1 x44214
_______________________________________________
Autotest-kernel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/autotest-kernel
