| Message ID | 1250069685-17727-3-git-send-email-avi@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2009-08-12 at 12:34 +0300, Avi Kivity wrote: > This is faster since we don't need to fork/exec/wait for an external > program each time. > > Signed-off-by: Avi Kivity <avi@redhat.com> > --- > client/tests/kvm/kvm_guest_wizard.py | 6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/client/tests/kvm/kvm_guest_wizard.py b/client/tests/kvm/kvm_guest_wizard.py > index 73b830e..f9e5476 100644 > --- a/client/tests/kvm/kvm_guest_wizard.py > +++ b/client/tests/kvm/kvm_guest_wizard.py > @@ -1,6 +1,7 @@ > import os, time, md5, re, shutil, logging > from autotest_lib.client.common_lib import utils, error > import kvm_utils, ppm_utils, kvm_subprocess > +import PIL.Image > > """ > Utilities to perform automatic guest installation using step files. > @@ -110,9 +111,8 @@ 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) > + image = PIL.Image.open(scrdump_filename) > + image.save(history_scrdump_filename, format = 'JPEG', quality = 30) Looks great, but since the python imaging library is an external library, we need to handle import failures. We can't guarantee that it will allways be installed, so we just degrade functionality gracefully in the case is not present. > # Compare md5sum of barrier region with the expected md5sum > calced_md5sum = ppm_utils.get_region_md5sum(w, h, data, x1, y1, dx, dy, -- 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
On 08/12/2009 03:44 PM, Lucas Meneghel Rodrigues wrote: >> @@ -110,9 +111,8 @@ 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) >> + image = PIL.Image.open(scrdump_filename) >> + image.save(history_scrdump_filename, format = 'JPEG', quality = 30) >> > Utilities to perform automatic guest installation using step files. > > Looks great, but since the python imaging library is an external > library, we need to handle import failures. We can't guarantee that it > will allways be installed, so we just degrade functionality gracefully > in the case is not present. > Why not require it unconditionally? It's not new and installing it is trivial.
On Wed, 2009-08-12 at 16:00 +0300, Avi Kivity wrote: > On 08/12/2009 03:44 PM, Lucas Meneghel Rodrigues wrote: > >> @@ -110,9 +111,8 @@ 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) > >> + image = PIL.Image.open(scrdump_filename) > >> + image.save(history_scrdump_filename, format = 'JPEG', quality = 30) > >> > > Utilities to perform automatic guest installation using step files. > > > > Looks great, but since the python imaging library is an external > > library, we need to handle import failures. We can't guarantee that it > > will allways be installed, so we just degrade functionality gracefully > > in the case is not present. > Why not require it unconditionally? It's not new and installing it is > trivial. Just general autotest policy I try to follow. For client side testing, we try to make as few assumptions as possible about the machine where we are going to run the client, except to have a python install > 2.4 and a working toolchain (to build C/C++ programs). We try to not rely on anything but those 2 items. For the autotest server machine it's OK to depend on external libraries. I also agree that this is a special case: * PIL is almost ubiquitous across linux distributions * It comes installed by default in most setups * It's not like we are going to test kvm on dramatically old linux systems But I prefer to follow the project policy when possible. The reason why I accepted the original code that Michael wrote to perform the conversion was graceful degradation of functionality (if you don't have ImageMagick installed, the test will not abort). We could handle import failures in a way that supports graceful degradation and prints a warning message saying that the user don't have PIL installed, and that the conversion functionality only works when having PIL installed. What do you think? If you still think it's not worth the effort of doing this import handling I am going to think a little bit more, and eventually apply this. -- 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 --git a/client/tests/kvm/kvm_guest_wizard.py b/client/tests/kvm/kvm_guest_wizard.py index 73b830e..f9e5476 100644 --- a/client/tests/kvm/kvm_guest_wizard.py +++ b/client/tests/kvm/kvm_guest_wizard.py @@ -1,6 +1,7 @@ import os, time, md5, re, shutil, logging from autotest_lib.client.common_lib import utils, error import kvm_utils, ppm_utils, kvm_subprocess +import PIL.Image """ Utilities to perform automatic guest installation using step files. @@ -110,9 +111,8 @@ 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) + image = PIL.Image.open(scrdump_filename) + image.save(history_scrdump_filename, format = 'JPEG', quality = 30) # Compare md5sum of barrier region with the expected md5sum calced_md5sum = ppm_utils.get_region_md5sum(w, h, data, x1, y1, dx, dy,
This is faster since we don't need to fork/exec/wait for an external program each time. Signed-off-by: Avi Kivity <avi@redhat.com> --- client/tests/kvm/kvm_guest_wizard.py | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)