diff mbox

KVM test: Remove unnecessary callouts to external programs

Message ID 1250089200-7522-1-git-send-email-lmr@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lucas Meneghel Rodrigues Aug. 12, 2009, 3 p.m. UTC 
From: Avi Kivity <avi@redhat.com>

Calling out to external commands is slow, noisy, and unreliable.  This
patchset replaces two such cases with Python equivalents.

  * Replace subprocess 'rm' by equivalent Python code
  * Convert images to JPEG and PNG using PIL instead of an external program.
  If we don't have the python imaging library installed, log a warning
  to the user and just degrade functionality accordingly.

Signed-off-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
---
 client/tests/kvm/kvm_guest_wizard.py  |   19 ++++++++++++++-----
 client/tests/kvm/kvm_preprocessing.py |   23 ++++++++++++++++-------
 2 files changed, 30 insertions(+), 12 deletions(-)

Comments

Lucas Meneghel Rodrigues Aug. 12, 2009, 3:18 p.m. UTC | #1 
On Wed, Aug 12, 2009 at 12:00 PM, Lucas Meneghel
Rodrigues<lmr@redhat.com> wrote:
> From: Avi Kivity <avi@redhat.com>
>
> Calling out to external commands is slow, noisy, and unreliable.  This
> patchset replaces two such cases with Python equivalents.
>
>  * Replace subprocess 'rm' by equivalent Python code
>  * Convert images to JPEG and PNG using PIL instead of an external program.
>  If we don't have the python imaging library installed, log a warning
>  to the user and just degrade functionality accordingly.

Applied, thanks Avi!

> Signed-off-by: Avi Kivity <avi@redhat.com>
> Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
> ---
>  client/tests/kvm/kvm_guest_wizard.py  |   19 ++++++++++++++-----
>  client/tests/kvm/kvm_preprocessing.py |   23 ++++++++++++++++-------
>  2 files changed, 30 insertions(+), 12 deletions(-)
>
> diff --git a/client/tests/kvm/kvm_guest_wizard.py b/client/tests/kvm/kvm_guest_wizard.py
> index 73b830e..dd1879d 100644
> --- a/client/tests/kvm/kvm_guest_wizard.py
> +++ b/client/tests/kvm/kvm_guest_wizard.py
> @@ -1,6 +1,13 @@
>  import os, time, md5, re, shutil, logging
>  from autotest_lib.client.common_lib import utils, error
>  import kvm_utils, ppm_utils, kvm_subprocess
> +try:
> +    import PIL.Image
> +except ImportError:
> +    logging.warning('No python imaging library installed. PPM image '
> +                    'conversion to JPEG disabled. In order to enable it, '
> +                    'please install python-imaging or the equivalent for your '
> +                    'distro.')
>
>  """
>  Utilities to perform automatic guest installation using step files.
> @@ -110,9 +117,12 @@ def barrier_2(vm, words, params, debug_dir, data_scrdump_filename,
>             history_scrdump_filename = os.path.join(history_dir,
>                     "scrdump-step_%s-%s.jpg" % (current_step_num,
>                                                 time.strftime("%Y%m%d-%H%M%S")))
> -            kvm_subprocess.run_fg("convert -quality 30 %s %s" %
> -                                  (scrdump_filename, history_scrdump_filename),
> -                                  logging.debug, "(convert) ", timeout=30)
> +            try:
> +                image = PIL.Image.open(scrdump_filename)
> +                image.save(history_scrdump_filename, format = 'JPEG',
> +                           quality = 30)
> +            except NameError:
> +                pass
>
>         # Compare md5sum of barrier region with the expected md5sum
>         calced_md5sum = ppm_utils.get_region_md5sum(w, h, data, x1, y1, dx, dy,
> @@ -120,8 +130,7 @@ def barrier_2(vm, words, params, debug_dir, data_scrdump_filename,
>         if calced_md5sum == md5sum:
>             # Success -- remove screendump history unless requested not to
>             if keep_screendump_history and not keep_all_history:
> -                kvm_subprocess.run_fg("rm -rvf %s" % history_dir,
> -                                      logging.debug, "(rm) ", timeout=30)
> +                shutil.rmtree(history_dir)
>             # Report success
>             return True
>
> diff --git a/client/tests/kvm/kvm_preprocessing.py b/client/tests/kvm/kvm_preprocessing.py
> index d118826..2e55b34 100644
> --- a/client/tests/kvm/kvm_preprocessing.py
> +++ b/client/tests/kvm/kvm_preprocessing.py
> @@ -1,7 +1,14 @@
> -import sys, os, time, commands, re, logging, signal
> +import sys, os, time, commands, re, logging, signal, glob
>  from autotest_lib.client.bin import test
>  from autotest_lib.client.common_lib import error
>  import kvm_vm, kvm_utils, kvm_subprocess
> +try:
> +    import PIL.Image
> +except ImportError:
> +    logging.warning('No python imaging library installed. PPM image '
> +                    'conversion to JPEG disabled. In order to enable it, '
> +                    'please install python-imaging or the equivalent for your '
> +                    'distro.')
>
>
>  def preprocess_image(test, params):
> @@ -260,17 +267,19 @@ def postprocess(test, params, env):
>     if params.get("convert_ppm_files_to_png") == "yes":
>         logging.debug("'convert_ppm_files_to_png' specified; converting PPM"
>                       " files to PNG format...")
> -        mogrify_cmd = ("mogrify -format png %s" %
> -                       os.path.join(test.debugdir, "*.ppm"))
> -        kvm_subprocess.run_fg(mogrify_cmd, logging.debug, "(mogrify) ",
> -                              timeout=30.0)
> +        try:
> +            for f in glob.glob(os.path.join(test.debugdir, "*.ppm")):
> +                image = PIL.Image.open(f)
> +                image.save(history_scrdump_filename, format = 'PNG')
> +        except NameError:
> +            pass
>
>     # Should we keep the PPM files?
>     if params.get("keep_ppm_files") != "yes":
>         logging.debug("'keep_ppm_files' not specified; removing all PPM files"
>                       " from debug dir...")
> -        rm_cmd = "rm -vf %s" % os.path.join(test.debugdir, "*.ppm")
> -        kvm_subprocess.run_fg(rm_cmd, logging.debug, "(rm) ", timeout=5.0)
> +        for f in glob.glob(os.path.join(test.debugdir, '*.ppm')):
> +            os.unlink(f)
>
>     # Execute any post_commands
>     if params.get("post_command"):
> --
> 1.6.2.5
>
> --
> 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
>
diff mbox

Patch

diff --git a/client/tests/kvm/kvm_guest_wizard.py b/client/tests/kvm/kvm_guest_wizard.py
index 73b830e..dd1879d 100644
--- a/client/tests/kvm/kvm_guest_wizard.py
+++ b/client/tests/kvm/kvm_guest_wizard.py
@@ -1,6 +1,13 @@ 
 import os, time, md5, re, shutil, logging
 from autotest_lib.client.common_lib import utils, error
 import kvm_utils, ppm_utils, kvm_subprocess
+try:
+    import PIL.Image
+except ImportError:
+    logging.warning('No python imaging library installed. PPM image '
+                    'conversion to JPEG disabled. In order to enable it, '
+                    'please install python-imaging or the equivalent for your '
+                    'distro.')
 
 """
 Utilities to perform automatic guest installation using step files.
@@ -110,9 +117,12 @@  def barrier_2(vm, words, params, debug_dir, data_scrdump_filename,
             history_scrdump_filename = os.path.join(history_dir,
                     "scrdump-step_%s-%s.jpg" % (current_step_num,
                                                 time.strftime("%Y%m%d-%H%M%S")))
-            kvm_subprocess.run_fg("convert -quality 30 %s %s" %
-                                  (scrdump_filename, history_scrdump_filename),
-                                  logging.debug, "(convert) ", timeout=30)
+            try:
+                image = PIL.Image.open(scrdump_filename)
+                image.save(history_scrdump_filename, format = 'JPEG',
+                           quality = 30)
+            except NameError:
+                pass
 
         # Compare md5sum of barrier region with the expected md5sum
         calced_md5sum = ppm_utils.get_region_md5sum(w, h, data, x1, y1, dx, dy,
@@ -120,8 +130,7 @@  def barrier_2(vm, words, params, debug_dir, data_scrdump_filename,
         if calced_md5sum == md5sum:
             # Success -- remove screendump history unless requested not to
             if keep_screendump_history and not keep_all_history:
-                kvm_subprocess.run_fg("rm -rvf %s" % history_dir,
-                                      logging.debug, "(rm) ", timeout=30)
+                shutil.rmtree(history_dir)
             # Report success
             return True
 
diff --git a/client/tests/kvm/kvm_preprocessing.py b/client/tests/kvm/kvm_preprocessing.py
index d118826..2e55b34 100644
--- a/client/tests/kvm/kvm_preprocessing.py
+++ b/client/tests/kvm/kvm_preprocessing.py
@@ -1,7 +1,14 @@ 
-import sys, os, time, commands, re, logging, signal
+import sys, os, time, commands, re, logging, signal, glob
 from autotest_lib.client.bin import test
 from autotest_lib.client.common_lib import error
 import kvm_vm, kvm_utils, kvm_subprocess
+try:
+    import PIL.Image
+except ImportError:
+    logging.warning('No python imaging library installed. PPM image '
+                    'conversion to JPEG disabled. In order to enable it, '
+                    'please install python-imaging or the equivalent for your '
+                    'distro.')
 
 
 def preprocess_image(test, params):
@@ -260,17 +267,19 @@  def postprocess(test, params, env):
     if params.get("convert_ppm_files_to_png") == "yes":
         logging.debug("'convert_ppm_files_to_png' specified; converting PPM"
                       " files to PNG format...")
-        mogrify_cmd = ("mogrify -format png %s" %
-                       os.path.join(test.debugdir, "*.ppm"))
-        kvm_subprocess.run_fg(mogrify_cmd, logging.debug, "(mogrify) ",
-                              timeout=30.0)
+        try:
+            for f in glob.glob(os.path.join(test.debugdir, "*.ppm")):
+                image = PIL.Image.open(f)
+                image.save(history_scrdump_filename, format = 'PNG')
+        except NameError:
+            pass
 
     # Should we keep the PPM files?
     if params.get("keep_ppm_files") != "yes":
         logging.debug("'keep_ppm_files' not specified; removing all PPM files"
                       " from debug dir...")
-        rm_cmd = "rm -vf %s" % os.path.join(test.debugdir, "*.ppm")
-        kvm_subprocess.run_fg(rm_cmd, logging.debug, "(rm) ", timeout=5.0)
+        for f in glob.glob(os.path.join(test.debugdir, '*.ppm')):
+            os.unlink(f)
 
     # Execute any post_commands
     if params.get("post_command"):