Re: [Qemu-devel] [RFC PATCH 07/14] KVM-test: Add a subtest of load/unload nic driver

Lucas Meneghel Rodrigues
2010-07-28T18:12:58Z

On Tue, 2010-07-20 at 09:35 +0800, Amos Kong wrote:
> Repeatedly load/unload nic driver, try to transfer file between guest and host
> by threads at the same time, and check the md5sum.
>
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
> 0 files changed, 0 insertions(+), 0 deletions(-)
>
> diff --git a/client/tests/kvm/tests/nicdriver_unload.py b/client/tests/kvm/tests/nicdriver_unload.py
> new file mode 100644
> index 0000000..22f9f44
> --- /dev/null
> +++ b/client/tests/kvm/tests/nicdriver_unload.py
> @@ -0,0 +1,128 @@
> +import logging, commands, threading, re, os
> +from autotest_lib.client.common_lib import error
> +import kvm_utils, kvm_test_utils, kvm_net_utils
> +
> +def run_nicdriver_unload(test, params, env):
> + """
> + Test nic driver
> +
> + 1) Boot a vm
> + 2) Get the nic driver name
> + 3) Repeatedly unload/load nic driver
> + 4) Multi-session TCP transfer on test interface
> + 5) Check the test interface should still work
> +
> + @param test: KVM test object
> + @param params: Dictionary with the test parameters
> + @param env: Dictionary with test environment.
> + """
> + timeout = int(params.get("login_timeout", 360))
> + vm = kvm_test_utils.get_living_vm(env, params.get("main_vm"))
> + session = kvm_test_utils.wait_for_login(vm, timeout=timeout)
> + logging.info("Trying to log into guest '%s' by serial", vm.name)
> + session2 = kvm_utils.wait_for(lambda: vm.serial_login(),
> + timeout, 0, step=2)
> + if not session2:
> + raise error.TestFail("Could not log into guest '%s'" % vm.name)
> +
> + ethname = kvm_net_utils.get_linux_ifname(session, vm.get_macaddr(0))
> + try:
> + # FIXME: Try three waies to get nic driver name, because the
> + # modprobe.conf is dropped in latest system, and ethtool method is not
> + # supported by virtio_nic.
> +
> + output = session.get_command_output("cat /etc/modprobe.conf")
> + driver = re.findall(r'%s (\w+)' % ethname,output)
> + if not driver:
> + output = session.get_command_output("ethtool -i %s" % ethname)
> + driver = re.findall(r'driver: (\w+)', output)
> + if not driver:
> + output = session.get_command_output("lspci -k")
> + driver = re.findall("Ethernet controller.*\n.*\n.*Kernel driver"
> + " in use: (\w+)", output)
> + driver = driver[0]
> + except IndexError:
> + raise error.TestError("Could not find driver name")

^ About this whole block where there's an attempt to discover what the
driver name is. The methods listed there are not reliable (depends on
ethtool being installed, lspci -k not allways will list the kernel
driver in use, /etc/modprobe.conf will not be present on more recent
distributions). Instead of these methods, why don't we have a variant
for this test on each linux distro definition block, with the
appropriate driver names? It'd be something along the lines:

- 13.64:
image_name = f13-64
cdrom_cd1 = linux/Fedora-13-x86_64-DVD.iso
md5sum = 6fbae6379cf27f36e1f2c7827ba7dc35
md5sum_1m = 68821b9de4d3b5975d6634334e7f47a6
unattended_install:
unattended_file = unattended/Fedora-13.ks
tftp = images/f13-64/tftpboot
floppy = images/f13-64/floppy.img
nicdriver_unload:
e1000:
driver = e1000c
virtio:
driver = virtio_net
rtl8139:
driver = rtl8939

I believe it's safer than having to rely on the above methods.

> +
> + logging.info("driver is %s" % driver)
> +
> + class ThreadScp(threading.Thread):
> + def run(self):
> + remote_file = '/tmp/' + self.getName()
> + file_list.append(remote_file)
> + ret = vm.copy_files_to(file_name, remote_file, timeout=scp_timeout)
> + logging.debug("Copy result of %s: %s" % (remote_file, ret))

^ Here it'd be worth to have 2 debug messages,

if ret:
logging.debug("File %s was transfered successfuly", remote_file)
else:
logging.debug("Failed to transfer file %s", remote_file)

> + def compare(origin_file, receive_file):
> + cmd = "md5sum %s"
> + output1 = commands.getstatusoutput(cmd % origin_file)[1].strip()

^ If we are only interested on the output, getoutput() could be used.
But in this case we care whether md5 succeeded or not, so better to do
appropriate return code treatment, as you do below. Also, you could use
utils.hash() instead of using directly md5, and that *must* yield that
exact same result, being faster than resorting to subprocesses.

Also, remembering that I don't really love seeing things from the
commands API instead of utils.system, utils.run and others.

> + check_sum1 = output1.split()[0]
> + s, output2 = session.get_command_status_output(cmd % receive_file)
> + if s != 0:
> + logging.error("Could not get md5sum of receive_file")
> + return False
> + check_sum2 = output2.strip().split()[0]
> + logging.debug("origin: %s, receive: %s" % (check_sum1, check_sum2))

^ "original file md5: %s, received file md5: %s"

> + if check_sum1 != check_sum2:
> + logging.error("md5sum doesn't match")
> + return False
> + return True
> +
> + #produce sized file in host
> + file_size = params.get("file_size")
> + file_name = "/tmp/nicdriver_unload_file"
> + cmd = "dd if=/dev/urandom of=%s bs=%sM count=1"
> + s, o = commands.getstatusoutput(cmd % (file_name, file_size))
> + if s != 0:
> + raise error.TestFail("Fail to create file by dd")

Here utils.system would already raise a CmdError exception, so you could
just use it and not do exception handling

> + connect_time = params.get("connect_time")
> + scp_timeout = int(params.get("scp_timeout"))
> + thread_num = int(params.get("thread_num"))
> + file_list = []
> +
> + unload_load_cmd = "sleep %s && ifconfig %s down && modprobe -r %s && "
> + unload_load_cmd += "sleep 1 && modprobe %s && ifconfig %s up"
> + unload_load_cmd = unload_load_cmd % (connect_time, ethname, driver,
> + driver, ethname)

^ Here it'd be better to construct the whole command on a single assignment:

unload_load_cmd = ("sleep %s && ifconfig %s down && modprobe -r %s && "
"sleep 1 && modprobe %s && ifconfig %s up" %
(connect_time, ethname, driver, driver, ethname))

> + pid = os.fork()
> + if pid != 0:
> + logging.info("unload/load nic driver repeatedly in guest...")
> + while True:
> + logging.debug("Try to unload/load nic drive once")
> + if session2.get_command_status(unload_load_cmd, timeout=120) != 0:
> + session.get_command_output("rm -rf /tmp/Thread-*")
> + raise error.TestFail("Unload/load nic driver failed")
> + pid, s = os.waitpid(pid, os.WNOHANG)
> + status = os.WEXITSTATUS(s)
> + if (pid, status) != (0, 0):
> + logging.debug("Child process ending")
> + break
> + else:
> + logging.info("Multi-session tcp data transfer")
> + threads = []
> + for i in range(thread_num):
> + t = ThreadScp()
> + t.start()
> + threads.append(t)
> + for t in threads:
> + t.join(timeout = scp_timeout)
> + os._exit(0)
> +
> + session2.close()
> +
> + try:
> + logging.info("Check md5sum for received files in multi-session")
> + for f in file_list:
> + if not compare(file_name, f):
> + raise error.TestFail("Fail to compare (guest)file %s" % f)
> +
> + logging.info("Test nic function after load/unload")
> + if not vm.copy_files_to(file_name, file_name):
> + raise error.TestFail("Fail to copy file from host to guest")
> + if not compare(file_name, file_name):
> + raise error.TestFail("Test nic function after load/unload fail")
> +
> + finally:
> + session.get_command_output("rm -rf /tmp/Thread-*")
> + session.close()
> diff --git a/client/tests/kvm/tests_base.cfg.sample b/client/tests/kvm/tests_base.cfg.sample
> index 872674e..03d15c0 100644
> --- a/client/tests/kvm/tests_base.cfg.sample
> +++ b/client/tests/kvm/tests_base.cfg.sample
> @@ -362,6 +362,14 @@ variants:
> filesize = 4000
> scp_timeout = 1000
>
> + - nicdriver_unload: install setup unattended_install.cdrom
> + type = nicdriver_unload
> + nic_mode = tap
> + file_size = 100
> + connect_time = 4
> + scp_timeout = 300
> + thread_num = 10
> +
> - physical_resources_check: install setup unattended_install.cdrom
> type = physical_resources_check
> catch_uuid_cmd = dmidecode | awk -F: '/UUID/ {print $2}'
> @@ -1038,7 +1046,7 @@ variants:
>
> # Windows section
> - @Windows:
> - no autotest linux_s3 vlan_tag ioquit unattended_install.(url|nfs|remote_ks) jumbo file_transfer
> + no autotest linux_s3 vlan_tag ioquit unattended_install.(url|nfs|remote_ks) jumbo file_transfer nicdriver_unload
> shutdown_command = shutdown /s /f /t 0
> reboot_command = shutdown /r /f /t 0
> status_test_command = echo %errorlevel%
>
>