[KVM-AUTOTEST PATCH] RHEL-4.7 step files: fix the initial boot barriers

[KVM-AUTOTEST PATCH] RHEL-4.7 step files: fix the initial boot barriers

[Michael Goldish]

Fix the first barrier to include the 'boot:' prompt. This is crucial because
the guest accepts keyboard input only after this prompt appears.

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
 .../tests/kvm_runtest_2/steps/RHEL-4.7-i386.steps  |    2 +-
 .../kvm_runtest_2/steps/RHEL-4.7-x86_64.steps      |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/client/tests/kvm_runtest_2/steps/RHEL-4.7-i386.steps b/client/tests/kvm_runtest_2/steps/RHEL-4.7-i386.steps
index 3816c15..763d473 100644
--- a/client/tests/kvm_runtest_2/steps/RHEL-4.7-i386.steps
+++ b/client/tests/kvm_runtest_2/steps/RHEL-4.7-i386.steps
@@ -4,7 +4,7 @@
 # --------------------------------
 step unknown
 screendump 20090413_013526_868fe81019ae64a0b066c4c0d4ebc4e1.ppm
-barrier_2 188 30 354 265 48ef114b5a42ba0d5bebfaee47dce498 50
+barrier_2 44 38 0 363 d9ca61811a10b33cc95515d4796541e7 50
 # Sending keys: ret
 key ret
 # --------------------------------
diff --git a/client/tests/kvm_runtest_2/steps/RHEL-4.7-x86_64.steps b/client/tests/kvm_runtest_2/steps/RHEL-4.7-x86_64.steps
index 644446f..36f0109 100644
--- a/client/tests/kvm_runtest_2/steps/RHEL-4.7-x86_64.steps
+++ b/client/tests/kvm_runtest_2/steps/RHEL-4.7-x86_64.steps
@@ -6,7 +6,7 @@ step 8.84
 screendump 20080101_000001_868fe81019ae64a0b066c4c0d4ebc4e1.ppm
 # boot options
 sleep 5
-barrier_2 194 59 101 59 8c4f6b29e4087e1bed13c40e4a6a904f 44 optional
+barrier_2 44 36 0 365 ea4c08daabe1f982b243fce9c5b542a0 44 optional
 # Sending keys: ret
 key ret
 # --------------------------------
-- 
1.5.4.1

[KVM-AUTOTEST PATCH] RHEL-5.3 step files: fix initial boot barriers and an inconsistent dialog

[Michael Goldish]

Fix the initial boot barriers to include the 'boot:' prompt.
Also fix a barrier in 5.3 of a dialog that sometimes appears at an unexpected
location.

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
 .../tests/kvm_runtest_2/steps/RHEL-5.3-i386.steps  |   12 ++++++++++--
 .../kvm_runtest_2/steps/RHEL-5.3-x86_64.steps      |    2 +-
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/client/tests/kvm_runtest_2/steps/RHEL-5.3-i386.steps b/client/tests/kvm_runtest_2/steps/RHEL-5.3-i386.steps
index 86066d8..0964f47 100644
--- a/client/tests/kvm_runtest_2/steps/RHEL-5.3-i386.steps
+++ b/client/tests/kvm_runtest_2/steps/RHEL-5.3-i386.steps
@@ -7,7 +7,7 @@ step 10.33
 screendump 20080101_000001_8333460cfad39ef04d6dbbf7d35fdcba.ppm
 # boot options
 sleep 10
-barrier_2 459 65 5 223 a817e977c049abaaab9a391d3cbeb1ab 52
+barrier_2 44 40 0 410 2f6c4cea4cf5b03bec757893e4982897 52
 # Sending keys: ret
 key ret
 # --------------------------------
@@ -187,10 +187,18 @@ key alt-f
 step 1518.52
 screendump 20080101_000024_3e641443dd4ea558467e517a4be68517.ppm
 # confirm no rhn
-barrier_2 97 18 146 77 a6a767b46d079c6879ebd5aec00cda46 43
+barrier_2 97 18 146 77 a6a767b46d079c6879ebd5aec00cda46 15 optional
 # Sending keys: alt-n
 key alt-n
 # --------------------------------
+step 1518.52
+screendump 20090506_091708_e5436357421502fccb3df89a830fd2f4.ppm
+# confirm no rhn (2)
+barrier_2 60 37 59 15 d94ff141696970e06545ce9854306970 5 optional
+# Sending keys: alt-tab alt-n
+key alt-tab
+key alt-n
+# --------------------------------
 step 1526.55
 screendump 20080101_000025_e10ec4a12e28baa41af798cbdbf308a1.ppm
 # finish update setup
diff --git a/client/tests/kvm_runtest_2/steps/RHEL-5.3-x86_64.steps b/client/tests/kvm_runtest_2/steps/RHEL-5.3-x86_64.steps
index 96ba87e..fc0db20 100644
--- a/client/tests/kvm_runtest_2/steps/RHEL-5.3-x86_64.steps
+++ b/client/tests/kvm_runtest_2/steps/RHEL-5.3-x86_64.steps
@@ -7,7 +7,7 @@ step 9.22
 screendump 20080101_000001_8333460cfad39ef04d6dbbf7d35fdcba.ppm
 # boot options
 sleep 10
-barrier_2 441 88 18 213 881883a4fa307a918f40804c70b9880d 46
+barrier_2 43 39 0 411 3c4a56d04a8ea1c0c5c1edfe10d472a0 46
 # Sending keys: ret
 key ret
 # --------------------------------
-- 
1.5.4.1

[KVM-AUTOTEST PATCH] WinXP step files: add an optional barrier to deal with a closed start menu

[Michael Goldish]

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
 .../kvm_runtest_2/steps/WinXP-32-setupssh.steps    |   10 ++++++++--
 client/tests/kvm_runtest_2/steps/WinXP-32.steps    |    4 +++-
 client/tests/kvm_runtest_2/steps/WinXP-64.steps    |    3 +--
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/client/tests/kvm_runtest_2/steps/WinXP-32-setupssh.steps b/client/tests/kvm_runtest_2/steps/WinXP-32-setupssh.steps
index 729d9df..ebb665f 100644
--- a/client/tests/kvm_runtest_2/steps/WinXP-32-setupssh.steps
+++ b/client/tests/kvm_runtest_2/steps/WinXP-32-setupssh.steps
@@ -4,8 +4,14 @@
 # --------------------------------
 step 24.72
 screendump 20080101_000001_5965948293222a6d6f3e545db40c23c1.ppm
-# open start menu
-barrier_2 125 79 342 270 368b3d82c870dbcdc4dfc2a49660e798 124
+# desktop reached
+barrier_2 36 32 392 292 3828d3a9587b3a9766a567a2b7570e42 124
+# --------------------------------
+step 24.72
+screendump 20080101_000001_5965948293222a6d6f3e545db40c23c1.ppm
+# open start menu if not already open
+sleep 10
+barrier_2 84 48 0 552 082462ce890968a264b9b13cddda8ae3 10 optional
 # Sending keys: ctrl-esc
 key ctrl-esc
 # --------------------------------
diff --git a/client/tests/kvm_runtest_2/steps/WinXP-32.steps b/client/tests/kvm_runtest_2/steps/WinXP-32.steps
index b0c6e35..f52fd0e 100644
--- a/client/tests/kvm_runtest_2/steps/WinXP-32.steps
+++ b/client/tests/kvm_runtest_2/steps/WinXP-32.steps
@@ -136,7 +136,8 @@ key alt-n
 step 2251.56
 screendump 20080101_000022_dcdc2fe9606c044ce648422afe42e23d.ppm
 # User
-barrier_2 409 35 64 188 3d71d4d7a9364c1e6415b3d554ce6e5b 9
+barrier_2 161 37 312 187 a941ecbeb73f9d73e3e9c38da9a4b743 9
+# Sending keys: $user alt-n
 var user
 key alt-n
 # --------------------------------
@@ -154,6 +155,7 @@ barrier_2 48 51 391 288 bbac8a522510d7c8d6e515f6a3fbd4c3 240
 step 2279.61
 screendump 20090416_150641_b72ad5c48ec2dbc9814d569e38cbb4cc.ppm
 # Win XP Start Menu (closed)
+sleep 20
 barrier_2 104 41 0 559 a7cc02cecff2cb495f300aefbb99d9ae 5 optional
 # Sending keys: ctrl-esc
 key ctrl-esc
diff --git a/client/tests/kvm_runtest_2/steps/WinXP-64.steps b/client/tests/kvm_runtest_2/steps/WinXP-64.steps
index 20bac81..91e6d0f 100644
--- a/client/tests/kvm_runtest_2/steps/WinXP-64.steps
+++ b/client/tests/kvm_runtest_2/steps/WinXP-64.steps
@@ -74,7 +74,6 @@ key ret
 # --------------------------------
 step 286.86
 screendump 20080101_000010_bb878343930f948c0346f103a387157a.ppm
-barrier_2 69 15 179 8 93889bdbe5351e61a6d9c7d00bb1c971 10
 # --------------------------------
 step 409.46
 screendump 20080101_000011_30db9777a7883a07e6e65bff74e1d98f.ppm
@@ -100,7 +99,7 @@ key 0xdc
 step 978.02
 screendump 20080101_000014_213fbe6fa13bf32dfac6a00bf4205e45.ppm
 # Windows XP Start Menu Opened
-barrier_2 48 20 274 420 c4a9620d84508013050e5a37a0d9e4ef 15
+barrier_2 129 30 196 72 aae68af7e05e2312c707f2f4bd73f024 15
 # Sending keys: u
 key u
 # --------------------------------
-- 
1.5.4.1

[KVM-AUTOTEST PATCH] stepeditor.py: get rid of some annoying keyboard shortcuts

[Michael Goldish]

Disable a few keyboard shortcuts that were initially assumed to be useful,
but apparently override the default functionality of the involved keys (e.g.
'home', 'end', 'delete') regardless of the widget that has the keyboard focus.

Also make some indentation changes to the UI description.

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
 client/tests/kvm_runtest_2/stepeditor.py |   59 ++++++++++++++----------------
 1 files changed, 28 insertions(+), 31 deletions(-)

diff --git a/client/tests/kvm_runtest_2/stepeditor.py b/client/tests/kvm_runtest_2/stepeditor.py
index de9a2b1..f21244b 100755
--- a/client/tests/kvm_runtest_2/stepeditor.py
+++ b/client/tests/kvm_runtest_2/stepeditor.py
@@ -790,27 +790,27 @@ class StepMakerWindow:
 class StepEditor(StepMakerWindow):
     ui = '''<ui>
     <menubar name="MenuBar">
-      <menu action="File">
-        <menuitem action="Open"/>
-        <separator/>
-        <menuitem action="Quit"/>
-      </menu>
-      <menu action="Edit">
-        <menuitem action="CopyStep"/>
-        <menuitem action="DeleteStep"/>
-      </menu>
-      <menu action="Insert">
-        <menuitem action="InsertNewBefore"/>
-        <menuitem action="InsertNewAfter"/>
-        <separator/>
-        <menuitem action="InsertStepsBefore"/>
-        <menuitem action="InsertStepsAfter"/>
-      </menu>
-      <menu action="Tools">
-        <menuitem action="CleanUp"/>
-      </menu>
+        <menu action="File">
+            <menuitem action="Open"/>
+            <separator/>
+            <menuitem action="Quit"/>
+        </menu>
+        <menu action="Edit">
+            <menuitem action="CopyStep"/>
+            <menuitem action="DeleteStep"/>
+        </menu>
+        <menu action="Insert">
+            <menuitem action="InsertNewBefore"/>
+            <menuitem action="InsertNewAfter"/>
+            <separator/>
+            <menuitem action="InsertStepsBefore"/>
+            <menuitem action="InsertStepsAfter"/>
+        </menu>
+        <menu action="Tools">
+            <menuitem action="CleanUp"/>
+        </menu>
     </menubar>
-    </ui>'''
+</ui>'''
 
     # Constructor
 
@@ -828,19 +828,19 @@ class StepEditor(StepMakerWindow):
         self.window.add_accel_group(accelgroup)
 
         # Create an ActionGroup
-        actiongroup = gtk.ActionGroup('UIManagerExample')
+        actiongroup = gtk.ActionGroup('StepEditor')
 
         # Create actions
         actiongroup.add_actions([
             ('Quit', gtk.STOCK_QUIT, '_Quit', None, 'Quit the Program', self.quit),
             ('Open', gtk.STOCK_OPEN, '_Open', None, 'Open steps file', self.open_steps_file),
-            ('CopyStep', gtk.STOCK_COPY, '_Copy current step...', None, 'Copy current step to user specified position', self.copy_step),
-            ('DeleteStep', gtk.STOCK_DELETE, '_Delete current step', None, 'Delete current step', self.event_remove_clicked),
-            ('InsertNewBefore', gtk.STOCK_ADD, '_New step before current', None, 'Insert new step before current step', self.insert_before),
-            ('InsertNewAfter', gtk.STOCK_ADD, 'N_ew step after current', None, 'Insert new step after current step', self.insert_after),
-            ('InsertStepsBefore', gtk.STOCK_ADD, '_Steps before current...', None, 'Insert steps (from file) before current step', self.insert_steps_before),
-            ('InsertStepsAfter', gtk.STOCK_ADD, 'Steps _after current...', None, 'Insert steps (from file) after current step', self.insert_steps_after),
-            ('CleanUp', gtk.STOCK_DELETE, '_Clean up data directory', None, 'Move unused PPM files to a backup directory', self.cleanup),
+            ('CopyStep', gtk.STOCK_COPY, '_Copy current step...', "", 'Copy current step to user specified position', self.copy_step),
+            ('DeleteStep', gtk.STOCK_DELETE, '_Delete current step', "", 'Delete current step', self.event_remove_clicked),
+            ('InsertNewBefore', gtk.STOCK_ADD, '_New step before current', "", 'Insert new step before current step', self.insert_before),
+            ('InsertNewAfter', gtk.STOCK_ADD, 'N_ew step after current', "", 'Insert new step after current step', self.insert_after),
+            ('InsertStepsBefore', gtk.STOCK_ADD, '_Steps before current...', "", 'Insert steps (from file) before current step', self.insert_steps_before),
+            ('InsertStepsAfter', gtk.STOCK_ADD, 'Steps _after current...', "", 'Insert steps (from file) after current step', self.insert_steps_after),
+            ('CleanUp', gtk.STOCK_DELETE, '_Clean up data directory', "", 'Move unused PPM files to a backup directory', self.cleanup),
             ('File', None, '_File'),
             ('Edit', None, '_Edit'),
             ('Insert', None, '_Insert'),
@@ -860,9 +860,6 @@ class StepEditor(StepMakerWindow):
 
         create_shortcut("Next", self.event_next_clicked, "Page_Down")
         create_shortcut("Previous", self.event_prev_clicked, "Page_Up")
-        create_shortcut("First", self.event_first_clicked, "Home")
-        create_shortcut("Last", self.event_last_clicked, "End")
-        create_shortcut("Delete", self.event_remove_clicked, "Delete")
 
         # Add the actiongroup to the uimanager
         uimanager.insert_action_group(actiongroup, 0)
-- 
1.5.4.1

[KVM-AUTOTEST PATCH] Use new function VM.get_name() to get the VM's name, instead of VM.name

[Michael Goldish]

kvm_vm.py: add function VM.get_name().
kvm_preprocessing.py: use VM.get_name() instead of directly accessing the .name
attribute.

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

diff --git a/client/tests/kvm_runtest_2/kvm_preprocessing.py b/client/tests/kvm_runtest_2/kvm_preprocessing.py
index c9eb35d..bcabf5a 100644
--- a/client/tests/kvm_runtest_2/kvm_preprocessing.py
+++ b/client/tests/kvm_runtest_2/kvm_preprocessing.py
@@ -178,7 +178,7 @@ def preprocess(test, params, env):
         if vm.is_dead():
             continue
         if not vm.verify_process_identity():
-            kvm_log.debug("VM '%s' seems to have been replaced by another process" % vm.name)
+            kvm_log.debug("VM '%s' seems to have been replaced by another process" % vm.get_name())
             vm.pid = None
 
     # Destroy and remove VMs that are no longer needed in the environment
@@ -187,8 +187,8 @@ def preprocess(test, params, env):
         vm = env[key]
         if not kvm_utils.is_vm(vm):
             continue
-        if not vm.name in requested_vms:
-            kvm_log.debug("VM '%s' found in environment but not required for test; removing it..." % vm.name)
+        if not vm.get_name() in requested_vms:
+            kvm_log.debug("VM '%s' found in environment but not required for test; removing it..." % vm.get_name())
             vm.destroy()
             del env[key]
 
diff --git a/client/tests/kvm_runtest_2/kvm_vm.py b/client/tests/kvm_runtest_2/kvm_vm.py
index fab839f..df99859 100644
--- a/client/tests/kvm_runtest_2/kvm_vm.py
+++ b/client/tests/kvm_runtest_2/kvm_vm.py
@@ -454,6 +454,10 @@ class VM:
         """Return True iff the VM's PID does not exist."""
         return not kvm_utils.pid_exists(self.pid)
 
+    def get_name(self):
+        """Return the VM's name."""
+        return self.name
+
     def get_params(self):
         """Return the VM's params dict.
 
-- 
1.5.4.1

[KVM-AUTOTEST PATCH] VM.create(): always destroy() the VM before attempting to start it

[Michael Goldish]

Also, don't do it in kvm_preprocessing.py since it's now done in kvm_vm.py.

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

diff --git a/client/tests/kvm_runtest_2/kvm_preprocessing.py b/client/tests/kvm_runtest_2/kvm_preprocessing.py
index bcabf5a..9ccaf78 100644
--- a/client/tests/kvm_runtest_2/kvm_preprocessing.py
+++ b/client/tests/kvm_runtest_2/kvm_preprocessing.py
@@ -84,7 +84,6 @@ def preprocess_vm(test, params, env, name):
             start_vm = True
 
     if start_vm:
-        vm.destroy()
         if not vm.create(name, params, qemu_path, image_dir, iso_dir, for_migration):
             message = "Could not start VM"
             kvm_log.error(message)
diff --git a/client/tests/kvm_runtest_2/kvm_vm.py b/client/tests/kvm_runtest_2/kvm_vm.py
index df99859..a1462c6 100644
--- a/client/tests/kvm_runtest_2/kvm_vm.py
+++ b/client/tests/kvm_runtest_2/kvm_vm.py
@@ -238,6 +238,8 @@ class VM:
         stored in the class attributes is used, and if it is supplied, it is stored
         for later use.
         """
+        self.destroy()
+
         if name != None:
             self.name = name
         if params != None:
-- 
1.5.4.1

[KVM-AUTOTEST PATCH] kvm_vm.py: choose a monitor filename in the constructor instead of VM.create()

[Michael Goldish]

This will reduce the number of monitor files left in /tmp, because the
constructor is called fewer times than VM.create().

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

diff --git a/client/tests/kvm_runtest_2/kvm_vm.py b/client/tests/kvm_runtest_2/kvm_vm.py
index a1462c6..3ce2003 100644
--- a/client/tests/kvm_runtest_2/kvm_vm.py
+++ b/client/tests/kvm_runtest_2/kvm_vm.py
@@ -106,6 +106,14 @@ class VM:
         self.image_dir = image_dir
         self.iso_dir = iso_dir
 
+        # Find available monitor filename
+        while True:
+            # The monitor filename should be unique
+            self.instance = time.strftime("%Y%m%d-%H%M%S-") + kvm_utils.generate_random_string(4)
+            self.monitor_file_name = os.path.join("/tmp", "monitor-" + self.instance)
+            if not os.path.exists(self.monitor_file_name):
+                break
+
     def verify_process_identity(self):
         """Make sure .pid really points to the original qemu process.
 
@@ -281,14 +289,6 @@ class VM:
                     kvm_log.error("Actual MD5 sum differs from expected one")
                     return False
 
-        # Find available monitor filename
-        while True:
-            # The monitor filename should be unique
-            self.instance = time.strftime("%Y%m%d-%H%M%S-") + kvm_utils.generate_random_string(4)
-            self.monitor_file_name = os.path.join("/tmp", "monitor-" + self.instance)
-            if not os.path.exists(self.monitor_file_name):
-                break
-
         # Handle port redirections
         redir_names = kvm_utils.get_sub_dict_names(params, "redirs")
         host_ports = kvm_utils.find_free_ports(5000, 6000, len(redir_names))
-- 
1.5.4.1

[KVM-AUTOTEST PATCH] kvm_vm.py: make sure the bulk of VM.create() is not executed in parallel

[Michael Goldish]

VM.create() does a few things (such as finding free ports) which are not safe
to execute in parallel. Use a lock file to make sure this doesn't happen. The
lock is released only after the VM is started or fails to start.

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

diff --git a/client/tests/kvm_runtest_2/kvm_vm.py b/client/tests/kvm_runtest_2/kvm_vm.py
index 3ce2003..af06693 100644
--- a/client/tests/kvm_runtest_2/kvm_vm.py
+++ b/client/tests/kvm_runtest_2/kvm_vm.py
@@ -3,6 +3,7 @@
 import time
 import socket
 import os
+import fcntl
 
 import kvm_utils
 import kvm_log
@@ -289,48 +290,58 @@ class VM:
                     kvm_log.error("Actual MD5 sum differs from expected one")
                     return False
 
-        # Handle port redirections
-        redir_names = kvm_utils.get_sub_dict_names(params, "redirs")
-        host_ports = kvm_utils.find_free_ports(5000, 6000, len(redir_names))
-        self.redirs = {}
-        for i in range(len(redir_names)):
-            redir_params = kvm_utils.get_sub_dict(params, redir_names[i])
-            guest_port = int(redir_params.get("guest_port"))
-            self.redirs[guest_port] = host_ports[i]
-
-        # Find available VNC port, if needed
-        if params.get("display") == "vnc":
-            self.vnc_port = kvm_utils.find_free_port(5900, 6000)
-
-        # Make qemu command
-        qemu_command = self.make_qemu_command()
+        # Make sure the following code is not executed by more than one thread
+        # at the same time
+        lockfile = open("/tmp/kvm-autotest-vm-create.lock", "w+")
+        fcntl.lockf(lockfile, fcntl.LOCK_EX)
 
-        # Is this VM supposed to accept incoming migrations?
-        if for_migration:
-            # Find available migration port
-            self.migration_port = kvm_utils.find_free_port(5200, 6000)
-            # Add -incoming option to the qemu command
-            qemu_command += " -incoming tcp:0:%d" % self.migration_port
+        try:
+            # Handle port redirections
+            redir_names = kvm_utils.get_sub_dict_names(params, "redirs")
+            host_ports = kvm_utils.find_free_ports(5000, 6000, len(redir_names))
+            self.redirs = {}
+            for i in range(len(redir_names)):
+                redir_params = kvm_utils.get_sub_dict(params, redir_names[i])
+                guest_port = int(redir_params.get("guest_port"))
+                self.redirs[guest_port] = host_ports[i]
+
+            # Find available VNC port, if needed
+            if params.get("display") == "vnc":
+                self.vnc_port = kvm_utils.find_free_port(5900, 6000)
+
+            # Make qemu command
+            qemu_command = self.make_qemu_command()
+
+            # Is this VM supposed to accept incoming migrations?
+            if for_migration:
+                # Find available migration port
+                self.migration_port = kvm_utils.find_free_port(5200, 6000)
+                # Add -incoming option to the qemu command
+                qemu_command += " -incoming tcp:0:%d" % self.migration_port
+
+            kvm_log.debug("Running qemu command:\n%s" % qemu_command)
+            (status, pid, output) = kvm_utils.run_bg(qemu_command, None, kvm_log.debug, "(qemu) ")
+
+            if status:
+                kvm_log.debug("qemu exited with status %d" % status)
+                kvm_log.error("VM could not be created -- qemu command failed:\n%s" % qemu_command)
+                return False
 
-        kvm_log.debug("Running qemu command:\n%s" % qemu_command)
-        (status, pid, output) = kvm_utils.run_bg(qemu_command, None, kvm_log.debug, "(qemu) ")
+            self.pid = pid
 
-        if status:
-            kvm_log.debug("qemu exited with status %d" % status)
-            kvm_log.error("VM could not be created -- qemu command failed:\n%s" % qemu_command)
-            return False
-
-        self.pid = pid
+            if not kvm_utils.wait_for(self.is_alive, timeout, 0, 1):
+                kvm_log.debug("VM is not alive for some reason")
+                kvm_log.error("VM could not be created with command:\n%s" % qemu_command)
+                self.destroy()
+                return False
 
-        if not kvm_utils.wait_for(self.is_alive, timeout, 0, 1):
-            kvm_log.debug("VM is not alive for some reason")
-            kvm_log.error("VM could not be created with command:\n%s" % qemu_command)
-            self.destroy()
-            return False
+            kvm_log.debug("VM appears to be alive with PID %d" % self.pid)
 
-        kvm_log.debug("VM appears to be alive with PID %d" % self.pid)
+            return True
 
-        return True
+        finally:
+            fcntl.lockf(lockfile, fcntl.LOCK_UN)
+            lockfile.close()
 
     def send_monitor_cmd(self, command, block=True, timeout=20.0):
         """Send command to the QEMU monitor.
-- 
1.5.4.1

[KVM-AUTOTEST PATCH] kvm_vm.py: add new function VM.clone() which returns a clone of the VM

[Michael Goldish]

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

diff --git a/client/tests/kvm_runtest_2/kvm_vm.py b/client/tests/kvm_runtest_2/kvm_vm.py
index af06693..9571a3b 100644
--- a/client/tests/kvm_runtest_2/kvm_vm.py
+++ b/client/tests/kvm_runtest_2/kvm_vm.py
@@ -115,6 +115,24 @@ class VM:
             if not os.path.exists(self.monitor_file_name):
                 break
 
+    def clone(self, name=None, params=None, qemu_path=None, image_dir=None, iso_dir=None):
+        """Return a clone of the VM object with optionally modified parameters.
+
+        The clone is initially not alive and needs to be started using create().
+        Any parameters not passed to this function are copied from the source VM.
+        """
+        if name == None:
+            name = self.name
+        if params == None:
+            params = self.params.copy()
+        if qemu_path == None:
+            qemu_path = self.qemu_path
+        if image_dir == None:
+            image_dir = self.image_dir
+        if iso_dir == None:
+            iso_dir = self.iso_dir
+        return VM(name, params, qemu_path, image_dir, iso_dir)
+
     def verify_process_identity(self):
         """Make sure .pid really points to the original qemu process.
 
-- 
1.5.4.1

[KVM-AUTOTEST PATCH] kvm_utils.py: remote_login(): improve regular expression matching

[Michael Goldish]

1. Make the 'login:' regular expression stricter so it doesn't match
'Last login: ...' messages.
2. Make the 'password:' regular expression stricter.
3. Handle 'Connection refused' messages.

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

diff --git a/client/tests/kvm_runtest_2/kvm_utils.py b/client/tests/kvm_runtest_2/kvm_utils.py
index be8ad95..5736cf6 100644
--- a/client/tests/kvm_runtest_2/kvm_utils.py
+++ b/client/tests/kvm_runtest_2/kvm_utils.py
@@ -413,7 +413,8 @@ def remote_login(command, password, prompt, linesep="\n", timeout=10):
 
     while True:
         (match, text) = sub.read_until_last_line_matches(
-                ["[Aa]re you sure", "[Pp]assword:", "[Ll]ogin:", "[Cc]onnection.*closed", prompt],
+                [r"[Aa]re you sure", r"[Pp]assword:\s*$", r"^\s*[Ll]ogin:\s*$",
+                    r"[Cc]onnection.*closed", r"[Cc]onnection.*refused", prompt],
                 timeout=timeout, internal_timeout=0.5)
         if match == 0:  # "Are you sure you want to continue connecting"
             kvm_log.debug("Got 'Are you sure...'; sending 'yes'")
@@ -437,11 +438,15 @@ def remote_login(command, password, prompt, linesep="\n", timeout=10):
             kvm_log.debug("Got 'Connection closed'")
             sub.close()
             return None
-        elif match == 4:  # prompt
+        elif match == 4:  # "Connection refused"
+            kvm_log.debug("Got 'Connection refused'")
+            sub.close()
+            return None
+        elif match == 5:  # prompt
             kvm_log.debug("Got shell prompt -- logged in")
             return sub
         else:  # match == None
-            kvm_log.debug("Timeout or process terminated")
+            kvm_log.debug("Timeout elapsed or process terminated")
             sub.close()
             return None
 
@@ -470,7 +475,7 @@ def remote_scp(command, password, timeout=300, login_timeout=10):
 
     while True:
         (match, text) = sub.read_until_last_line_matches(
-                ["[Aa]re you sure", "[Pp]assword:", "lost connection"],
+                [r"[Aa]re you sure", r"[Pp]assword:\s*$", r"lost connection"],
                 timeout=_timeout, internal_timeout=0.5)
         if match == 0:  # "Are you sure you want to continue connecting"
             kvm_log.debug("Got 'Are you sure...'; sending 'yes'")
-- 
1.5.4.1

[KVM-AUTOTEST PATCH] kvm_vm.py: add new VM parameter 'x11_display' that controls $DISPLAY

[Michael Goldish]

If x11_display is specified, the DISPLAY environment variable is set to this
value for the QEMU process. This may be useful for SDL rendering.

Also add some comments.

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

diff --git a/client/tests/kvm_runtest_2/kvm_vm.py b/client/tests/kvm_runtest_2/kvm_vm.py
index 9571a3b..7a4ce4a 100644
--- a/client/tests/kvm_runtest_2/kvm_vm.py
+++ b/client/tests/kvm_runtest_2/kvm_vm.py
@@ -173,6 +173,8 @@ class VM:
                 (iso_dir is pre-pended to the ISO filename)
             extra_params -- a string to append to the qemu command
             ssh_port -- should be 22 for SSH, 23 for Telnet
+            x11_display -- if specified, the DISPLAY environment variable will be be set
+                to this value for the qemu process (useful for SDL rendering)
             images -- a list of image object names, separated by spaces
             nics -- a list of NIC object names, separated by spaces
 
@@ -198,8 +200,16 @@ class VM:
         if iso_dir == None:
             iso_dir = self.iso_dir
 
-        qemu_cmd = qemu_path
+        # Start constructing the qemu command
+        qemu_cmd = ""
+        # Set the X11 display parameter if requested
+        if params.get("x11_display"):
+            qemu_cmd += "DISPLAY=%s " % params.get("x11_display")
+        # Add the qemu binary
+        qemu_cmd += qemu_path
+        # Add the VM's name
         qemu_cmd += " -name '%s'" % name
+        # Add the monitor socket parameter
         qemu_cmd += " -monitor unix:%s,server,nowait" % self.monitor_file_name
 
         for image_name in kvm_utils.get_sub_dict_names(params, "images"):
-- 
1.5.4.1

[KVM-AUTOTEST PATCH] kvm_runtest_2.py: use environment filename specified by the 'env' parameter

[Michael Goldish]

Do not use hardcoded environment filename 'env'. Instead use the value
specified by the 'env' parameter. If unspecified, use 'env' as the filename.

This is important for parallel execution; it may be necessary to use a separate
environment file for each process.

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

diff --git a/client/tests/kvm_runtest_2/kvm_runtest_2.py b/client/tests/kvm_runtest_2/kvm_runtest_2.py
index fda7282..a69951b 100644
--- a/client/tests/kvm_runtest_2/kvm_runtest_2.py
+++ b/client/tests/kvm_runtest_2/kvm_runtest_2.py
@@ -64,7 +64,7 @@ class kvm_runtest_2(test.test):
             self.write_test_keyval({key: params[key]})
 
         # Open the environment file
-        env_filename = os.path.join(self.bindir, "env")
+        env_filename = os.path.join(self.bindir, params.get("env", "env"))
         env = shelve.open(env_filename, writeback=True)
         kvm_log.debug("Contents of environment: %s" % str(env))
 
-- 
1.5.4.1

[KVM-AUTOTEST PATCH] kvm_runtest_2.py: use pickle instead of shelve when loading/saving env

[Michael Goldish]

pickle allows more control over the load/save process. Specifically, it enables
us to dump the contents of an object to disk without having to unpickle it.
shelve, which uses pickle, seems to pickle and unpickle every time sync() is
called. This is bad for classes that need to be unpickled only once per test
(such a class will be introduced in a future patch).

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

diff --git a/client/tests/kvm_runtest_2/kvm_runtest_2.py b/client/tests/kvm_runtest_2/kvm_runtest_2.py
index a69951b..5f7f6ad 100644
--- a/client/tests/kvm_runtest_2/kvm_runtest_2.py
+++ b/client/tests/kvm_runtest_2/kvm_runtest_2.py
@@ -3,9 +3,9 @@
 import sys
 import os
 import time
-import shelve
 import random
 import resource
+import cPickle
 
 from autotest_lib.client.bin import test
 from autotest_lib.client.common_lib import error
@@ -18,6 +18,22 @@ class test_routine:
         self.routine = None
 
 
+def dump_env(obj, filename):
+    file = open(filename, "w")
+    cPickle.dump(obj, file)
+    file.close()
+
+
+def load_env(filename, default=None):
+    try:
+        file = open(filename, "r")
+    except:
+        return default
+    obj = cPickle.load(file)
+    file.close()
+    return obj
+
+
 class kvm_runtest_2(test.test):
     version = 1
 
@@ -65,7 +81,7 @@ class kvm_runtest_2(test.test):
 
         # Open the environment file
         env_filename = os.path.join(self.bindir, params.get("env", "env"))
-        env = shelve.open(env_filename, writeback=True)
+        env = load_env(env_filename, {})
         kvm_log.debug("Contents of environment: %s" % str(env))
 
         try:
@@ -87,24 +103,23 @@ class kvm_runtest_2(test.test):
 
                 # Preprocess
                 kvm_preprocessing.preprocess(self, params, env)
-                env.sync()
+                dump_env(env, env_filename)
                 # Run the test function
                 routine_obj.routine(self, params, env)
-                env.sync()
+                dump_env(env, env_filename)
 
             except Exception, e:
                 kvm_log.error("Test failed: %s" % e)
                 kvm_log.debug("Postprocessing on error...")
                 kvm_preprocessing.postprocess_on_error(self, params, env)
-                env.sync()
+                dump_env(env, env_filename)
                 raise
 
         finally:
             # Postprocess
             kvm_preprocessing.postprocess(self, params, env)
             kvm_log.debug("Contents of environment: %s" % str(env))
-            env.sync()
-            env.close()
+            dump_env(env, env_filename)
 
     def postprocess(self):
         pass
-- 
1.5.4.1

Re: [KVM-AUTOTEST PATCH] kvm_utils.py: remote_login(): improve regular expression matching

[sudhir kumar]

The patch looks sane to me. A very good thing that can be done for
remote_login() is to tune the tmeouts. I have seen especialy with
windows guests or sometimes when the machine is heavily loaded the
timeouts elapse and the test fails. When I increased the timeouts the
test did not fail. internal_timeout=0.5 is too less in my views and
even timeouts of 10 seconds prove insufficient sometimes. Do you too
have any such experience?

On Sun, May 24, 2009 at 9:16 PM, Michael Goldish <mgoldish@redhat.com> wrote:
> 1. Make the 'login:' regular expression stricter so it doesn't match
> 'Last login: ...' messages.
> 2. Make the 'password:' regular expression stricter.
> 3. Handle 'Connection refused' messages.
>
> Signed-off-by: Michael Goldish <mgoldish@redhat.com>
> ---
>  client/tests/kvm_runtest_2/kvm_utils.py |   13 +++++++++----
>  1 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/client/tests/kvm_runtest_2/kvm_utils.py b/client/tests/kvm_runtest_2/kvm_utils.py
> index be8ad95..5736cf6 100644
> --- a/client/tests/kvm_runtest_2/kvm_utils.py
> +++ b/client/tests/kvm_runtest_2/kvm_utils.py
> @@ -413,7 +413,8 @@ def remote_login(command, password, prompt, linesep="\n", timeout=10):
>
>     while True:
>         (match, text) = sub.read_until_last_line_matches(
> -                ["[Aa]re you sure", "[Pp]assword:", "[Ll]ogin:", "[Cc]onnection.*closed", prompt],
> +                [r"[Aa]re you sure", r"[Pp]assword:\s*$", r"^\s*[Ll]ogin:\s*$",
> +                    r"[Cc]onnection.*closed", r"[Cc]onnection.*refused", prompt],
>                 timeout=timeout, internal_timeout=0.5)
>         if match == 0:  # "Are you sure you want to continue connecting"
>             kvm_log.debug("Got 'Are you sure...'; sending 'yes'")
> @@ -437,11 +438,15 @@ def remote_login(command, password, prompt, linesep="\n", timeout=10):
>             kvm_log.debug("Got 'Connection closed'")
>             sub.close()
>             return None
> -        elif match == 4:  # prompt
> +        elif match == 4:  # "Connection refused"
> +            kvm_log.debug("Got 'Connection refused'")
> +            sub.close()
> +            return None
> +        elif match == 5:  # prompt
>             kvm_log.debug("Got shell prompt -- logged in")
>             return sub
>         else:  # match == None
> -            kvm_log.debug("Timeout or process terminated")
> +            kvm_log.debug("Timeout elapsed or process terminated")
>             sub.close()
>             return None
>
> @@ -470,7 +475,7 @@ def remote_scp(command, password, timeout=300, login_timeout=10):
>
>     while True:
>         (match, text) = sub.read_until_last_line_matches(
> -                ["[Aa]re you sure", "[Pp]assword:", "lost connection"],
> +                [r"[Aa]re you sure", r"[Pp]assword:\s*$", r"lost connection"],
>                 timeout=_timeout, internal_timeout=0.5)
>         if match == 0:  # "Are you sure you want to continue connecting"
>             kvm_log.debug("Got 'Are you sure...'; sending 'yes'")
> --
> 1.5.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Sudhir Kumar

Re: [KVM-AUTOTEST PATCH] kvm_runtest_2.py: use environment filename specified by the 'env' parameter

[sudhir kumar]

good one to be in.  thanks

On Sun, May 24, 2009 at 9:16 PM, Michael Goldish <mgoldish@redhat.com> wrote:
> Do not use hardcoded environment filename 'env'. Instead use the value
> specified by the 'env' parameter. If unspecified, use 'env' as the filename.
>
> This is important for parallel execution; it may be necessary to use a separate
> environment file for each process.
>
> Signed-off-by: Michael Goldish <mgoldish@redhat.com>
> ---
>  client/tests/kvm_runtest_2/kvm_runtest_2.py |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/client/tests/kvm_runtest_2/kvm_runtest_2.py b/client/tests/kvm_runtest_2/kvm_runtest_2.py
> index fda7282..a69951b 100644
> --- a/client/tests/kvm_runtest_2/kvm_runtest_2.py
> +++ b/client/tests/kvm_runtest_2/kvm_runtest_2.py
> @@ -64,7 +64,7 @@ class kvm_runtest_2(test.test):
>             self.write_test_keyval({key: params[key]})
>
>         # Open the environment file
> -        env_filename = os.path.join(self.bindir, "env")
> +        env_filename = os.path.join(self.bindir, params.get("env", "env"))
>         env = shelve.open(env_filename, writeback=True)
>         kvm_log.debug("Contents of environment: %s" % str(env))
>
> --
> 1.5.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Sudhir Kumar

Re: [KVM-AUTOTEST PATCH] kvm_vm.py: make sure the bulk of VM.create() is not executed in parallel

[sudhir kumar]

On Sun, May 24, 2009 at 9:16 PM, Michael Goldish <mgoldish@redhat.com> wrote:
> VM.create() does a few things (such as finding free ports) which are not safe
> to execute in parallel. Use a lock file to make sure this doesn't happen. The
> lock is released only after the VM is started or fails to start.
>
> Signed-off-by: Michael Goldish <mgoldish@redhat.com>
> ---
>  client/tests/kvm_runtest_2/kvm_vm.py |   85 +++++++++++++++++++---------------
>  1 files changed, 48 insertions(+), 37 deletions(-)
>
> diff --git a/client/tests/kvm_runtest_2/kvm_vm.py b/client/tests/kvm_runtest_2/kvm_vm.py
> index 3ce2003..af06693 100644
> --- a/client/tests/kvm_runtest_2/kvm_vm.py
> +++ b/client/tests/kvm_runtest_2/kvm_vm.py
> @@ -3,6 +3,7 @@
>  import time
>  import socket
>  import os
> +import fcntl
>
>  import kvm_utils
>  import kvm_log
> @@ -289,48 +290,58 @@ class VM:
>                     kvm_log.error("Actual MD5 sum differs from expected one")
>                     return False
>
> -        # Handle port redirections
> -        redir_names = kvm_utils.get_sub_dict_names(params, "redirs")
> -        host_ports = kvm_utils.find_free_ports(5000, 6000, len(redir_names))
> -        self.redirs = {}
> -        for i in range(len(redir_names)):
> -            redir_params = kvm_utils.get_sub_dict(params, redir_names[i])
> -            guest_port = int(redir_params.get("guest_port"))
> -            self.redirs[guest_port] = host_ports[i]
> -
> -        # Find available VNC port, if needed
> -        if params.get("display") == "vnc":
> -            self.vnc_port = kvm_utils.find_free_port(5900, 6000)
> -
> -        # Make qemu command
> -        qemu_command = self.make_qemu_command()
> +        # Make sure the following code is not executed by more than one thread
> +        # at the same time
> +        lockfile = open("/tmp/kvm-autotest-vm-create.lock", "w+")
How do you handle an open failure?
> +        fcntl.lockf(lockfile, fcntl.LOCK_EX)
What if other instance has locked the file at the moment. Definitely
you would not like to fail. You may want to wait for a while and try
agian. I feel the default behaviour should be a blocking one but still
you want to print the debug message
kvm_log.debug("Trying to acquire lock for port selection")
before getting the lock.

>
> -        # Is this VM supposed to accept incoming migrations?
> -        if for_migration:
> -            # Find available migration port
> -            self.migration_port = kvm_utils.find_free_port(5200, 6000)
> -            # Add -incoming option to the qemu command
> -            qemu_command += " -incoming tcp:0:%d" % self.migration_port
> +        try:
> +            # Handle port redirections
> +            redir_names = kvm_utils.get_sub_dict_names(params, "redirs")
> +            host_ports = kvm_utils.find_free_ports(5000, 6000, len(redir_names))
> +            self.redirs = {}
> +            for i in range(len(redir_names)):
> +                redir_params = kvm_utils.get_sub_dict(params, redir_names[i])
> +                guest_port = int(redir_params.get("guest_port"))
> +                self.redirs[guest_port] = host_ports[i]
> +
> +            # Find available VNC port, if needed
> +            if params.get("display") == "vnc":
> +                self.vnc_port = kvm_utils.find_free_port(5900, 6000)
> +
> +            # Make qemu command
> +            qemu_command = self.make_qemu_command()
> +
> +            # Is this VM supposed to accept incoming migrations?
> +            if for_migration:
> +                # Find available migration port
> +                self.migration_port = kvm_utils.find_free_port(5200, 6000)
> +                # Add -incoming option to the qemu command
> +                qemu_command += " -incoming tcp:0:%d" % self.migration_port
> +
> +            kvm_log.debug("Running qemu command:\n%s" % qemu_command)
> +            (status, pid, output) = kvm_utils.run_bg(qemu_command, None, kvm_log.debug, "(qemu) ")
> +
> +            if status:
> +                kvm_log.debug("qemu exited with status %d" % status)
> +                kvm_log.error("VM could not be created -- qemu command failed:\n%s" % qemu_command)
> +                return False
>
> -        kvm_log.debug("Running qemu command:\n%s" % qemu_command)
> -        (status, pid, output) = kvm_utils.run_bg(qemu_command, None, kvm_log.debug, "(qemu) ")
> +            self.pid = pid
>
> -        if status:
> -            kvm_log.debug("qemu exited with status %d" % status)
> -            kvm_log.error("VM could not be created -- qemu command failed:\n%s" % qemu_command)
> -            return False
> -
> -        self.pid = pid
> +            if not kvm_utils.wait_for(self.is_alive, timeout, 0, 1):
> +                kvm_log.debug("VM is not alive for some reason")
> +                kvm_log.error("VM could not be created with command:\n%s" % qemu_command)
> +                self.destroy()
> +                return False
>
> -        if not kvm_utils.wait_for(self.is_alive, timeout, 0, 1):
> -            kvm_log.debug("VM is not alive for some reason")
> -            kvm_log.error("VM could not be created with command:\n%s" % qemu_command)
> -            self.destroy()
> -            return False
> +            kvm_log.debug("VM appears to be alive with PID %d" % self.pid)
>
> -        kvm_log.debug("VM appears to be alive with PID %d" % self.pid)
> +            return True
>
> -        return True
> +        finally:
> +            fcntl.lockf(lockfile, fcntl.LOCK_UN)
> +            lockfile.close()
>
>     def send_monitor_cmd(self, command, block=True, timeout=20.0):
>         """Send command to the QEMU monitor.
> --
> 1.5.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Sudhir Kumar

Re: [KVM-AUTOTEST PATCH] WinXP step files: add an optional barrier to deal with a closed start menu

[sudhir kumar]

Any plans to add other windows stepfiles?

On Sun, May 24, 2009 at 9:16 PM, Michael Goldish <mgoldish@redhat.com> wrote:
> Signed-off-by: Michael Goldish <mgoldish@redhat.com>
> ---
>  .../kvm_runtest_2/steps/WinXP-32-setupssh.steps    |   10 ++++++++--
>  client/tests/kvm_runtest_2/steps/WinXP-32.steps    |    4 +++-
>  client/tests/kvm_runtest_2/steps/WinXP-64.steps    |    3 +--
>  3 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/client/tests/kvm_runtest_2/steps/WinXP-32-setupssh.steps b/client/tests/kvm_runtest_2/steps/WinXP-32-setupssh.steps
> index 729d9df..ebb665f 100644
> --- a/client/tests/kvm_runtest_2/steps/WinXP-32-setupssh.steps
> +++ b/client/tests/kvm_runtest_2/steps/WinXP-32-setupssh.steps
> @@ -4,8 +4,14 @@
>  # --------------------------------
>  step 24.72
>  screendump 20080101_000001_5965948293222a6d6f3e545db40c23c1.ppm
> -# open start menu
> -barrier_2 125 79 342 270 368b3d82c870dbcdc4dfc2a49660e798 124
> +# desktop reached
> +barrier_2 36 32 392 292 3828d3a9587b3a9766a567a2b7570e42 124
> +# --------------------------------
> +step 24.72
> +screendump 20080101_000001_5965948293222a6d6f3e545db40c23c1.ppm
> +# open start menu if not already open
> +sleep 10
> +barrier_2 84 48 0 552 082462ce890968a264b9b13cddda8ae3 10 optional
>  # Sending keys: ctrl-esc
>  key ctrl-esc
>  # --------------------------------
> diff --git a/client/tests/kvm_runtest_2/steps/WinXP-32.steps b/client/tests/kvm_runtest_2/steps/WinXP-32.steps
> index b0c6e35..f52fd0e 100644
> --- a/client/tests/kvm_runtest_2/steps/WinXP-32.steps
> +++ b/client/tests/kvm_runtest_2/steps/WinXP-32.steps
> @@ -136,7 +136,8 @@ key alt-n
>  step 2251.56
>  screendump 20080101_000022_dcdc2fe9606c044ce648422afe42e23d.ppm
>  # User
> -barrier_2 409 35 64 188 3d71d4d7a9364c1e6415b3d554ce6e5b 9
> +barrier_2 161 37 312 187 a941ecbeb73f9d73e3e9c38da9a4b743 9
> +# Sending keys: $user alt-n
>  var user
>  key alt-n
>  # --------------------------------
> @@ -154,6 +155,7 @@ barrier_2 48 51 391 288 bbac8a522510d7c8d6e515f6a3fbd4c3 240
>  step 2279.61
>  screendump 20090416_150641_b72ad5c48ec2dbc9814d569e38cbb4cc.ppm
>  # Win XP Start Menu (closed)
> +sleep 20
>  barrier_2 104 41 0 559 a7cc02cecff2cb495f300aefbb99d9ae 5 optional
>  # Sending keys: ctrl-esc
>  key ctrl-esc
> diff --git a/client/tests/kvm_runtest_2/steps/WinXP-64.steps b/client/tests/kvm_runtest_2/steps/WinXP-64.steps
> index 20bac81..91e6d0f 100644
> --- a/client/tests/kvm_runtest_2/steps/WinXP-64.steps
> +++ b/client/tests/kvm_runtest_2/steps/WinXP-64.steps
> @@ -74,7 +74,6 @@ key ret
>  # --------------------------------
>  step 286.86
>  screendump 20080101_000010_bb878343930f948c0346f103a387157a.ppm
> -barrier_2 69 15 179 8 93889bdbe5351e61a6d9c7d00bb1c971 10
>  # --------------------------------
>  step 409.46
>  screendump 20080101_000011_30db9777a7883a07e6e65bff74e1d98f.ppm
> @@ -100,7 +99,7 @@ key 0xdc
>  step 978.02
>  screendump 20080101_000014_213fbe6fa13bf32dfac6a00bf4205e45.ppm
>  # Windows XP Start Menu Opened
> -barrier_2 48 20 274 420 c4a9620d84508013050e5a37a0d9e4ef 15
> +barrier_2 129 30 196 72 aae68af7e05e2312c707f2f4bd73f024 15
>  # Sending keys: u
>  key u
>  # --------------------------------
> --
> 1.5.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Sudhir Kumar

Re: [KVM-AUTOTEST PATCH] kvm_vm.py: add new function VM.clone() which returns a clone of the VM

[Avi Kivity]

Michael Goldish wrote:
> Signed-off-by: Michael Goldish <mgoldish@redhat.com>
> ---
>  client/tests/kvm_runtest_2/kvm_vm.py |   18 ++++++++++++++++++
>  1 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/client/tests/kvm_runtest_2/kvm_vm.py b/client/tests/kvm_runtest_2/kvm_vm.py
> index af06693..9571a3b 100644
> --- a/client/tests/kvm_runtest_2/kvm_vm.py
> +++ b/client/tests/kvm_runtest_2/kvm_vm.py
> @@ -115,6 +115,24 @@ class VM:
>              if not os.path.exists(self.monitor_file_name):
>                  break
>  
> +    def clone(self, name=None, params=None, qemu_path=None, image_dir=None, iso_dir=None):
> +        """Return a clone of the VM object with optionally modified parameters.
> +
> +        The clone is initially not alive and needs to be started using create().
> +        Any parameters not passed to this function are copied from the source VM.
> +        """
> +        if name == None:
> +            name = self.name
> +        if params == None:
> +            params = self.params.copy()
> +        if qemu_path == None:
> +            qemu_path = self.qemu_path
> +        if image_dir == None:
> +            image_dir = self.image_dir
> +        if iso_dir == None:
> +            iso_dir = self.iso_dir
> +        return VM(name, params, qemu_path, image_dir, iso_dir)
> +
>   

    return VM(name or self.name, params or self.params.copy(), ...)


-- 
error compiling committee.c: too many arguments to function

Re: [KVM-AUTOTEST PATCH] kvm_vm.py: add new function VM.clone() which returns a clone of the VM

[Michael Goldish]

----- "Avi Kivity" <avi@redhat.com> wrote:

> Michael Goldish wrote:
> > Signed-off-by: Michael Goldish <mgoldish@redhat.com>
> > ---
> >  client/tests/kvm_runtest_2/kvm_vm.py |   18 ++++++++++++++++++
> >  1 files changed, 18 insertions(+), 0 deletions(-)
> >
> > diff --git a/client/tests/kvm_runtest_2/kvm_vm.py
> b/client/tests/kvm_runtest_2/kvm_vm.py
> > index af06693..9571a3b 100644
> > --- a/client/tests/kvm_runtest_2/kvm_vm.py
> > +++ b/client/tests/kvm_runtest_2/kvm_vm.py
> > @@ -115,6 +115,24 @@ class VM:
> >              if not os.path.exists(self.monitor_file_name):
> >                  break
> >  
> > +    def clone(self, name=None, params=None, qemu_path=None,
> image_dir=None, iso_dir=None):
> > +        """Return a clone of the VM object with optionally modified
> parameters.
> > +
> > +        The clone is initially not alive and needs to be started
> using create().
> > +        Any parameters not passed to this function are copied from
> the source VM.
> > +        """
> > +        if name == None:
> > +            name = self.name
> > +        if params == None:
> > +            params = self.params.copy()
> > +        if qemu_path == None:
> > +            qemu_path = self.qemu_path
> > +        if image_dir == None:
> > +            image_dir = self.image_dir
> > +        if iso_dir == None:
> > +            iso_dir = self.iso_dir
> > +        return VM(name, params, qemu_path, image_dir, iso_dir)
> > +
> >   
> 
>     return VM(name or self.name, params or self.params.copy(), ...)

That's much nicer, but it doesn't allow name to be "" and doesn't allow
params to be {}. It was once common for name to be "". I'm not sure
there's a good reason to allow for it now.

In any case, the shorter syntax can be used for the other parameters.
I used '== None' for all of them just to be consistent.

> -- 
> error compiling committee.c: too many arguments to function
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [KVM-AUTOTEST PATCH] Use new function VM.get_name() to get the VM's name, instead of VM.name

[Lucas Meneghel Rodrigues]

On Sun, 2009-05-24 at 18:46 +0300, Michael Goldish wrote:
> kvm_vm.py: add function VM.get_name().
> kvm_preprocessing.py: use VM.get_name() instead of directly accessing the .name
> attribute.

Are there any advantages of creating this method over directly accessing
the attribute?

> Signed-off-by: Michael Goldish <mgoldish@redhat.com>
> ---
>  client/tests/kvm_runtest_2/kvm_preprocessing.py |    6 +++---
>  client/tests/kvm_runtest_2/kvm_vm.py            |    4 ++++
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/client/tests/kvm_runtest_2/kvm_preprocessing.py b/client/tests/kvm_runtest_2/kvm_preprocessing.py
> index c9eb35d..bcabf5a 100644
> --- a/client/tests/kvm_runtest_2/kvm_preprocessing.py
> +++ b/client/tests/kvm_runtest_2/kvm_preprocessing.py
> @@ -178,7 +178,7 @@ def preprocess(test, params, env):
>          if vm.is_dead():
>              continue
>          if not vm.verify_process_identity():
> -            kvm_log.debug("VM '%s' seems to have been replaced by another process" % vm.name)
> +            kvm_log.debug("VM '%s' seems to have been replaced by another process" % vm.get_name())
>              vm.pid = None
>  
>      # Destroy and remove VMs that are no longer needed in the environment
> @@ -187,8 +187,8 @@ def preprocess(test, params, env):
>          vm = env[key]
>          if not kvm_utils.is_vm(vm):
>              continue
> -        if not vm.name in requested_vms:
> -            kvm_log.debug("VM '%s' found in environment but not required for test; removing it..." % vm.name)
> +        if not vm.get_name() in requested_vms:
> +            kvm_log.debug("VM '%s' found in environment but not required for test; removing it..." % vm.get_name())
>              vm.destroy()
>              del env[key]
>  
> diff --git a/client/tests/kvm_runtest_2/kvm_vm.py b/client/tests/kvm_runtest_2/kvm_vm.py
> index fab839f..df99859 100644
> --- a/client/tests/kvm_runtest_2/kvm_vm.py
> +++ b/client/tests/kvm_runtest_2/kvm_vm.py
> @@ -454,6 +454,10 @@ class VM:
>          """Return True iff the VM's PID does not exist."""
>          return not kvm_utils.pid_exists(self.pid)
>  
> +    def get_name(self):
> +        """Return the VM's name."""
> +        return self.name
> +
>      def get_params(self):
>          """Return the VM's params dict.
>  
-- 
Lucas Meneghel Rodrigues
Software Engineer (QE)
Red Hat - Emerging Technologies

Re: [KVM-AUTOTEST PATCH] kvm_runtest_2.py: use pickle instead of shelve when loading/saving env

[Lucas Meneghel Rodrigues]

On Sun, 2009-05-24 at 18:46 +0300, Michael Goldish wrote:
> pickle allows more control over the load/save process. Specifically, it enables
> us to dump the contents of an object to disk without having to unpickle it.
> shelve, which uses pickle, seems to pickle and unpickle every time sync() is
> called. This is bad for classes that need to be unpickled only once per test
> (such a class will be introduced in a future patch).

Looks good to me.

> Signed-off-by: Michael Goldish <mgoldish@redhat.com>
> ---
>  client/tests/kvm_runtest_2/kvm_runtest_2.py |   29 ++++++++++++++++++++------
>  1 files changed, 22 insertions(+), 7 deletions(-)
> 
> diff --git a/client/tests/kvm_runtest_2/kvm_runtest_2.py b/client/tests/kvm_runtest_2/kvm_runtest_2.py
> index a69951b..5f7f6ad 100644
> --- a/client/tests/kvm_runtest_2/kvm_runtest_2.py
> +++ b/client/tests/kvm_runtest_2/kvm_runtest_2.py
> @@ -3,9 +3,9 @@
>  import sys
>  import os
>  import time
> -import shelve
>  import random
>  import resource
> +import cPickle
>  
>  from autotest_lib.client.bin import test
>  from autotest_lib.client.common_lib import error
> @@ -18,6 +18,22 @@ class test_routine:
>          self.routine = None
>  
> 
> +def dump_env(obj, filename):
> +    file = open(filename, "w")
> +    cPickle.dump(obj, file)
> +    file.close()
> +
> +
> +def load_env(filename, default=None):
> +    try:
> +        file = open(filename, "r")
> +    except:
> +        return default
> +    obj = cPickle.load(file)
> +    file.close()
> +    return obj
> +
> +
>  class kvm_runtest_2(test.test):
>      version = 1
>  
> @@ -65,7 +81,7 @@ class kvm_runtest_2(test.test):
>  
>          # Open the environment file
>          env_filename = os.path.join(self.bindir, params.get("env", "env"))
> -        env = shelve.open(env_filename, writeback=True)
> +        env = load_env(env_filename, {})
>          kvm_log.debug("Contents of environment: %s" % str(env))
>  
>          try:
> @@ -87,24 +103,23 @@ class kvm_runtest_2(test.test):
>  
>                  # Preprocess
>                  kvm_preprocessing.preprocess(self, params, env)
> -                env.sync()
> +                dump_env(env, env_filename)
>                  # Run the test function
>                  routine_obj.routine(self, params, env)
> -                env.sync()
> +                dump_env(env, env_filename)
>  
>              except Exception, e:
>                  kvm_log.error("Test failed: %s" % e)
>                  kvm_log.debug("Postprocessing on error...")
>                  kvm_preprocessing.postprocess_on_error(self, params, env)
> -                env.sync()
> +                dump_env(env, env_filename)
>                  raise
>  
>          finally:
>              # Postprocess
>              kvm_preprocessing.postprocess(self, params, env)
>              kvm_log.debug("Contents of environment: %s" % str(env))
> -            env.sync()
> -            env.close()
> +            dump_env(env, env_filename)
>  
>      def postprocess(self):
>          pass
-- 
Lucas Meneghel Rodrigues
Software Engineer (QE)
Red Hat - Emerging Technologies

Re: [KVM-AUTOTEST PATCH] kvm_runtest_2.py: use environment filename specified by the 'env' parameter

[Lucas Meneghel Rodrigues]

On Sun, 2009-05-24 at 18:46 +0300, Michael Goldish wrote:
> Do not use hardcoded environment filename 'env'. Instead use the value
> specified by the 'env' parameter. If unspecified, use 'env' as the filename.

Looks good to me!

> This is important for parallel execution; it may be necessary to use a separate
> environment file for each process.
> 
> Signed-off-by: Michael Goldish <mgoldish@redhat.com>
> ---
>  client/tests/kvm_runtest_2/kvm_runtest_2.py |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/client/tests/kvm_runtest_2/kvm_runtest_2.py b/client/tests/kvm_runtest_2/kvm_runtest_2.py
> index fda7282..a69951b 100644
> --- a/client/tests/kvm_runtest_2/kvm_runtest_2.py
> +++ b/client/tests/kvm_runtest_2/kvm_runtest_2.py
> @@ -64,7 +64,7 @@ class kvm_runtest_2(test.test):
>              self.write_test_keyval({key: params[key]})
>  
>          # Open the environment file
> -        env_filename = os.path.join(self.bindir, "env")
> +        env_filename = os.path.join(self.bindir, params.get("env", "env"))
>          env = shelve.open(env_filename, writeback=True)
>          kvm_log.debug("Contents of environment: %s" % str(env))
>  
-- 
Lucas Meneghel Rodrigues
Software Engineer (QE)
Red Hat - Emerging Technologies

Re: [KVM-AUTOTEST PATCH] kvm_vm.py: add new VM parameter 'x11_display' that controls $DISPLAY

[Lucas Meneghel Rodrigues]

On Sun, 2009-05-24 at 18:46 +0300, Michael Goldish wrote:
> If x11_display is specified, the DISPLAY environment variable is set to this
> value for the QEMU process. This may be useful for SDL rendering.

Looks good to me!

> Also add some comments.
> 
> Signed-off-by: Michael Goldish <mgoldish@redhat.com>
> ---
>  client/tests/kvm_runtest_2/kvm_vm.py |   12 +++++++++++-
>  1 files changed, 11 insertions(+), 1 deletions(-)
> 
> diff --git a/client/tests/kvm_runtest_2/kvm_vm.py b/client/tests/kvm_runtest_2/kvm_vm.py
> index 9571a3b..7a4ce4a 100644
> --- a/client/tests/kvm_runtest_2/kvm_vm.py
> +++ b/client/tests/kvm_runtest_2/kvm_vm.py
> @@ -173,6 +173,8 @@ class VM:
>                  (iso_dir is pre-pended to the ISO filename)
>              extra_params -- a string to append to the qemu command
>              ssh_port -- should be 22 for SSH, 23 for Telnet
> +            x11_display -- if specified, the DISPLAY environment variable will be be set
> +                to this value for the qemu process (useful for SDL rendering)
>              images -- a list of image object names, separated by spaces
>              nics -- a list of NIC object names, separated by spaces
>  
> @@ -198,8 +200,16 @@ class VM:
>          if iso_dir == None:
>              iso_dir = self.iso_dir
>  
> -        qemu_cmd = qemu_path
> +        # Start constructing the qemu command
> +        qemu_cmd = ""
> +        # Set the X11 display parameter if requested
> +        if params.get("x11_display"):
> +            qemu_cmd += "DISPLAY=%s " % params.get("x11_display")
> +        # Add the qemu binary
> +        qemu_cmd += qemu_path
> +        # Add the VM's name
>          qemu_cmd += " -name '%s'" % name
> +        # Add the monitor socket parameter
>          qemu_cmd += " -monitor unix:%s,server,nowait" % self.monitor_file_name
>  
>          for image_name in kvm_utils.get_sub_dict_names(params, "images"):
-- 
Lucas Meneghel Rodrigues
Software Engineer (QE)
Red Hat - Emerging Technologies

Re: [KVM-AUTOTEST PATCH] VM.create(): always destroy() the VM before attempting to start it

[Lucas Meneghel Rodrigues]

On Sun, 2009-05-24 at 18:46 +0300, Michael Goldish wrote:
> Also, don't do it in kvm_preprocessing.py since it's now done in kvm_vm.py.

Looks good to me!

> Signed-off-by: Michael Goldish <mgoldish@redhat.com>
> ---
>  client/tests/kvm_runtest_2/kvm_preprocessing.py |    1 -
>  client/tests/kvm_runtest_2/kvm_vm.py            |    2 ++
>  2 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/client/tests/kvm_runtest_2/kvm_preprocessing.py b/client/tests/kvm_runtest_2/kvm_preprocessing.py
> index bcabf5a..9ccaf78 100644
> --- a/client/tests/kvm_runtest_2/kvm_preprocessing.py
> +++ b/client/tests/kvm_runtest_2/kvm_preprocessing.py
> @@ -84,7 +84,6 @@ def preprocess_vm(test, params, env, name):
>              start_vm = True
>  
>      if start_vm:
> -        vm.destroy()
>          if not vm.create(name, params, qemu_path, image_dir, iso_dir, for_migration):
>              message = "Could not start VM"
>              kvm_log.error(message)
> diff --git a/client/tests/kvm_runtest_2/kvm_vm.py b/client/tests/kvm_runtest_2/kvm_vm.py
> index df99859..a1462c6 100644
> --- a/client/tests/kvm_runtest_2/kvm_vm.py
> +++ b/client/tests/kvm_runtest_2/kvm_vm.py
> @@ -238,6 +238,8 @@ class VM:
>          stored in the class attributes is used, and if it is supplied, it is stored
>          for later use.
>          """
> +        self.destroy()
> +
>          if name != None:
>              self.name = name
>          if params != None:
-- 
Lucas Meneghel Rodrigues
Software Engineer (QE)
Red Hat - Emerging Technologies

Re: [KVM-AUTOTEST PATCH] kvm_utils.py: remote_login(): improve regular expression matching

[Lucas Meneghel Rodrigues]

On Sun, 2009-05-24 at 18:46 +0300, Michael Goldish wrote:
> 1. Make the 'login:' regular expression stricter so it doesn't match
> 'Last login: ...' messages.
> 2. Make the 'password:' regular expression stricter.
> 3. Handle 'Connection refused' messages.

Looks good to me

> Signed-off-by: Michael Goldish <mgoldish@redhat.com>
> ---
>  client/tests/kvm_runtest_2/kvm_utils.py |   13 +++++++++----
>  1 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/client/tests/kvm_runtest_2/kvm_utils.py b/client/tests/kvm_runtest_2/kvm_utils.py
> index be8ad95..5736cf6 100644
> --- a/client/tests/kvm_runtest_2/kvm_utils.py
> +++ b/client/tests/kvm_runtest_2/kvm_utils.py
> @@ -413,7 +413,8 @@ def remote_login(command, password, prompt, linesep="\n", timeout=10):
>  
>      while True:
>          (match, text) = sub.read_until_last_line_matches(
> -                ["[Aa]re you sure", "[Pp]assword:", "[Ll]ogin:", "[Cc]onnection.*closed", prompt],
> +                [r"[Aa]re you sure", r"[Pp]assword:\s*$", r"^\s*[Ll]ogin:\s*$",
> +                    r"[Cc]onnection.*closed", r"[Cc]onnection.*refused", prompt],
>                  timeout=timeout, internal_timeout=0.5)
>          if match == 0:  # "Are you sure you want to continue connecting"
>              kvm_log.debug("Got 'Are you sure...'; sending 'yes'")
> @@ -437,11 +438,15 @@ def remote_login(command, password, prompt, linesep="\n", timeout=10):
>              kvm_log.debug("Got 'Connection closed'")
>              sub.close()
>              return None
> -        elif match == 4:  # prompt
> +        elif match == 4:  # "Connection refused"
> +            kvm_log.debug("Got 'Connection refused'")
> +            sub.close()
> +            return None
> +        elif match == 5:  # prompt
>              kvm_log.debug("Got shell prompt -- logged in")
>              return sub
>          else:  # match == None
> -            kvm_log.debug("Timeout or process terminated")
> +            kvm_log.debug("Timeout elapsed or process terminated")
>              sub.close()
>              return None
>  
> @@ -470,7 +475,7 @@ def remote_scp(command, password, timeout=300, login_timeout=10):
>  
>      while True:
>          (match, text) = sub.read_until_last_line_matches(
> -                ["[Aa]re you sure", "[Pp]assword:", "lost connection"],
> +                [r"[Aa]re you sure", r"[Pp]assword:\s*$", r"lost connection"],
>                  timeout=_timeout, internal_timeout=0.5)
>          if match == 0:  # "Are you sure you want to continue connecting"
>              kvm_log.debug("Got 'Are you sure...'; sending 'yes'")
-- 
Lucas Meneghel Rodrigues
Software Engineer (QE)
Red Hat - Emerging Technologies

Re: [KVM-AUTOTEST PATCH] Use new function VM.get_name() to get the VM's name, instead of VM.name

[Michael Goldish]

----- "Lucas Meneghel Rodrigues" <lmr@redhat.com> wrote:

> On Sun, 2009-05-24 at 18:46 +0300, Michael Goldish wrote:
> > kvm_vm.py: add function VM.get_name().
> > kvm_preprocessing.py: use VM.get_name() instead of directly
> accessing the .name
> > attribute.
> 
> Are there any advantages of creating this method over directly
> accessing
> the attribute?

Not really, it's just that everywhere else we use "get" interface
functions and I like to be consistent. I also thought it was good
OOP practice to make the interface independent of the internal
implementation. For example, we may choose not to store the name
attribute in the class instance, but rather retrieve it or generate
it (if it gets more complex) from the params upon request.

> > Signed-off-by: Michael Goldish <mgoldish@redhat.com>
> > ---
> >  client/tests/kvm_runtest_2/kvm_preprocessing.py |    6 +++---
> >  client/tests/kvm_runtest_2/kvm_vm.py            |    4 ++++
> >  2 files changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/client/tests/kvm_runtest_2/kvm_preprocessing.py
> b/client/tests/kvm_runtest_2/kvm_preprocessing.py
> > index c9eb35d..bcabf5a 100644
> > --- a/client/tests/kvm_runtest_2/kvm_preprocessing.py
> > +++ b/client/tests/kvm_runtest_2/kvm_preprocessing.py
> > @@ -178,7 +178,7 @@ def preprocess(test, params, env):
> >          if vm.is_dead():
> >              continue
> >          if not vm.verify_process_identity():
> > -            kvm_log.debug("VM '%s' seems to have been replaced by
> another process" % vm.name)
> > +            kvm_log.debug("VM '%s' seems to have been replaced by
> another process" % vm.get_name())
> >              vm.pid = None
> >  
> >      # Destroy and remove VMs that are no longer needed in the
> environment
> > @@ -187,8 +187,8 @@ def preprocess(test, params, env):
> >          vm = env[key]
> >          if not kvm_utils.is_vm(vm):
> >              continue
> > -        if not vm.name in requested_vms:
> > -            kvm_log.debug("VM '%s' found in environment but not
> required for test; removing it..." % vm.name)
> > +        if not vm.get_name() in requested_vms:
> > +            kvm_log.debug("VM '%s' found in environment but not
> required for test; removing it..." % vm.get_name())
> >              vm.destroy()
> >              del env[key]
> >  
> > diff --git a/client/tests/kvm_runtest_2/kvm_vm.py
> b/client/tests/kvm_runtest_2/kvm_vm.py
> > index fab839f..df99859 100644
> > --- a/client/tests/kvm_runtest_2/kvm_vm.py
> > +++ b/client/tests/kvm_runtest_2/kvm_vm.py
> > @@ -454,6 +454,10 @@ class VM:
> >          """Return True iff the VM's PID does not exist."""
> >          return not kvm_utils.pid_exists(self.pid)
> >  
> > +    def get_name(self):
> > +        """Return the VM's name."""
> > +        return self.name
> > +
> >      def get_params(self):
> >          """Return the VM's params dict.
> >  
> -- 
> Lucas Meneghel Rodrigues
> Software Engineer (QE)
> Red Hat - Emerging Technologies

Re: [KVM-AUTOTEST PATCH] Use new function VM.get_name() to get the VM's name, instead of VM.name

[Lucas Meneghel Rodrigues]

On Wed, 2009-06-03 at 01:01 -0400, Michael Goldish wrote:
> ----- "Lucas Meneghel Rodrigues" <lmr@redhat.com> wrote:
> 
> > On Sun, 2009-05-24 at 18:46 +0300, Michael Goldish wrote:
> > > kvm_vm.py: add function VM.get_name().
> > > kvm_preprocessing.py: use VM.get_name() instead of directly
> > accessing the .name
> > > attribute.
> > 
> > Are there any advantages of creating this method over directly
> > accessing
> > the attribute?
> 
> Not really, it's just that everywhere else we use "get" interface
> functions and I like to be consistent. I also thought it was good
> OOP practice to make the interface independent of the internal
> implementation. For example, we may choose not to store the name
> attribute in the class instance, but rather retrieve it or generate
> it (if it gets more complex) from the params upon request.

Fair enough, now that you've put it that way I agree with you.

> > > Signed-off-by: Michael Goldish <mgoldish@redhat.com>
> > > ---
> > >  client/tests/kvm_runtest_2/kvm_preprocessing.py |    6 +++---
> > >  client/tests/kvm_runtest_2/kvm_vm.py            |    4 ++++
> > >  2 files changed, 7 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/client/tests/kvm_runtest_2/kvm_preprocessing.py
> > b/client/tests/kvm_runtest_2/kvm_preprocessing.py
> > > index c9eb35d..bcabf5a 100644
> > > --- a/client/tests/kvm_runtest_2/kvm_preprocessing.py
> > > +++ b/client/tests/kvm_runtest_2/kvm_preprocessing.py
> > > @@ -178,7 +178,7 @@ def preprocess(test, params, env):
> > >          if vm.is_dead():
> > >              continue
> > >          if not vm.verify_process_identity():
> > > -            kvm_log.debug("VM '%s' seems to have been replaced by
> > another process" % vm.name)
> > > +            kvm_log.debug("VM '%s' seems to have been replaced by
> > another process" % vm.get_name())
> > >              vm.pid = None
> > >  
> > >      # Destroy and remove VMs that are no longer needed in the
> > environment
> > > @@ -187,8 +187,8 @@ def preprocess(test, params, env):
> > >          vm = env[key]
> > >          if not kvm_utils.is_vm(vm):
> > >              continue
> > > -        if not vm.name in requested_vms:
> > > -            kvm_log.debug("VM '%s' found in environment but not
> > required for test; removing it..." % vm.name)
> > > +        if not vm.get_name() in requested_vms:
> > > +            kvm_log.debug("VM '%s' found in environment but not
> > required for test; removing it..." % vm.get_name())
> > >              vm.destroy()
> > >              del env[key]
> > >  
> > > diff --git a/client/tests/kvm_runtest_2/kvm_vm.py
> > b/client/tests/kvm_runtest_2/kvm_vm.py
> > > index fab839f..df99859 100644
> > > --- a/client/tests/kvm_runtest_2/kvm_vm.py
> > > +++ b/client/tests/kvm_runtest_2/kvm_vm.py
> > > @@ -454,6 +454,10 @@ class VM:
> > >          """Return True iff the VM's PID does not exist."""
> > >          return not kvm_utils.pid_exists(self.pid)
> > >  
> > > +    def get_name(self):
> > > +        """Return the VM's name."""
> > > +        return self.name
> > > +
> > >      def get_params(self):
> > >          """Return the VM's params dict.
> > >  
> > -- 
> > Lucas Meneghel Rodrigues
> > Software Engineer (QE)
> > Red Hat - Emerging Technologies
-- 
Lucas Meneghel Rodrigues
Software Engineer (QE)
Red Hat - Emerging Technologies