On Fri, 2010-01-29 at 18:17 +0800, Feng Yang wrote: > Add check_image.py in scripts folder. It will performance qemu-img info > and qemu-img check. > Add check_image.py as post command of qcow2 variant. Hi Feng, thanks for your patch! See some comments below: > Signed-off-by: Feng Yang <fyang@xxxxxxxxxx> > --- > client/tests/kvm/scripts/check_image.py | 89 +++++++++++++++++++++++++++++++ > client/tests/kvm/tests_base.cfg.sample | 6 ++ > 2 files changed, 95 insertions(+), 0 deletions(-) > create mode 100755 client/tests/kvm/scripts/check_image.py > > diff --git a/client/tests/kvm/scripts/check_image.py b/client/tests/kvm/scripts/check_image.py > new file mode 100755 > index 0000000..a37f055 > --- /dev/null > +++ b/client/tests/kvm/scripts/check_image.py > @@ -0,0 +1,89 @@ > +import os, sys, commands > + > + > +class ImageCheckError(Exception): > + """ > + Simple wrapper for the builtin Exception class. > + """ > + pass > + > + > +class ImageCheck(object): > + """ > + Check qcow2 image by qemu-img info/check command. > + """ > + def __init__(self): > + """ > + Gets params from environment variables and sets class attributes. > + """ > + self.image_path_list = [] > + client_dir = os.environ['AUTODIR'] > + self.kvm_dir = os.path.join(client_dir, 'tests/kvm') > + if os.environ.has_key('KVM_TEST_img_to_check'): > + img_to_check = os.environ['KVM_TEST_img_to_check'].split() > + else: > + img_to_check = os.environ['KVM_TEST_images'].split() On the above attribution, I didn't understand why we would 'hardcode' a particular image to be checked, it is my understanding that we want to check systematically all the images after each test. > + for img in img_to_check: > + img_name_str = "KVM_TEST_image_name_%s" % img > + if not os.environ.has_key(img_name_str): > + img_name_str = "KVM_TEST_image_name" > + img_format_str = "KVM_TEST_image_format_%s" % img > + if os.environ.has_key(img_format_str): > + image_format = os.environ[img_format_str] > + else: > + image_format = os.environ['KVM_TEST_image_format'] > + if image_format != "qcow2": > + continue > + image_name = os.environ[img_name_str] > + image_filename = "%s.%s" % (image_name, image_format) > + image_filename = os.path.join(self.kvm_dir, image_filename) > + self.image_path_list.append(image_filename) > + > + > + def exec_img_cmd(self, cmd_type, image_path): > + """ > + Run qemu-img info/check on given image. > + > + @param cmd_type: Sub command used together with qemu. > + @param image_path: Real path of the image. > + """ > + if os.environ.has_key('KVM_TEST_qemu_img_binary'): > + qemu_img_path = os.environ['KVM_TEST_qemu_img_binary'] > + else: > + qemu_img_path = os.path.join(self.kvm_dir, 'qemu-img') The above can be put on the class constructor, make qemu_img_cmd a class attribute. > + cmd = ' '.join([qemu_img_path, cmd_type, image_path]) > + print "checking with %s" % cmd > + (status, output) = commands.getstatusoutput(cmd) > + if status or ( cmd_type == "check" and not "No errors" in output ): You can remove the spaces around the expression you want to evaluate. > + msg = "Command %s failed" % cmd > + print output > + return False, msg > + else: > + return True, '' > + > + > + def check_image(self): > + """ > + Run qemu-img info/check to check the image in list. > + > + If the image checking is failed, raise an exception. > + """ > + # Check all the image in list. > + errmsg = [] > + for image_path in self.image_path_list: > + s, o = self.exec_img_cmd('info', image_path) > + if not s: > + errmsg.append(o) > + s, o = self.exec_img_cmd('check', image_path) > + if not s: > + errmsg.append(o) > + > + if len(errmsg) > 0: > + raise ImageCheckError('Errors are found and please check log '\ > + 'for details!') You don't need the \ to break line in the above statement, you can just remove it. > + > +if __name__ == "__main__": > + image_check = ImageCheck() > + image_check.check_image() > diff --git a/client/tests/kvm/tests_base.cfg.sample b/client/tests/kvm/tests_base.cfg.sample > index 2d03f1f..33f9586 100644 > --- a/client/tests/kvm/tests_base.cfg.sample > +++ b/client/tests/kvm/tests_base.cfg.sample > @@ -938,6 +938,12 @@ virtio|virtio_blk|e1000: > variants: > - @qcow2: > image_format = qcow2 > + kill_vm = yes Why the use of the preprocessor directive 'kill_vm' here? qemu-img check doesn't work with the VM turned on? > + post_command = python scripts/check_image.py > + remove_image = no > + # img_to_check = > + post_command_timeout = 600 > + > - vmdk: > only Fedora Ubuntu Windows > only smp2 -- 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