Re: [PATCH KVM-AUTOTEST 2/2] Convert images to JPEG using PIL instead of an external program

[Lucas Meneghel Rodrigues <lmr@redhat.com>]

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.