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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html