Re: [Autotest PATCH] KVM-test: Add a subtest 'qemu_img'

[Yolkfull Chow <yzhou@redhat.com>]

On Tue, Jan 26, 2010 at 03:11:34PM -0200, Lucas Meneghel Rodrigues wrote:
> On Tue, 2010-01-26 at 11:25 +0800, Yolkfull Chow wrote:
> > This is designed to test all subcommands of 'qemu-img' however
> > so far 'commit' is not implemented.
> 
> Hi Yolkful, this is very good! Seeing this test made me think about that
> stand alone autotest module we commited a while ago, that does
> qemu_iotests testsuite on the host.
> 
> Perhaps we could 'port' this module to the kvm test, since it is more

Lucas, do you mean the client-side 'kvmtest' ?

And thanks for comments. :)

> convenient to execute it inside a kvm test job (in a job where we test
> more than 2 build types, for example, we need to execute qemu_img and
> qemu_io_tests for every qemu-img built).
> 
> Could you look at implementing this?
> 
> > * For 'check' subcommand test, it will 'dd' to create a file with specified
> > size and see whether it's supported to be checked. Then convert it to be
> > supported formats (qcow2 and raw so far) to see whether there's error
> > after convertion.
> > 
> > * For 'convert' subcommand test, it will convert both to 'qcow2' and 'raw' from
> > the format specified in config file. And only check 'qcow2' after convertion.
> > 
> > * For 'snapshot' subcommand test, it will create two snapshots and list them.
> > Finally delete them if no errors found.
> > 
> > * For 'info' subcommand test, it simply get output from specified image file.
> > 
> > Signed-off-by: Yolkfull Chow <yzhou@redhat.com>
> > ---
> >  client/tests/kvm/tests/qemu_img.py     |  155 ++++++++++++++++++++++++++++++++
> >  client/tests/kvm/tests_base.cfg.sample |   36 ++++++++
> >  2 files changed, 191 insertions(+), 0 deletions(-)
> >  create mode 100644 client/tests/kvm/tests/qemu_img.py
> > 
> > diff --git a/client/tests/kvm/tests/qemu_img.py b/client/tests/kvm/tests/qemu_img.py
> > new file mode 100644
> > index 0000000..1ae04f0
> > --- /dev/null
> > +++ b/client/tests/kvm/tests/qemu_img.py
> > @@ -0,0 +1,155 @@
> > +import os, logging, commands
> > +from autotest_lib.client.common_lib import error
> > +import kvm_vm
> > +
> > +
> > +def run_qemu_img(test, params, env):
> > +    """
> > +    `qemu-img' functions test:
> > +    1) Judge what subcommand is going to be tested
> > +    2) Run subcommand test
> > +
> > +    @param test: kvm test object
> > +    @param params: Dictionary with the test parameters
> > +    @param env: Dictionary with test environment.
> > +    """
> > +    cmd = params.get("qemu_img_binary")
> 
> It is a good idea to verify if cmd above resolves to an absolute path,
> to avoid problems. If it doesn't resolve, verify if there's the symbolic
> link under kvm test dir pointing to qemu-img, and if it does exist, make
> sure it points to a valid file (ie, symlink is not broken).
> 
> > +    subcommand = params.get("subcommand")
> > +    image_format = params.get("image_format")
> > +    image_name = kvm_vm.get_image_filename(params, test.bindir)
> > +
> > +    def check(img):
> > +        global cmd
> > +        cmd += " check %s" % img
> > +        logging.info("Checking image '%s'..." % img)
> > +        s, o = commands.getstatusoutput(cmd)
> > +        if not (s == 0 or "does not support checks" in o):
> > +            return (False, o)
> > +        return (True, "")
> 
> Please use utils.system_output here instead of the equivalent commands
> API on the above code. This comment applies to all further uses of
> commands.[function].
> 
> > +        
> > +    # Subcommand 'qemu-img check' test
> > +    # This tests will 'dd' to create a specified size file, and check it.
> > +    # Then convert it to supported image_format in each loop and check again.
> > +    def check_test():
> > +        size = params.get("dd_image_size")
> > +        test_image = params.get("dd_image_name")
> > +        create_image_cmd = params.get("create_image_cmd")
> > +        create_image_cmd = create_image_cmd % (test_image, size)
> > +        s, o = commands.getstatusoutput(create_image_cmd)
> > +        if s != 0:
> > +            raise error.TestError("Failed command: %s; Output is: %s" %
> > +                                                 (create_image_cmd, o))
> > +        s, o = check(test_image)
> > +        if not s:
> > +            raise error.TestFail("Failed to check image '%s' with error: %s" %
> > +                                                              (test_image, o))
> > +        for fmt in params.get("supported_image_formats").split():
> > +            output_image = test_image + ".%s" % fmt
> > +            convert(fmt, test_image, output_image)
> > +            s, o = check(output_image)
> > +            if not s:
> > +                raise error.TestFail("Check image '%s' got error: %s" %
> > +                                                     (output_image, o))
> > +            commands.getoutput("rm -f %s" % output_image)
> > +        commands.getoutput("rm -f %s" % test_image)
> > +    #Subcommand 'qemu-img create' test
> > +    def create_test():
> > +        global cmd
> 
> I don't like very much this idea of using a global variable, instead it
> should be preferrable to use a class and have a class attribute with
> 'cmd'. This way it would be safer, since the usage of cmd is
> encapsulated. This comment applies to all further usage of 'global cmd'.
> 
> > +        cmd += " create"
> > +        if params.get("encrypted") == "yes":
> > +            cmd += " -e"
> > +        if params.get("base_image"):
> > +            cmd += " -F %s -b %s" % (params.get("base_image_format"),
> > +                                     params.get("base_image"))
> > +        format = params.get("image_format")
> > +        cmd += " -f %s" % format
> > +        image_name_test = os.path.join(test.bindir,
> > +                          params.get("image_name_test")) + '.' + format
> > +        cmd += " %s %s" % (image_name_test, params.get("image_size_test"))
> > +        s, o = commands.getstatusoutput(cmd)
> > +        if s != 0:
> > +            raise error.TestFail("Create image '%s' failed: %s" %
> > +                                            (image_name_test, o))
> > +        commands.getoutput("rm -f %s" % image_name_test)
> > +    def convert(output_format, image_name, output_filename,
> > +                format=None, compressed="no", encrypted="no"):
> > +        global cmd
> > +        cmd += " convert"
> > +        if compressed == "yes":
> > +            cmd += " -c"
> > +        if encrypted == "yes":
> > +            cmd += " -e"
> > +        if format:
> > +            cmd += " -f %s" % image_format
> > +        cmd += " -O %s" % params.get("dest_image_format")
> > +        cmd += " %s %s" % (image_name, output_filename)
> > +        s, o = commands.getstatusoutput(cmd)
> > +        if s != 0:
> > +            raise error.TestFail("Image converted failed; Command: %s;"
> > +                                 "Output is: %s" % (cmd, o))
> > +    #Subcommand 'qemu-img convert' test
> > +    def convert_test():
> > +        dest_image_format = params.get("dest_image_format")
> > +        output_filename = "%s.converted_%s" % (image_name, dest_image_format)
> > +
> > +        convert(dest_image_format, image_name, params.get("dest_image_format"),
> > +                image_format, params.get("compressed"), params.get("encrypted"))
> > +      
> > +        if dest_image_format == "qcow2":
> > +            s, o = check(output_filename)
> > +            if s:
> > +                commands.getoutput("rm -f %s" % output_filename)
> > +            else:
> > +                raise error.TestFail("Check image '%s' failed with error: %s" %
> > +                                                        (dest_image_format, o))
> > +        else:
> > +            commands.getoutput("rm -f %s" % output_filename)
> > +    #Subcommand 'qemu-img info' test
> > +    def info_test():
> > +        global cmd
> > +        cmd += " info"
> > +        cmd += " -f %s %s" % (image_format, image_name)
> > +        s, o = commands.getstatusoutput(cmd)
> > +        if s != 0:
> > +            raise error.TestFail("Get info of image '%s' failed: %s" %
> > +                                                      (image_name, o))
> > +        logging.info("Info of image '%s': \n%s" % (image_name, o))
> 
> In the above, we could parse info output and see if at least it says
> that the image is the type we expected it to be.
> 
> > +    #Subcommand 'qemu-img snapshot' test
> > +    def snapshot_test():
> > +        global cmd
> > +        cmd += " snapshot"
> > +        for i in range(2):
> > +            crtcmd = cmd
> > +            snapshot_name = "snapshot%d" % i
> > +            crtcmd += " -c %s %s" % (snapshot_name, image_name)
> > +            s, o = commands.getstatusoutput(crtcmd)
> > +            if s != 0:
> > +                raise error.TestFail("Create snapshot failed via command: %s;"
> > +                                     "Output is: %s" % (crtcmd, o))
> > +            logging.info("Created snapshot#%d" % i)
> > +        listcmd = cmd
> > +        listcmd += " -l %s" % image_name
> > +        s, o = commands.getstatusoutput(listcmd)
> > +        if not ("snapshot0" in o and "snapshot1" in o and s == 0):
> > +            raise error.TestFail("Snapshot created failed or missed;"
> > +                                 "snapshot list is: \n%s" % o)
> > +        for i in range(2):
> > +            snapshot_name = "snapshot%d" % i
> > +            delcmd = cmd
> > +            delcmd += " -d %s %s" % (snapshot_name, image_name)
> > +            s, o = commands.getstatusoutput(delcmd)
> > +            if s != 0:
> > +                raise error.TestFail("Delete snapshot '%s' failed: %s" %
> > +                                                     (snapshot_name, o))
> > +
> > +    #Subcommand 'qemu-img commit' test
> > +    def commit_test(cmd):
> > +        pass
> > +
> > +    # Here starts test
> > +    eval("%s_test" % subcommand)
> 
> In the above expression, we would also benefit from encapsulating all
> qemu-img tests on a class:
> 
>       tester = QemuImgTester()
>       tester.test(subcommand)
> 
> Or something similar.
> 
> That said, I was wondering if we could consolidate all qemu-img tests to
> a single execution, instead of splitting it to several variants. We
> could keep a failure record, execute all tests and fail the entire test
> if any of them failed. It's not like terribly important, but it seems
> more logical to group all qemu-img subcommands testing under a single
> test.
> 
> Thanks for your work, and please take a look at implementing my
> suggestions.
> 
> --
> 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