On 07/08/2009 09:51 AM, Lucas Meneghel Rodrigues wrote:
I've spent some time doing a second review and test of the code.
During my tests:
* I found some problems with PCI hotplug itself and would like help
to figure out why things are not working as expected.
* Made suggestions regarding the phrasing of the error messages
thrown by the test. Mostly nipticking. Let me know if you think the
new messages make sense.
* The order of the final test steps looks a bit weird to me
Comments follow.
Lucas, sorry for reply you a little late since I left two days. :)
On Fri, Jul 3, 2009 at 3:00 AM, Yolkfull Chow<yzhou@xxxxxxxxxx> wrote:
On 07/03/2009 01:57 PM, Yolkfull Chow wrote:
Signed-off-by: Yolkfull Chow<yzhou@xxxxxxxxxx>
---
client/tests/kvm/kvm.py | 1 +
client/tests/kvm/kvm_tests.cfg.sample | 65 ++++++++++++++++++++++-
client/tests/kvm/kvm_tests.py | 94 +++++++++++++++++++++++++++++++++
client/tests/kvm/kvm_vm.py | 2 +
4 files changed, 161 insertions(+), 1 deletions(-)
diff --git a/client/tests/kvm/kvm.py b/client/tests/kvm/kvm.py
index b18b643..fc92e10 100644
--- a/client/tests/kvm/kvm.py
+++ b/client/tests/kvm/kvm.py
@@ -55,6 +55,7 @@ class kvm(test.test):
"kvm_install": test_routine("kvm_install", "run_kvm_install"),
"linux_s3": test_routine("kvm_tests", "run_linux_s3"),
"stress_boot": test_routine("kvm_tests", "run_stress_boot"),
+ "pci_hotplug": test_routine("kvm_tests", "run_pci_hotplug"),
}
# Make it possible to import modules from the test's bindir
diff --git a/client/tests/kvm/kvm_tests.cfg.sample b/client/tests/kvm/kvm_tests.cfg.sample
index 2f864de..a9e16d6 100644
--- a/client/tests/kvm/kvm_tests.cfg.sample
+++ b/client/tests/kvm/kvm_tests.cfg.sample
@@ -94,6 +94,53 @@ variants:
max_vms = 5
alive_test_cmd = ps aux
+
+ - nic_hotplug:
+ type = pci_hotplug
+ pci_type = nic
+ modprobe_acpiphp = yes
+ reference_cmd = lspci
+ find_pci_cmd = 'lspci | tail -n1'
I tried block device hotplug, lspci doesn't show up the newly added
devices. Already tried with F8 and F9. Any idea why?
Strange, it could be run successfully with guest RHEL.5.3.i386 and
Win2008.32(use telnet login), but I never run it on Fedora. Jason
reminded me several days ago that there is no 'acpiphp' module on Fedora
which may be related. Anyway, I will fix it.
+ pci_test_cmd = 'nslookup www.redhat.com'
+ seconds_wait_for_device_install = 3
+ variants:
+ - @nic_8139:
+ pci_model = rtl8139
+ match_string = "8139"
+ - nic_virtio:
+ pci_model = virtio
+ match_string = "Virtio network device"
+ - nic_e1000:
+ pci_model = e1000
+ match_string = "Gigabit Ethernet Controller"
Pretty much all block hotplug 'guest side check' is failing during the
stage where the output of lspci | tail -n1 is being compared with the
match strings. Hypervisor is qemu 0.10.5 (kvm-87 upstream).
+ - block_hotplug:
+ type = pci_hotplug
+ pci_type = block
+ modprobe_acpiphp = yes
+ reference_cmd = lspci
+ find_pci_cmd = 'lspci | tail -n1'
+ images += " stg"
+ boot_drive_stg = no
+ image_name_stg = storage
+ image_size = 1G
+ force_create_image_stg = yes
+ pci_test_cmd = 'dir'
+ seconds_wait_for_device_install = 3
+ variants:
+ - block_virtio:
+ pci_model = virtio
+ match_string = "Virtio block device"
+ - block_scsi:
+ pci_model = scsi
+ match_string = "SCSI storage controller"
+ variants:
+ - fmt_qcow2:
+ image_format_stg = qcow2
+ - fmt_raw:
+ image_format_stg = raw
+
+
# NICs
variants:
- @rtl8139:
@@ -306,6 +353,22 @@ variants:
migration_test_command = ver&& vol
stress_boot:
alive_test_cmd = systeminfo
+ nic_hotplug:
+ modprobe_acpiphp = no
+ reference_cmd = systeminfo
+ seconds_wait_for_device_install = 10
+ find_pci_cmd = ipconfig /all | find "Description"
+ nic_e1000:
+ match_string = "Intel(R) PRO/1000 MT Network Connection"
+ block_hotplug:
+ use_telnet = yes
+ modprobe_acpiphp = no
+ reference_cmd = wmic diskdrive
+ find_pci_cmd = wmic diskdrive | find "disk drives"
+ seconds_wait_for_device_install = 10
+ only block_scsi
+ block_scsi:
+ match_string = "SCSI"
It supports Windows block_hotplug now. But we need use_telnet to login
Windows OS since command 'wmic' could only run on telnet session.
variants:
- Win2000:
@@ -571,4 +634,4 @@ variants:
only rtl8139
# Choose your test list
-only fc8_quick
+#only fc8_quick
diff --git a/client/tests/kvm/kvm_tests.py b/client/tests/kvm/kvm_tests.py
index 2d11fed..230385a 100644
--- a/client/tests/kvm/kvm_tests.py
+++ b/client/tests/kvm/kvm_tests.py
@@ -585,3 +585,97 @@ def run_stress_boot(tests, params, env):
for se in sessions:
se.close()
logging.info("Total number booted: %d" % (num -1))
+
+
+def run_pci_hotplug(test, params, env):
+ """
+ Test pci devices' hotplug
+ 1) pci_add a deivce (nic or storage)
+ 2) Compare 'info pci' output
+ 3) Compare $reference_cmd output
+ 4) Verify whether pci_model is shown in $pci_find_cmd
+ 5) pci_del the device, verify whether could remove the pci device
What about if the last steps were
5) Verify if the newly added devices are working properly
6) pci_del the device, verify whether could remove the pci device
6) is already happening in the code, but in reverse order than mentioned above.
+ @param test: kvm test object
+ @param params: Dictionary with the test parameters
+ @param env: Dictionary with test environment.
+ """
+ vm = kvm_utils.env_get_vm(env, params.get("main_vm"))
+ if not vm:
+ raise error.TestError("VM object not found in environment")
+ if not vm.is_alive():
+ raise error.TestError("VM seems to be dead; Test requires a living VM")
+
+ logging.info("Waiting for guest to be up...")
+
+ session = kvm_utils.wait_for(vm.ssh_login, 240, 0, 2)
+ if not session:
+ raise error.TestFail("Could not log into guest")
+
+ logging.info("Logged in")
+
+ # modprobe the module that enable hotplug
+ if params.get("modprobe_acpiphp") == "yes":
+ if session.get_command_status("modprobe acpiphp"):
+ raise error.TestError("Modprobe module 'acpiphp' failed")
+
+ # get reference output
+ s, info_pci_ref = vm.send_monitor_cmd("info pci")
+
+ # compare the output of `reference_cmd`
+ ref_cmd = params.get("reference_cmd")
+ reference = session.get_command_output(ref_cmd)
+
+ # implement pci hotplug
+ tested_model = params.get("pci_model")
+ logging.info("Testing hotplug pci device:%s" % tested_model)
+
+ test_type = params.get("pci_type")
+ if test_type == "nic":
+ pci_add_cmd = "pci_add pci_addr=auto nic model=%s" % tested_model
+ s, add_output = vm.send_monitor_cmd(pci_add_cmd)
The command
s, add_output = vm.send_monitor_cmd(pci_add_cmd)
Can be moved out the if blocks since it will be executed in both
paths, we save one line of code with this (and it's cleaner anyway).
+ elif test_type == "block":
+ image_name = params.get("image_name_stg")
+ image_format = params.get("image_format_stg", "qcow2")
+ image_filename = "%s.%s" % (image_name, image_format)
+ image_dir = os.path.join(test.bindir, "images")
+ storage_name = os.path.join(image_dir, image_filename)
+ pci_add_cmd = "pci_add pci_addr=auto storage file=%s,if=%s" % \
+ (storage_name, tested_model)
The above line would be better with implicit line continuation
pci_add_cmd = ("pci_add pci_addr=auto storage file=%s,if=%s" %
(storage_name, tested_model))
+ s, add_output = vm.send_monitor_cmd(pci_add_cmd)
+
+ if not "OK domain" in add_output:
+ raise error.TestFail("Command failed: %s" % pci_add_cmd)
Since we're executing a command on the hypervisor to add the device, a
better way to put the above message would be
if not "OK domain" in add_output:
raise error.TestFail("Hypervisor command to add device '%s'failed."
"Output: %s" % (pci_add_cmd, add_output))
+ # compare the output of 'info pci'
+ s, after_add = vm.send_monitor_cmd("info pci")
+ if after_add == info_pci_ref:
+ raise error.TestFail("No new pci device shown in 'info pci'")
Another suggestion:
raise error.TestFail("No new PCI device shown after executing "
"hypervisor command 'info pci'")
+ time.sleep(int(params.get("seconds_wait_for_device_install")))
+
+ o = session.get_command_output(ref_cmd)
+ if reference == o:
+ raise error.TestFail("No new device shown in cmd: %s" % ref_cmd)
I found useful to explain on which side of the check the test fail,
'hypervisor', or 'guest'
raise error.TestFail("No new device shown on guest command %s output" %
ref_cmd)
+ cmd = params.get("find_pci_cmd")
+ output = session.get_command_output(cmd)
+ if not params.get("match_string") in output:
+ raise error.TestFail("Not found pci model: %s; Command is: %s" %
+ (tested_model, cmd))
raise error.TestFail("PCI device model '%s' not found on host "
"command '%s' ouput" % (tested_model, cmd))
+ # del pci device
+ slot_id = "0" + add_output.split(",")[2].split()[1]
+ cmd = "pci_del pci_addr=%s" % slot_id
+ s, after_del = vm.send_monitor_cmd(cmd)
+ if after_del == after_add:
+ raise error.TestFail("Failed to hot remove pci device:%s; Command:%s" %
+ (tested_model, cmd))
raise error.TestFail("Failed to hot remove pci device '%s'. "
"Hypervisor command: '%s'" % (tested_model, cmd))
Hot remove doesn't work for me either... Since we are here, I still
didn't understand why we do a hot remove of a device and right below
it we try to verify if the newly added devices are working properly.
Shouldn't be the other way around (ie checking and then hot removing)?
+ # check whether VM's network& disk work fine
+ if session.get_command_status(params.get("pci_test_cmd")):
+ raise error.TestFail("Check for %s device failed after PCI hotplug" %
+ test_type)
+
+ session.close()
diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 503f636..95b55eb 100644
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -239,6 +239,8 @@ class VM:
for image_name in kvm_utils.get_sub_dict_names(params, "images"):
image_params = kvm_utils.get_sub_dict(params, image_name)
+ if image_params.get("boot_drive") == "no":
+ continue
qemu_cmd += " -drive file=%s" % get_image_filename(image_params,
image_dir)
if image_params.get("drive_format"):
--
Yolkfull
Regards,
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
