Re: [Autotest] [PATCH] Add a subtest pci_hotplug in kvm test

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

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?
It doesn't need to modprobe acpiphp on Fedora, also, both F8 and F9 don't have virtio & virtio_ring modules. I ran it on guest Fedora-11 and Win2008, both ran successfully:

# ./scan_results.py
test status seconds info ---- ------ ------- ---- Fedora.11.32.nic_hotplug.nic_rtl8139 GOOD 68 completed successfully Fedora.11.32.nic_hotplug.nic_virtio GOOD 46 completed successfully Fedora.11.32.block_hotplug.fmt_qcow2.block_virtio GOOD 46 completed successfully Fedora.11.32.block_hotplug.fmt_qcow2.block_scsi GOOD 44 completed successfully Fedora.11.32.block_hotplug.fmt_raw.block_virtio GOOD 45 completed successfully Fedora.11.32.block_hotplug.fmt_raw.block_scsi GOOD 46 completed successfully Win2008.32.nic_hotplug.nic_rtl8139 GOOD 66 completed successfully Win2008.32.block_hotplug.fmt_qcow2.block_scsi GOOD 186 completed successfully Win2008.32.block_hotplug.fmt_raw.block_scsi GOOD 71 completed successfully

+        pci_test_cmd = 'nslookupwww.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
Had worked out how to verify the newly added block device. Will send it here right now. Any suggestion about how to verify newly added nic card ? So far I just keep
 'nslookup www.redhat.com' to check network simply.
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)?

It did actually not verify the newly added device but simply verify if pci_del bring in any problem to the guest (disk or network). Since sometimes I found pci_del a nic card can break the guest's network as tested in our internal kvm version.

+    # 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

[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux