All of lore.kernel.org
 help / color / mirror / Atom feed
* [KVM-AUTOTEST PATCH 1/7] KVM test: make all tests depend on install, setup & unattended_install
@ 2009-11-05 10:01 Michael Goldish
  2009-11-05 10:01 ` [KVM-AUTOTEST PATCH 2/7] KVM test: kvm_vm.py: use kvm_utils.get_path() for floppy and tftp Michael Goldish
  0 siblings, 1 reply; 9+ messages in thread
From: Michael Goldish @ 2009-11-05 10:01 UTC (permalink / raw)
  To: autotest, kvm; +Cc: Michael Goldish

The dependency is effective only if these tests are executed.  For example,
if unattended_install is not selected to run, the tests depending on it will
run normally.  Tests will be skipped only if unattended_install runs and fails.

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
 client/tests/kvm/kvm_tests.cfg.sample |   31 ++++++++++++++++---------------
 1 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/client/tests/kvm/kvm_tests.cfg.sample b/client/tests/kvm/kvm_tests.cfg.sample
index bcc31bb..f271a09 100644
--- a/client/tests/kvm/kvm_tests.cfg.sample
+++ b/client/tests/kvm/kvm_tests.cfg.sample
@@ -58,23 +58,23 @@ variants:
         floppy = "images/floppy.img"
         extra_params += " -boot d"
 
-    - setup:        install
+    - setup:        install unattended_install
         type = steps
         fail_if_stuck_for = 300
         stuck_detection_history = 2
         kill_vm_on_error = yes
         keep_screendump_history = yes
 
-    - boot:         install setup
+    - boot:         install setup unattended_install
         type = boot
         kill_vm_on_error = yes
 
-    - reboot:       install setup
+    - reboot:       install setup unattended_install
         type = boot
         reboot_method = shell
         kill_vm_on_error = yes
 
-    - migrate:      install setup
+    - migrate:      install setup unattended_install
         type = migration
         migration_test_command = help
         migration_bg_command = "cd /tmp; nohup tcpdump -q -t ip host localhost"
@@ -84,7 +84,7 @@ variants:
         iterations = 2
         used_mem = 1024
 
-    - autotest:     install setup
+    - autotest:     install setup unattended_install
         type = autotest
         test_timeout = 720
         variants:
@@ -118,10 +118,10 @@ variants:
                 test_control_file = npb.control
 
 
-    - linux_s3:     install setup
+    - linux_s3:     install setup unattended_install
         type = linux_s3
 
-    - timedrift:    install setup
+    - timedrift:    install setup unattended_install
         extra_params += " -rtc-td-hack"
         variants:
             - with_load:
@@ -148,7 +148,7 @@ variants:
                 drift_threshold = 10
                 drift_threshold_single = 3
 
-    - stress_boot:  install setup
+    - stress_boot:  install setup unattended_install
         type = stress_boot
         max_vms = 5    
         alive_test_cmd = uname -a
@@ -160,7 +160,7 @@ variants:
         used_cpus = 5
         used_mem = 2560
 
-    - autoit:       install setup
+    - autoit:       install setup unattended_install
         type = autoit
         autoit_binary = D:\AutoIt3.exe
         autoit_script_timeout = 600
@@ -169,7 +169,7 @@ variants:
             - notepad:
                 autoit_script = autoit/notepad1.au3
 
-    - guest_s4:
+    - guest_s4:     install setup unattended_install
         type = guest_s4
         check_s4_support_cmd = grep -q disk /sys/power/state
         test_s4_cmd = "cd /tmp; nohup tcpdump -q -t ip host localhost"
@@ -178,7 +178,7 @@ variants:
         kill_test_s4_cmd = pkill tcpdump
         services_up_timeout = 30
 
-    - nic_hotplug:       install setup
+    - nic_hotplug:  install setup unattended_install
         type = pci_hotplug
         pci_type = nic
         modprobe_acpiphp = no
@@ -197,7 +197,7 @@ variants:
                 pci_model = e1000
                 match_string = "Gigabit Ethernet Controller"
 
-    - block_hotplug:       install setup
+    - block_hotplug: install setup unattended_install
         type = pci_hotplug
         pci_type = block
         reference_cmd = lspci
@@ -225,25 +225,26 @@ variants:
                 image_format_stg = raw
                 only Fedora Ubuntu Windows
 
-    - system_reset: install setup
+    - system_reset: install setup unattended_install
         type = boot
         reboot_method = system_reset
         sleep_before_reset = 20
         kill_vm_on_error = yes
 
-    - system_powerdown: install setup
+    - system_powerdown: install setup unattended_install
         type = shutdown
         shutdown_method = system_powerdown
         sleep_before_powerdown = 20
         kill_vm = yes
         kill_vm_gracefully = no
 
-    - shutdown:     install setup
+    - shutdown:     install setup unattended_install
         type = shutdown
         shutdown_method = shell
         kill_vm = yes
         kill_vm_gracefully = no
 
+
 # NICs
 variants:
     - @rtl8139:
-- 
1.5.4.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [KVM-AUTOTEST PATCH 2/7] KVM test: kvm_vm.py: use kvm_utils.get_path() for floppy and tftp
  2009-11-05 10:01 [KVM-AUTOTEST PATCH 1/7] KVM test: make all tests depend on install, setup & unattended_install Michael Goldish
@ 2009-11-05 10:01 ` Michael Goldish
  2009-11-05 10:01   ` [KVM-AUTOTEST PATCH 3/7] KVM test: kvm_vm.py: use 'is None' instead of '== None' Michael Goldish
  0 siblings, 1 reply; 9+ messages in thread
From: Michael Goldish @ 2009-11-05 10:01 UTC (permalink / raw)
  To: autotest, kvm; +Cc: Michael Goldish

kvm_utils.get_path() allows using absolute as well as relative paths.

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
 client/tests/kvm/kvm_vm.py |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index ee6796b..e1f5cad 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -270,12 +270,12 @@ class VM:
         # {fat:floppy:}/path/. However vvfat is not usually recommended
         floppy = params.get("floppy")
         if floppy:
-            floppy = os.path.join(root_dir, floppy)
+            floppy = kvm_utils.get_path(root_dir, floppy)
             qemu_cmd += " -fda %s" % floppy
 
         tftp = params.get("tftp")
         if tftp:
-            tftp = os.path.join(root_dir, tftp)
+            tftp = kvm_utils.get_path(root_dir, tftp)
             qemu_cmd += " -tftp %s" % tftp
 
         extra_params = params.get("extra_params")
-- 
1.5.4.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [KVM-AUTOTEST PATCH 3/7] KVM test: kvm_vm.py: use 'is None' instead of '== None'
  2009-11-05 10:01 ` [KVM-AUTOTEST PATCH 2/7] KVM test: kvm_vm.py: use kvm_utils.get_path() for floppy and tftp Michael Goldish
@ 2009-11-05 10:01   ` Michael Goldish
  2009-11-05 10:01     ` [KVM-AUTOTEST PATCH 4/7] KVM test: kvm_vm.py: fix typos in the code that compares -cdrom hashes Michael Goldish
  0 siblings, 1 reply; 9+ messages in thread
From: Michael Goldish @ 2009-11-05 10:01 UTC (permalink / raw)
  To: autotest, kvm; +Cc: Michael Goldish

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
 client/tests/kvm/kvm_vm.py |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index e1f5cad..ccf8fa9 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -143,13 +143,13 @@ class VM:
         @param root_dir: Optional new base directory for relative filenames
         @param address_cache: A dict that maps MAC addresses to IP addresses
         """
-        if name == None:
+        if name is None:
             name = self.name
-        if params == None:
+        if params is None:
             params = self.params.copy()
-        if root_dir == None:
+        if root_dir is None:
             root_dir = self.root_dir
-        if address_cache == None:
+        if address_cache is None:
             address_cache = self.address_cache
         return VM(name, params, root_dir, address_cache)
 
@@ -190,11 +190,11 @@ class VM:
                nic_model -- string to pass as 'model' parameter for this
                NIC (e.g. e1000)
         """
-        if name == None:
+        if name is None:
             name = self.name
-        if params == None:
+        if params is None:
             params = self.params
-        if root_dir == None:
+        if root_dir is None:
             root_dir = self.root_dir
 
         # Start constructing the qemu command
@@ -320,11 +320,11 @@ class VM:
         """
         self.destroy()
 
-        if name != None:
+        if name is not None:
             self.name = name
-        if params != None:
+        if params is not None:
             self.params = params
-        if root_dir != None:
+        if root_dir is not None:
             self.root_dir = root_dir
         name = self.name
         params = self.params
-- 
1.5.4.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [KVM-AUTOTEST PATCH 4/7] KVM test: kvm_vm.py: fix typos in the code that compares -cdrom hashes
  2009-11-05 10:01   ` [KVM-AUTOTEST PATCH 3/7] KVM test: kvm_vm.py: use 'is None' instead of '== None' Michael Goldish
@ 2009-11-05 10:01     ` Michael Goldish
  2009-11-05 10:01       ` [KVM-AUTOTEST PATCH 5/7] KVM test: minor pci_hotplug fixes Michael Goldish
  0 siblings, 1 reply; 9+ messages in thread
From: Michael Goldish @ 2009-11-05 10:01 UTC (permalink / raw)
  To: autotest, kvm; +Cc: Michael Goldish

Replace md5sum_file() with hash_file() and 'md5sum' with 'sha1sum' where
appropriate.

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
 client/tests/kvm/kvm_vm.py |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index ccf8fa9..5781dbc 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -347,14 +347,14 @@ class VM:
             elif params.get("md5sum"):
                 logging.debug("Comparing expected MD5 sum with MD5 sum of ISO "
                               "file...")
-                actual_hash = kvm_utils.md5sum_file(iso, method="md5")
+                actual_hash = kvm_utils.hash_file(iso, method="md5")
                 expected_hash = params.get("md5sum")
                 compare = True
             elif params.get("sha1sum"):
                 logging.debug("Comparing expected SHA1 sum with SHA1 sum of "
                               "ISO file...")
-                actual_hash = kvm_utils.md5sum_file(iso, method="sha1")
-                expected_hash = params.get("md5sum")
+                actual_hash = kvm_utils.hash_file(iso, method="sha1")
+                expected_hash = params.get("sha1sum")
                 compare = True
             if compare:
                 if actual_hash == expected_hash:
-- 
1.5.4.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [KVM-AUTOTEST PATCH 5/7] KVM test: minor pci_hotplug fixes
  2009-11-05 10:01     ` [KVM-AUTOTEST PATCH 4/7] KVM test: kvm_vm.py: fix typos in the code that compares -cdrom hashes Michael Goldish
@ 2009-11-05 10:01       ` Michael Goldish
  2009-11-05 10:01         ` [KVM-AUTOTEST PATCH 6/7] KVM test: kvm_config.py: match(): put filter inside parentheses Michael Goldish
  2009-11-06  3:08         ` [Autotest] [KVM-AUTOTEST PATCH 5/7] KVM test: minor pci_hotplug fixes Yolkfull Chow
  0 siblings, 2 replies; 9+ messages in thread
From: Michael Goldish @ 2009-11-05 10:01 UTC (permalink / raw)
  To: autotest, kvm; +Cc: Michael Goldish

- Put the PCI device removal code in a finally clause.
- Use kvm_vm.get_image_filename() instead of os.path.join().
It's a bit cleaner because if we ever change the names of image parameters
we'll only have to change the code in one place.
Also, the way os.path.join() was used lead to image filenames being prefixed
with 'images/' twice, e.g. 'images/images/foo.qcow2'.
- Make some failure messages clearer.
- Remove 'only Fedora Ubuntu Windows' from the fmt_raw variant.
'only' works for things that have already been defined, but the guests are
defined later.
- Remove unused 'modprobe_acpiphp' parameter.
- Change 'online disk' to 'online' in pci_test_cmd for Windows ('online disk'
doesn't seem to work).
- Remove the unneeded telnet/ssh/guest_port parameters from the Windows
block_hotplug parameters.

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
 client/tests/kvm/kvm_tests.cfg.sample |    7 +--
 client/tests/kvm/tests/pci_hotplug.py |  112 +++++++++++++++++----------------
 2 files changed, 58 insertions(+), 61 deletions(-)

diff --git a/client/tests/kvm/kvm_tests.cfg.sample b/client/tests/kvm/kvm_tests.cfg.sample
index f271a09..326ae20 100644
--- a/client/tests/kvm/kvm_tests.cfg.sample
+++ b/client/tests/kvm/kvm_tests.cfg.sample
@@ -181,7 +181,6 @@ variants:
     - nic_hotplug:  install setup unattended_install
         type = pci_hotplug
         pci_type = nic
-        modprobe_acpiphp = no
         reference_cmd = lspci
         find_pci_cmd = 'lspci | tail -n1'
         pci_test_cmd = 'nslookup www.redhat.com'
@@ -223,7 +222,6 @@ variants:
                 image_format_stg = qcow2
             - fmt_raw:
                 image_format_stg = raw
-                only Fedora Ubuntu Windows
 
     - system_reset: install setup unattended_install
         type = boot
@@ -538,13 +536,10 @@ variants:
             nic_virtio:
                 match_string = "VirtIO Ethernet"
         block_hotplug:
-            use_telnet = yes
-            ssh_port = 23
-            guest_port_ssh = 23
             wait_secs_for_hook_up = 10
             reference_cmd = wmic diskdrive list brief
             find_pci_cmd = wmic diskdrive list brief
-            pci_test_cmd = echo select disk 1 > dt && echo online disk >> dt && echo detail disk >> dt && echo exit >> dt && diskpart /s dt
+            pci_test_cmd = echo select disk 1 > dt && echo online >> dt && echo detail disk >> dt && echo exit >> dt && diskpart /s dt
 
         variants:
             - Win2000:
diff --git a/client/tests/kvm/tests/pci_hotplug.py b/client/tests/kvm/tests/pci_hotplug.py
index 3ad9ea2..876d8b8 100644
--- a/client/tests/kvm/tests/pci_hotplug.py
+++ b/client/tests/kvm/tests/pci_hotplug.py
@@ -1,6 +1,6 @@
 import logging, os
 from autotest_lib.client.common_lib import error
-import kvm_subprocess, kvm_test_utils, kvm_utils
+import kvm_subprocess, kvm_test_utils, kvm_utils, kvm_vm
 
 
 def run_pci_hotplug(test, params, env):
@@ -21,8 +21,8 @@ def run_pci_hotplug(test, params, env):
     session = kvm_test_utils.wait_for_login(vm)
 
     # Modprobe the module if specified in config file
-    if params.get("modprobe_module"):
-        module = params.get("modprobe_module")
+    module = params.get("modprobe_module")
+    if module:
         if session.get_command_status("modprobe %s" % module):
             raise error.TestError("Modprobe module '%s' failed" % module)
 
@@ -38,61 +38,63 @@ def run_pci_hotplug(test, params, env):
     if test_type == "nic":
         pci_add_cmd = "pci_add pci_addr=auto nic model=%s" % tested_model
     elif test_type == "block":
-        image_name = params.get("image_name_stg")
-        image_filename = "%s.%s" % (image_name, params.get("image_format_stg"))
-        image_dir = os.path.join(test.bindir, "images")
-        storage_name = os.path.join(image_dir, image_filename)
+        image_params = kvm_utils.get_sub_dict(params, "stg")
+        image_filename = kvm_vm.get_image_filename(image_params, test.bindir)
         pci_add_cmd = ("pci_add pci_addr=auto storage file=%s,if=%s" %
-                                    (storage_name, tested_model))
+                                    (image_filename, tested_model))
 
     # Implement pci_add
     s, add_output = vm.send_monitor_cmd(pci_add_cmd)
     if not "OK domain" in add_output:
         raise error.TestFail("Add device failed. Hypervisor command is: %s. "
-                             "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 after executing "
-                             "hypervisor command: 'info pci'")
-
-    # Define a helper function to compare the output
-    def new_shown():
-        o = session.get_command_output(params.get("reference_cmd"))
-        if reference == o:
-            return False
-        return True
-
-    secs = int(params.get("wait_secs_for_hook_up"))
-    if not kvm_utils.wait_for(new_shown, 30, secs, 3):
-        raise error.TestFail("No new device shown in output of command "
-                             "executed inside the guest: %s" %
-                             params.get("reference_cmd"))
-
-    # Define a helper function to catch PCI device string
-    def find_pci():
-        output = session.get_command_output(params.get("find_pci_cmd"))
-        if not params.get("match_string") in output:
-            return False
-        return True
-
-    if not kvm_utils.wait_for(find_pci, 30, 3, 3):
-        raise error.TestFail("PCI model not found: %s. Command is: %s" %
-                             (tested_model, params.get("find_pci_cmd")))
-
-    # Test the newly added device
-    s, o = session.get_command_status_output(params.get("pci_test_cmd"))
-    if s != 0:
-        raise error.TestFail("Check for %s device failed after PCI hotplug. "
-                             "Output: %s" % (test_type, o))
-
-    # Delete the added 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. "
-                             "Hypervisor command: %s" % (tested_model, cmd))
-
-    session.close()
+                             "Output: %r" % (pci_add_cmd, add_output))
+
+    try:
+        # 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 after executing "
+                                 "hypervisor command: 'info pci'")
+
+        # Define a helper function to compare the output
+        def new_shown():
+            o = session.get_command_output(params.get("reference_cmd"))
+            if reference == o:
+                return False
+            return True
+
+        secs = int(params.get("wait_secs_for_hook_up"))
+        if not kvm_utils.wait_for(new_shown, 30, secs, 3):
+            raise error.TestFail("No new device shown in output of command "
+                                 "executed inside the guest: %s" %
+                                 params.get("reference_cmd"))
+
+        # Define a helper function to catch PCI device string
+        def find_pci():
+            output = session.get_command_output(params.get("find_pci_cmd"))
+            if not params.get("match_string") in output:
+                return False
+            return True
+
+        if not kvm_utils.wait_for(find_pci, 30, 3, 3):
+            raise error.TestFail("PCI %s %s device not found in guest. "
+                                 "Command was: %s" %
+                                 (tested_model, test_type,
+                                  params.get("find_pci_cmd")))
+
+        # Test the newly added device
+        s, o = session.get_command_status_output(params.get("pci_test_cmd"))
+        if s != 0:
+            raise error.TestFail("Check for %s device failed after PCI "
+                                 "hotplug. Output: %r" % (test_type, o))
+
+    finally:
+        # Delete the added 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. "
+                                 "Hypervisor command: %s" % (tested_model,
+                                                             cmd))
+        session.close()
-- 
1.5.4.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [KVM-AUTOTEST PATCH 6/7] KVM test: kvm_config.py: match(): put filter inside parentheses
  2009-11-05 10:01       ` [KVM-AUTOTEST PATCH 5/7] KVM test: minor pci_hotplug fixes Michael Goldish
@ 2009-11-05 10:01         ` Michael Goldish
  2009-11-05 10:01           ` [KVM-AUTOTEST PATCH 7/7] KVM test: remove monitor socket file when destroying a VM Michael Goldish
  2009-11-06  3:08         ` [Autotest] [KVM-AUTOTEST PATCH 5/7] KVM test: minor pci_hotplug fixes Yolkfull Chow
  1 sibling, 1 reply; 9+ messages in thread
From: Michael Goldish @ 2009-11-05 10:01 UTC (permalink / raw)
  To: autotest, kvm; +Cc: Michael Goldish

This solves problems due to operator precedence (such as virtio|e1000 matching
nic_e1000 even though it shouldn't).

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
 client/tests/kvm/kvm_config.py |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/client/tests/kvm/kvm_config.py b/client/tests/kvm/kvm_config.py
index 52de4c7..649f2c3 100755
--- a/client/tests/kvm/kvm_config.py
+++ b/client/tests/kvm/kvm_config.py
@@ -83,7 +83,7 @@ class config:
         @param filter: A regular expression that defines the filter.
         @param dict: Dictionary that will be inspected.
         """
-        filter = re.compile(r"(\.|^)" + filter + r"(\.|$)")
+        filter = re.compile(r"(\.|^)(%s)(\.|$)" % filter)
         return bool(filter.search(dict["name"]))
 
 
-- 
1.5.4.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [KVM-AUTOTEST PATCH 7/7] KVM test: remove monitor socket file when destroying a VM
  2009-11-05 10:01         ` [KVM-AUTOTEST PATCH 6/7] KVM test: kvm_config.py: match(): put filter inside parentheses Michael Goldish
@ 2009-11-05 10:01           ` Michael Goldish
  2009-11-06  3:14             ` [Autotest] " Yolkfull Chow
  0 siblings, 1 reply; 9+ messages in thread
From: Michael Goldish @ 2009-11-05 10:01 UTC (permalink / raw)
  To: autotest, kvm; +Cc: Michael Goldish

This should slow the rate of accumulation of monitor files in /tmp.

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
 client/tests/kvm/kvm_vm.py |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 5781dbc..62a10b9 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -578,6 +578,10 @@ class VM:
         finally:
             if self.process:
                 self.process.close()
+            try:
+                os.unlink(self.monitor_file_name)
+            except OSError:
+                pass
 
 
     def is_alive(self):
-- 
1.5.4.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [Autotest] [KVM-AUTOTEST PATCH 5/7] KVM test: minor pci_hotplug fixes
  2009-11-05 10:01       ` [KVM-AUTOTEST PATCH 5/7] KVM test: minor pci_hotplug fixes Michael Goldish
  2009-11-05 10:01         ` [KVM-AUTOTEST PATCH 6/7] KVM test: kvm_config.py: match(): put filter inside parentheses Michael Goldish
@ 2009-11-06  3:08         ` Yolkfull Chow
  1 sibling, 0 replies; 9+ messages in thread
From: Yolkfull Chow @ 2009-11-06  3:08 UTC (permalink / raw)
  To: Michael Goldish; +Cc: autotest, kvm

On Thu, Nov 05, 2009 at 12:01:10PM +0200, Michael Goldish wrote:
> - Put the PCI device removal code in a finally clause.

Hi Michael,

I have a little concern with the removal procedure. Thinking about if
pci_add failed, the output will not contain right information including PCI
ID. The slice operation during removing therefore could involve traceback
and removal will be failed at the end. Thus I would not place it in a finally
clause at that time.

But looking from opposite side, if we don't place it in a finally
clause and, pci_add is succeeded whereas verification is failed, the
device will not be removed finally. 

We may need to balance both if possible. Do you have any idea about
this? I can agree on applying the method proposed by this patch first.

> - Use kvm_vm.get_image_filename() instead of os.path.join().
> It's a bit cleaner because if we ever change the names of image parameters
> we'll only have to change the code in one place.
> Also, the way os.path.join() was used lead to image filenames being prefixed
> with 'images/' twice, e.g. 'images/images/foo.qcow2'.
> - Make some failure messages clearer.
> - Remove 'only Fedora Ubuntu Windows' from the fmt_raw variant.
> 'only' works for things that have already been defined, but the guests are
> defined later.
> - Remove unused 'modprobe_acpiphp' parameter.
> - Change 'online disk' to 'online' in pci_test_cmd for Windows ('online disk'
> doesn't seem to work).
> - Remove the unneeded telnet/ssh/guest_port parameters from the Windows
> block_hotplug parameters.
> 
> Signed-off-by: Michael Goldish <mgoldish@redhat.com>
> ---
>  client/tests/kvm/kvm_tests.cfg.sample |    7 +--
>  client/tests/kvm/tests/pci_hotplug.py |  112 +++++++++++++++++----------------
>  2 files changed, 58 insertions(+), 61 deletions(-)
> 
> diff --git a/client/tests/kvm/kvm_tests.cfg.sample b/client/tests/kvm/kvm_tests.cfg.sample
> index f271a09..326ae20 100644
> --- a/client/tests/kvm/kvm_tests.cfg.sample
> +++ b/client/tests/kvm/kvm_tests.cfg.sample
> @@ -181,7 +181,6 @@ variants:
>      - nic_hotplug:  install setup unattended_install
>          type = pci_hotplug
>          pci_type = nic
> -        modprobe_acpiphp = no
>          reference_cmd = lspci
>          find_pci_cmd = 'lspci | tail -n1'
>          pci_test_cmd = 'nslookup www.redhat.com'
> @@ -223,7 +222,6 @@ variants:
>                  image_format_stg = qcow2
>              - fmt_raw:
>                  image_format_stg = raw
> -                only Fedora Ubuntu Windows
>  
>      - system_reset: install setup unattended_install
>          type = boot
> @@ -538,13 +536,10 @@ variants:
>              nic_virtio:
>                  match_string = "VirtIO Ethernet"
>          block_hotplug:
> -            use_telnet = yes
> -            ssh_port = 23
> -            guest_port_ssh = 23
>              wait_secs_for_hook_up = 10
>              reference_cmd = wmic diskdrive list brief
>              find_pci_cmd = wmic diskdrive list brief
> -            pci_test_cmd = echo select disk 1 > dt && echo online disk >> dt && echo detail disk >> dt && echo exit >> dt && diskpart /s dt
> +            pci_test_cmd = echo select disk 1 > dt && echo online >> dt && echo detail disk >> dt && echo exit >> dt && diskpart /s dt
>  
>          variants:
>              - Win2000:
> diff --git a/client/tests/kvm/tests/pci_hotplug.py b/client/tests/kvm/tests/pci_hotplug.py
> index 3ad9ea2..876d8b8 100644
> --- a/client/tests/kvm/tests/pci_hotplug.py
> +++ b/client/tests/kvm/tests/pci_hotplug.py
> @@ -1,6 +1,6 @@
>  import logging, os
>  from autotest_lib.client.common_lib import error
> -import kvm_subprocess, kvm_test_utils, kvm_utils
> +import kvm_subprocess, kvm_test_utils, kvm_utils, kvm_vm
>  
>  
>  def run_pci_hotplug(test, params, env):
> @@ -21,8 +21,8 @@ def run_pci_hotplug(test, params, env):
>      session = kvm_test_utils.wait_for_login(vm)
>  
>      # Modprobe the module if specified in config file
> -    if params.get("modprobe_module"):
> -        module = params.get("modprobe_module")
> +    module = params.get("modprobe_module")
> +    if module:
>          if session.get_command_status("modprobe %s" % module):
>              raise error.TestError("Modprobe module '%s' failed" % module)
>  
> @@ -38,61 +38,63 @@ def run_pci_hotplug(test, params, env):
>      if test_type == "nic":
>          pci_add_cmd = "pci_add pci_addr=auto nic model=%s" % tested_model
>      elif test_type == "block":
> -        image_name = params.get("image_name_stg")
> -        image_filename = "%s.%s" % (image_name, params.get("image_format_stg"))
> -        image_dir = os.path.join(test.bindir, "images")
> -        storage_name = os.path.join(image_dir, image_filename)
> +        image_params = kvm_utils.get_sub_dict(params, "stg")
> +        image_filename = kvm_vm.get_image_filename(image_params, test.bindir)
>          pci_add_cmd = ("pci_add pci_addr=auto storage file=%s,if=%s" %
> -                                    (storage_name, tested_model))
> +                                    (image_filename, tested_model))
>  
>      # Implement pci_add
>      s, add_output = vm.send_monitor_cmd(pci_add_cmd)
>      if not "OK domain" in add_output:
>          raise error.TestFail("Add device failed. Hypervisor command is: %s. "
> -                             "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 after executing "
> -                             "hypervisor command: 'info pci'")
> -
> -    # Define a helper function to compare the output
> -    def new_shown():
> -        o = session.get_command_output(params.get("reference_cmd"))
> -        if reference == o:
> -            return False
> -        return True
> -
> -    secs = int(params.get("wait_secs_for_hook_up"))
> -    if not kvm_utils.wait_for(new_shown, 30, secs, 3):
> -        raise error.TestFail("No new device shown in output of command "
> -                             "executed inside the guest: %s" %
> -                             params.get("reference_cmd"))
> -
> -    # Define a helper function to catch PCI device string
> -    def find_pci():
> -        output = session.get_command_output(params.get("find_pci_cmd"))
> -        if not params.get("match_string") in output:
> -            return False
> -        return True
> -
> -    if not kvm_utils.wait_for(find_pci, 30, 3, 3):
> -        raise error.TestFail("PCI model not found: %s. Command is: %s" %
> -                             (tested_model, params.get("find_pci_cmd")))
> -
> -    # Test the newly added device
> -    s, o = session.get_command_status_output(params.get("pci_test_cmd"))
> -    if s != 0:
> -        raise error.TestFail("Check for %s device failed after PCI hotplug. "
> -                             "Output: %s" % (test_type, o))
> -
> -    # Delete the added 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. "
> -                             "Hypervisor command: %s" % (tested_model, cmd))
> -
> -    session.close()
> +                             "Output: %r" % (pci_add_cmd, add_output))
> +
> +    try:
> +        # 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 after executing "
> +                                 "hypervisor command: 'info pci'")
> +
> +        # Define a helper function to compare the output
> +        def new_shown():
> +            o = session.get_command_output(params.get("reference_cmd"))
> +            if reference == o:
> +                return False
> +            return True
> +
> +        secs = int(params.get("wait_secs_for_hook_up"))
> +        if not kvm_utils.wait_for(new_shown, 30, secs, 3):
> +            raise error.TestFail("No new device shown in output of command "
> +                                 "executed inside the guest: %s" %
> +                                 params.get("reference_cmd"))
> +
> +        # Define a helper function to catch PCI device string
> +        def find_pci():
> +            output = session.get_command_output(params.get("find_pci_cmd"))
> +            if not params.get("match_string") in output:
> +                return False
> +            return True
> +
> +        if not kvm_utils.wait_for(find_pci, 30, 3, 3):
> +            raise error.TestFail("PCI %s %s device not found in guest. "
> +                                 "Command was: %s" %
> +                                 (tested_model, test_type,
> +                                  params.get("find_pci_cmd")))
> +
> +        # Test the newly added device
> +        s, o = session.get_command_status_output(params.get("pci_test_cmd"))
> +        if s != 0:
> +            raise error.TestFail("Check for %s device failed after PCI "
> +                                 "hotplug. Output: %r" % (test_type, o))
> +
> +    finally:
> +        # Delete the added 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. "
> +                                 "Hypervisor command: %s" % (tested_model,
> +                                                             cmd))
> +        session.close()
> -- 
> 1.5.4.1
> 
> _______________________________________________
> Autotest mailing list
> Autotest@test.kernel.org
> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Autotest] [KVM-AUTOTEST PATCH 7/7] KVM test: remove monitor socket file when destroying a VM
  2009-11-05 10:01           ` [KVM-AUTOTEST PATCH 7/7] KVM test: remove monitor socket file when destroying a VM Michael Goldish
@ 2009-11-06  3:14             ` Yolkfull Chow
  0 siblings, 0 replies; 9+ messages in thread
From: Yolkfull Chow @ 2009-11-06  3:14 UTC (permalink / raw)
  To: Michael Goldish; +Cc: autotest, kvm

On Thu, Nov 05, 2009 at 12:01:12PM +0200, Michael Goldish wrote:
> This should slow the rate of accumulation of monitor files in /tmp.

Hi Michael,

I recommend we use TCP as monitor dev of VM. Two reasons:
 1) we don't need to add extra code to remove monitor files
 2) it's necessary for some users want to implement client-side
    migration.

What do you think? ;-)

> 
> Signed-off-by: Michael Goldish <mgoldish@redhat.com>
> ---
>  client/tests/kvm/kvm_vm.py |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
> index 5781dbc..62a10b9 100755
> --- a/client/tests/kvm/kvm_vm.py
> +++ b/client/tests/kvm/kvm_vm.py
> @@ -578,6 +578,10 @@ class VM:
>          finally:
>              if self.process:
>                  self.process.close()
> +            try:
> +                os.unlink(self.monitor_file_name)
> +            except OSError:
> +                pass
>  
>  
>      def is_alive(self):
> -- 
> 1.5.4.1
> 
> _______________________________________________
> Autotest mailing list
> Autotest@test.kernel.org
> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2009-11-06  3:14 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-11-05 10:01 [KVM-AUTOTEST PATCH 1/7] KVM test: make all tests depend on install, setup & unattended_install Michael Goldish
2009-11-05 10:01 ` [KVM-AUTOTEST PATCH 2/7] KVM test: kvm_vm.py: use kvm_utils.get_path() for floppy and tftp Michael Goldish
2009-11-05 10:01   ` [KVM-AUTOTEST PATCH 3/7] KVM test: kvm_vm.py: use 'is None' instead of '== None' Michael Goldish
2009-11-05 10:01     ` [KVM-AUTOTEST PATCH 4/7] KVM test: kvm_vm.py: fix typos in the code that compares -cdrom hashes Michael Goldish
2009-11-05 10:01       ` [KVM-AUTOTEST PATCH 5/7] KVM test: minor pci_hotplug fixes Michael Goldish
2009-11-05 10:01         ` [KVM-AUTOTEST PATCH 6/7] KVM test: kvm_config.py: match(): put filter inside parentheses Michael Goldish
2009-11-05 10:01           ` [KVM-AUTOTEST PATCH 7/7] KVM test: remove monitor socket file when destroying a VM Michael Goldish
2009-11-06  3:14             ` [Autotest] " Yolkfull Chow
2009-11-06  3:08         ` [Autotest] [KVM-AUTOTEST PATCH 5/7] KVM test: minor pci_hotplug fixes Yolkfull Chow

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.