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

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

[Yolkfull Chow]

This is designed to test all subcommands of 'qemu-img' however
so far 'commit' is not implemented.

* 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")
+    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, "")
+        
+    # 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
+        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))
+
+    #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)
diff --git a/client/tests/kvm/tests_base.cfg.sample b/client/tests/kvm/tests_base.cfg.sample
index 2d03f1f..cedd919 100644
--- a/client/tests/kvm/tests_base.cfg.sample
+++ b/client/tests/kvm/tests_base.cfg.sample
@@ -273,6 +273,42 @@ variants:
         type = physical_resources_check
         catch_uuid_cmd = dmidecode | awk -F: '/UUID/ {print $2}'
 
+    - qemu_img:
+        type = qemu_img
+        vms = ''
+        variants:
+            - check:
+                subcommand = check
+                dd_image_size = 1G
+                dd_image_name = /tmp/test_image
+                force_create_image_dd = no
+                create_image_cmd = "dd if=/dev/zero of=%s bs=%s count=1"
+                # Test the convertion from 'dd_image_name' to specified format
+                supported_image_formats = qcow2 raw
+            - create:
+                subcommand = create
+                post_command = ""
+                images += " stg"
+                force_create_image_test = yes
+                image_size_test = 5000G
+                image_name_test = qemu_img_test
+                remove_image_test = yes
+            - convert:
+                subcommand = convert
+                variants:
+                    - to_qcow2:
+                        dest_image_format = qcow2
+                        compressed = no
+                        encrypted = no
+                    - to_raw:
+                        dest_image_format = raw
+            - snapshot:
+                subcommand = snapshot
+            - commit:
+                subcommand = commit
+            - info:
+                subcommand = info
+
 # NICs
 variants:
     - @rtl8139:
-- 
1.6.6

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

[Lucas Meneghel Rodrigues]

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
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.

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

[Yolkfull Chow]

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

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

[Lucas Meneghel Rodrigues]

On Wed, 2010-01-27 at 17:41 +0800, Yolkfull Chow wrote:
> 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' ?

I mean, client/tests/kvm. Sorry I wasn't clear enough.

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

[Yolkfull Chow]

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
> 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.

Hi Lucas,

After considering above suggestion about merging all qemu-img tests into single test,
I did decision that keep current method due to reason that:
 1) it's convenient to maintain so many parameters of each test variant.
 2) current method does not affect global results since even if one subtest of 'qemu-img' failes,
    it will nor block following subtests (we can still get whole test results)
 3) it's possible for user to run single subtest of qemu-img, say only test 'rebase' subcommand

As you also said this is not terribly important, let's keep it going. What's your opinion?

> 
> Thanks for your work, and please take a look at implementing my
> suggestions.

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

[Lucas Meneghel Rodrigues]

On Thu, 2010-01-28 at 17:37 +0800, Yolkfull Chow wrote:
> On Tue, Jan 26, 2010 at 03:11:34PM -0200, Lucas Meneghel Rodrigues wrote:
> > 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.
> 
> Hi Lucas,
> 
> After considering above suggestion about merging all qemu-img tests into single test,
> I did decision that keep current method due to reason that:
>  1) it's convenient to maintain so many parameters of each test variant.
>  2) current method does not affect global results since even if one subtest of 'qemu-img' failes,
>     it will nor block following subtests (we can still get whole test results)
>  3) it's possible for user to run single subtest of qemu-img, say only test 'rebase' subcommand
> 
> As you also said this is not terribly important, let's keep it going. What's your opinion?

Ok, I am fine with it!

Lucas

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

[Yolkfull Chow]

On Wed, Mar 17, 2010 at 10:38:58AM -0300, Lucas Meneghel Rodrigues wrote:
> Copying Michael on the message.
> 
> Hi Yolkfull, I have reviewed this patch and I have some comments to
> make on it, similar to the ones I made on an earlier version of it:
> 
> One of the things that I noticed is that this patch doesn't work very
> well out of the box:
> 
> [lmr@freedom kvm]$ ./scan_results.py
> Test                                            		Status	Seconds	Info
> ----                                            		------	-------	----
>         (Result file: ../../results/default/status)
> smp2.Fedora.11.64.qemu_img.check                		GOOD	47	completed successfully
> smp2.Fedora.11.64.qemu_img.create               		GOOD	44	completed successfully
> smp2.Fedora.11.64.qemu_img.convert.to_qcow2     		FAIL	45	Image
> converted failed; Command: /usr/bin/qemu-img convert -f qcow2 -O qcow2
> /tmp/kvm_autotest_root/images/fc11-64.qcow2
> /tmp/kvm_autotest_root/images/fc11-64.qcow2.converted_qcow2;Output is:
> qemu-img: Could not open '/tmp/kvm_autotest_root/images/fc11-64.qcow2'
> smp2.Fedora.11.64.qemu_img.convert.to_raw       		FAIL	46	Image
> converted failed; Command: /usr/bin/qemu-img convert -f qcow2 -O raw
> /tmp/kvm_autotest_root/images/fc11-64.qcow2
> /tmp/kvm_autotest_root/images/fc11-64.qcow2.converted_raw;Output is:
> qemu-img: Could not open '/tmp/kvm_autotest_root/images/fc11-64.qcow2'
> smp2.Fedora.11.64.qemu_img.snapshot             		FAIL	44	Create
> snapshot failed via command: /usr/bin/qemu-img snapshot -c snapshot0
> /tmp/kvm_autotest_root/images/fc11-64.qcow2;Output is: qemu-img: Could
> not open '/tmp/kvm_autotest_root/images/fc11-64.qcow2'
> smp2.Fedora.11.64.qemu_img.commit               		GOOD	44	completed successfully
> smp2.Fedora.11.64.qemu_img.info                 		FAIL	44	Unhandled
> str: Unhandled TypeError: argument of type 'NoneType' is not iterable
> smp2.Fedora.11.64.qemu_img.rebase               		TEST_NA	43	Current
> kvm user space version does not support 'rebase' subcommand
> ----                                            		GOOD	412	
> 
> We need to fix that before upstream inclusion.

Hi Lucas, did you run the test on fedora or other box? I ran this test on my fedora 13 box for
several times, worked fine:

# ./scan_results.py 
Test                                            		Status	Seconds	Info
----                                            		------	-------	----
        (Result file: ../../results/default/status)
smp2.RHEL.5.4.i386.qemu_img.check               		GOOD	132	completed successfully
smp2.RHEL.5.4.i386.qemu_img.create              		GOOD	144	completed successfully
smp2.RHEL.5.4.i386.qemu_img.convert.to_qcow2    		GOOD	251	completed successfully
smp2.RHEL.5.4.i386.qemu_img.convert.to_raw      		GOOD	245	completed successfully
smp2.RHEL.5.4.i386.qemu_img.snapshot            		GOOD	140	completed successfully
smp2.RHEL.5.4.i386.qemu_img.commit              		GOOD	146	completed successfully
smp2.RHEL.5.4.i386.qemu_img.info                		GOOD	133	completed successfully
smp2.RHEL.5.4.i386.qemu_img.rebase              		TEST_NA	137	Current kvm user space version does not support 'rebase' subcommand
----                                            		GOOD	1392	
[root@aFu kvm]# 

Weird why there are some case failed...
Please test again based on the new patch I will send later.

> 
> Also, one thing that I've noticed is that this test doesn't depend of
> any other variants, so we don't need to repeat it to every combination
> of guest and qemu command line options. Michael, does it occur to you
> a way to get this test out of the variants block, so it gets executed
> only once per job and not every combination of guest and other qemu
> options?

Lucas and Michael, maybe we could add a parameter say 'ignore_vm_config = yes' to config file
which let a test ignore all configurations combination. 
Another method is ugly adding following block into config file:

---
qemu_img:
    only ide
    only qcow2
    only up
    only ...
    (use 'only' to filter all configurations combination)
---

But I don't think it's a good idea. What do you think?

> 
> On Fri, Jan 29, 2010 at 4:00 AM, Yolkfull Chow <yzhou@redhat.com> wrote:
> > This is designed to test all subcommands of 'qemu-img' however
> > so far 'commit' is not implemented.
> >
> > * 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 will check image format & size according to
> > output of 'info' subcommand  at specified image file.
> >
> > * For 'rebase' subcommand test, it will create first snapshot 'sn1' based on original
> > base_img, and create second snapshot based on sn1. And then rebase sn2 to base_img.
> > After rebase check the baking_file of sn2.
> >
> > This supports two rebase mode: unsafe mode and safe mode:
> > Unsafe mode:
> > With -u an unsafe mode is enabled that doesn't require the backing files to exist.
> > It merely changes the backing file reference in the COW image. This is useful for
> > renaming or moving the backing file. The user is responsible to make sure that the
> > new backing file has no changes compared to the old one, or corruption may occur.
> >
> > Safe Mode:
> > Both the current and the new backing file need to exist, and after the rebase, the
> > COW image is guaranteed to have the same guest visible content as before.
> > To achieve this, old and new backing file are compared and, if necessary, data is
> > copied from the old backing file into the COW image.
> >
> > Signed-off-by: Yolkfull Chow <yzhou@redhat.com>
> > ---
> >  client/tests/kvm/tests/qemu_img.py     |  235 ++++++++++++++++++++++++++++++++
> >  client/tests/kvm/tests_base.cfg.sample |   40 ++++++
> >  2 files changed, 275 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..e6352a0
> > --- /dev/null
> > +++ b/client/tests/kvm/tests/qemu_img.py
> > @@ -0,0 +1,235 @@
> > +import re, os, logging, commands
> > +from autotest_lib.client.common_lib import utils, error
> > +import kvm_vm, kvm_utils
> > +
> > +
> > +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 = kvm_utils.get_path(test.bindir, params.get("qemu_img_binary"))
> > +    if not os.path.exists(cmd):
> > +        raise error.TestError("Binary of 'qemu-img' not found")
> > +    image_format = params.get("image_format")
> > +    image_size = params.get("image_size", "10G")
> > +    image_name = kvm_vm.get_image_filename(params, test.bindir)
> > +
> > +    def check(cmd, img):
> > +        cmd += " check %s" % img
> > +        logging.info("Checking image '%s'..." % img)
> > +        o = commands.getoutput(cmd)
> > +        if "does not support checks" in o or "No errors" in o:
> > +            return (True, "")
> > +        return (False, o)
> 
> Here you're using the commands module. We already have an API on
> autotest, utils,system() that will return the exit code, with command
> logging and will raise an exception if the test fails. This would
> allow for shorter code and brevity. I see you have used system_output
> down on the patch, so why not make it all consistent? In one or two
> cases you have to do exception handling, but in the majority of the
> executions, a simple utils.system(command) would work.

Here I wanted to catch the output and ignore the exit status. 
However 'utils.system_output(cmd, ignore_status=True)' doesn't work well here.
Any suggestion?

> 
> Also, it would be a good idea to make all the auxiliary functions (ie,
> the ones being used by the actual test functions) as private functions
> (with an underscore _ at the beginning of its name. Another thing that
> occurred to me is that all functions but the top level one lack
> docstrings.

Will modify them.

> 
> > +
> > +    # 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(cmd):
> > +        test_image = params.get("image_name_dd")
> > +        create_image_cmd = params.get("create_image_cmd")
> > +        create_image_cmd = create_image_cmd % test_image
> > +        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(cmd, test_image)
> 
> In the above code you are trying to qemu-img check a raw image, which
> doesn't support checks. This will fail every single time unless I am
> very mistaken.

It doesn't matter because the error 'does not support checks' will be caught in '_check()' 
during checking raw format.

> 
> > +        if not s:
> > +            raise error.TestFail("Check image '%s' failed with error: %s" %
> > +                                                           (test_image, o))
> > +        for fmt in params.get("supported_image_formats").split():
> > +            output_image = test_image + ".%s" % fmt
> > +            convert(cmd, fmt, test_image, output_image)
> > +            s, o = check(cmd, output_image)
> > +            if not s:
> > +                raise error.TestFail("Check image '%s' got error: %s" %
> > +                                                     (output_image, o))
> > +            os.remove(output_image)
> > +        os.remove(test_image)
> > +
> > +    def create(cmd, img_name, fmt, img_size=None, base_img=None,
> > +               base_img_fmt=None, encrypted="no"):
> > +        cmd += " create"
> > +        if encrypted == "yes":
> > +            cmd += " -e"
> > +        if base_img:
> > +            cmd += " -b %s" % base_img
> > +            if base_img_fmt:
> > +                cmd += " -F %s" % base_img_fmt
> > +        cmd += " -f %s" % fmt
> > +        cmd += " %s" % img_name
> > +        if img_size:
> > +            cmd += " %s" % img_size
> > +        s, o = commands.getstatusoutput(cmd)
> > +        if s != 0:
> > +            raise error.TestFail("Create image '%s' failed: %s;Command:\n%s" %
> > +                                                           (img_name, o, cmd))
> > +        logging.info("Created image '%s'" % img_name)
> > +
> > +    # Subcommand 'qemu-img create' test
> > +    def create_test(cmd):
> > +        image_large = params.get("image_name_large")
> > +        img = kvm_utils.get_path(test.bindir, image_large)
> > +        img += '.' + image_format
> > +        create(cmd, img_name=img, fmt=image_format,
> > +               img_size=params.get("image_size_large"))
> > +        os.remove(img)
> > +
> > +    def convert(cmd, output_fmt, img_name, output_filename,
> > +                fmt=None, compressed="no", encrypted="no"):
> > +        cmd += " convert"
> > +        if compressed == "yes":
> > +            cmd += " -c"
> > +        if encrypted == "yes":
> > +            cmd += " -e"
> > +        if fmt:
> > +            cmd += " -f %s" % fmt
> > +        cmd += " -O %s" % output_fmt
> > +        cmd += " %s %s" % (img_name, output_filename)
> > +        logging.info("Converting '%s' from format '%s' to '%s'" %
> > +                                     (img_name, fmt, output_fmt))
> > +        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(cmd):
> > +        dest_img_fmt = params.get("dest_image_format")
> > +        output_filename = "%s.converted_%s" % (image_name, dest_img_fmt)
> > +
> > +        convert(cmd, dest_img_fmt, image_name, output_filename,
> > +                image_format, params.get("compressed"), params.get("encrypted"))
> > +
> > +        if dest_img_fmt == "qcow2":
> > +            s, o = check(cmd, output_filename)
> > +            if s:
> > +                os.remove(output_filename)
> > +            else:
> > +                raise error.TestFail("Check image '%s' failed with error: %s" %
> > +                                                        (output_filename, o))
> > +        else:
> > +            os.remove(output_filename)
> > +
> > +    def info(cmd, img, string=None, fmt=None):
> > +        cmd += " info"
> > +        if fmt:
> > +            cmd += " -f %s" % fmt
> > +        cmd += " %s" % img
> > +        s, o = commands.getstatusoutput(cmd)
> > +        if s != 0:
> > +            logging.error("Get info of image '%s' failed: %s" % (img, o))
> > +            return None
> > +        if not string:
> > +            return o
> > +        string += ": (.*)"
> > +        str = re.findall(string, o)
> > +        if str:
> > +            return str[0]
> > +        return None
> > +
> > +    # Subcommand 'qemu-img info' test
> > +    def info_test(cmd):
> > +        img_info = info(cmd, image_name)
> > +        logging.info("Info of image '%s': \n%s" % (image_name, img_info))
> > +        if not image_format in img_info:
> > +            raise error.TestFail("Got unexpected format of image '%s'"
> > +                                 " in info test" % image_name)
> > +        if not image_size in img_info:
> > +            raise error.TestFail("Got unexpected size of image '%s'"
> > +                                 " in info test" % image_name)
> > +
> > +    # Subcommand 'qemu-img snapshot' test
> > +    def snapshot_test(cmd):
> > +        cmd += " snapshot"
> > +        for i in range(2):
> > +            crtcmd = cmd
> > +            sn_name = "snapshot%d" % i
> > +            crtcmd += " -c %s %s" % (sn_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 '%s' in '%s'" % (sn_name,image_name))
> > +        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):
> > +            sn_name = "snapshot%d" % i
> > +            delcmd = cmd
> > +            delcmd += " -d %s %s" % (sn_name, image_name)
> > +            s, o = commands.getstatusoutput(delcmd)
> > +            if s != 0:
> > +                raise error.TestFail("Delete snapshot '%s' failed: %s" %
> > +                                                     (sn_name, o))
> > +
> > +    #Subcommand 'qemu-img commit' test
> > +    def commit_test(cmd):
> > +        pass
> > +
> > +    def rebase(cmd, img_name, base_img, backing_fmt, mode="unsafe"):
> > +        cmd += " rebase"
> > +        if mode == "unsafe":
> > +            cmd += " -u"
> > +        cmd += " -b %s -F %s %s" % (base_img, backing_fmt, img_name)
> > +        logging.info("Trying to rebase '%s' to '%s'..." % (img_name, base_img))
> > +        s, o = commands.getstatusoutput(cmd)
> > +        if s != 0:
> > +            raise error.TestError("Failed to rebase '%s' to '%s': %s" %
> > +                                               (img_name, base_img, o))
> > +
> > +    # Subcommand 'qemu-img rebase' test
> > +    # Change the backing file of a snapshot image in "unsafe mode":
> > +    # Assume the previous backing file had missed and we just have to change
> > +    # reference of snapshot to new one. After change the backing file of a
> > +    # snapshot image in unsafe mode, the snapshot should work still.
> > +    def rebase_test(cmd):
> > +        if not 'rebase' in utils.system_output(cmd+' --help',ignore_status=True):
> > +            raise error.TestNAError("Current kvm user space version does not"
> > +                                    " support 'rebase' subcommand")
> > +        sn_fmt = params.get("snapshot_format", "qcow2")
> > +        sn1 = params.get("image_name_snapshot1")
> > +        sn1 = kvm_utils.get_path(test.bindir, sn1) + ".%s" % sn_fmt
> > +        base_img = kvm_vm.get_image_filename(params, test.bindir)
> > +        create(cmd, sn1, sn_fmt, base_img=base_img, base_img_fmt=image_format)
> > +
> > +        # Create snapshot2 based on snapshot1
> > +        sn2 = params.get("image_name_snapshot2")
> > +        sn2 = kvm_utils.get_path(test.bindir, sn2) + ".%s" % sn_fmt
> > +        create(cmd, sn2, sn_fmt, base_img=sn1, base_img_fmt=sn_fmt)
> > +
> > +        rebase_mode = params.get("rebase_mode")
> > +        if rebase_mode == "unsafe":
> > +            os.remove(sn1)
> > +
> > +        rebase(cmd, sn2, base_img, image_format, mode=rebase_mode)
> > +
> > +        # Check sn2's format and backing_file
> > +        actual_base_img = info(cmd, sn2, "backing file")
> > +        base_img_name = os.path.basename(params.get("image_name"))
> > +        if not base_img_name in actual_base_img:
> > +            raise error.TestFail("After rebase the backing_file of 'sn2' is "
> > +                                 "'%s' which is not expected as '%s'"
> > +                                 % (actual_base_img, base_img_name))
> > +        s, o = check(cmd, sn2)
> > +        if not s:
> > +            raise error.TestFail("Check image '%s' failed after rebase;"
> > +                                 "got error: %s" % (sn2, o))
> > +        try:
> > +            os.remove(sn2)
> > +            os.remove(sn1)
> > +        except:
> > +            pass
> > +
> > +    # Here starts test
> > +    subcommand = params.get("subcommand")
> > +    eval("%s_test(cmd)" % subcommand)
> > diff --git a/client/tests/kvm/tests_base.cfg.sample b/client/tests/kvm/tests_base.cfg.sample
> > index 2d03f1f..e7670c7 100644
> > --- a/client/tests/kvm/tests_base.cfg.sample
> > +++ b/client/tests/kvm/tests_base.cfg.sample
> > @@ -273,6 +273,46 @@ variants:
> >         type = physical_resources_check
> >         catch_uuid_cmd = dmidecode | awk -F: '/UUID/ {print $2}'
> >
> > +    - qemu_img:
> > +        type = qemu_img
> > +        vms = ''
> > +        variants:
> > +            - check:
> > +                subcommand = check
> > +                image_name_dd = dd_created_image
> > +                force_create_image_dd = no
> > +                remove_image_dd = yes
> > +                create_image_cmd = "dd if=/dev/zero of=%s bs=1G count=1"
> > +                # Test the convertion from 'dd_image_name' to specified format
> > +                supported_image_formats = qcow2 raw
> > +            - create:
> > +                subcommand = create
> > +                images += " large"
> > +                force_create_image_large = yes
> > +                image_size_large = 1G
> > +                image_name_large = create_large_image
> > +                remove_image_large = yes
> > +            - convert:
> > +                subcommand = convert
> > +                variants:
> > +                    - to_qcow2:
> > +                        dest_image_format = qcow2
> > +                        compressed = no
> > +                        encrypted = no
> > +                    - to_raw:
> > +                        dest_image_format = raw
> > +            - snapshot:
> > +                subcommand = snapshot
> > +            - commit:
> > +                subcommand = commit
> > +            - info:
> > +                subcommand = info
> > +            - rebase:
> > +                subcommand = rebase
> > +                rebase_mode = unsafe
> > +                image_name_snapshot1 = sn1
> > +                image_name_snapshot2 = sn2
> > +
> >  # NICs
> >  variants:
> >     - @rtl8139:
> > --
> > 1.5.5.6
> >
> > _______________________________________________
> > Autotest mailing list
> > Autotest@test.kernel.org
> > http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
> >
> 
> 
> 
> -- 
> Lucas
> --
> 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

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

[Yolkfull Chow]

On Thu, Jan 28, 2010 at 09:01:43AM +0530, sudhir kumar wrote:
> Yolkfull,
> Good test. Did never come to my mind to add such a test to autotest.
> I would like to test your latest patch!!

Thanks Sudhir, please then help review that patch as well.:)

> 
> On Thu, Jan 28, 2010 at 8:37 AM, Yolkfull Chow <yzhou@redhat.com> wrote:
> > On Wed, Jan 27, 2010 at 07:37:46AM -0500, Michael Goldish wrote:
> >>
> >> ----- "Yolkfull Chow" <yzhou@redhat.com> wrote:
> >>
> >> > 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).
> >>
> >> This can be done quickly using kvm_utils.get_path() and os.path.exists(),
> >> like this:
> >>
> >> cmd = kvm_utils.get_path(params.get("qemu_img_binary"))
> >> if not os.path.exists(cmd):
> >>     raise error.TestError("qemu-img binary not found")
> >>
> >> kvm_utils.get_path() is the standard way of getting both absolute and
> >> relative paths, and os.path.exists() checks whether the file exists and
> >> makes sure it's not a broken symlink.
> >
> > Yes, thanks for pointing that out.
> >
> >>
> >> > > > +    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'.
> >>
> >> Do we really need a class for this?  If we want to avoid using a global
> >> variable, 'cmd' can be passed as a parameter to all the inner functions
> >> that use it (check(), convert(), create_test(), etc).
> >
> > Actually before using 'global' to declare it, I was passing 'cmd' as a parameter
> > to all inner functions of subcommand. Anyway, I could change it back. :)
> >
> >>
> >> > > > +        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)
> >>
> >> Aren't you missing a () -- eval("%s_test()" % subcommand)?
> >>
> >> BTW, Yolkfull, have you tried running the test and verifying that it
> >> works?
> >
> > Oh, really missed it. I tested it when using method of passing 'cmd' as a parameter
> > to subcommand functions and it worked fine. So introduced this mistake when changing
> > to use 'global'. Will fix it in next version which will also add 'rebase' subcommand test.
> >
> > Thanks for comments, Michael.
> >
> >>
> >> > >
> >> > > 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
> >> > --
> >> > 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
> > _______________________________________________
> > Autotest mailing list
> > Autotest@test.kernel.org
> > http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
> >
> 
> 
> 
> -- 
> Sudhir Kumar

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

[sudhir kumar]

Yolkfull,
Good test. Did never come to my mind to add such a test to autotest.
I would like to test your latest patch!!

On Thu, Jan 28, 2010 at 8:37 AM, Yolkfull Chow <yzhou@redhat.com> wrote:
> On Wed, Jan 27, 2010 at 07:37:46AM -0500, Michael Goldish wrote:
>>
>> ----- "Yolkfull Chow" <yzhou@redhat.com> wrote:
>>
>> > 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).
>>
>> This can be done quickly using kvm_utils.get_path() and os.path.exists(),
>> like this:
>>
>> cmd = kvm_utils.get_path(params.get("qemu_img_binary"))
>> if not os.path.exists(cmd):
>>     raise error.TestError("qemu-img binary not found")
>>
>> kvm_utils.get_path() is the standard way of getting both absolute and
>> relative paths, and os.path.exists() checks whether the file exists and
>> makes sure it's not a broken symlink.
>
> Yes, thanks for pointing that out.
>
>>
>> > > > +    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'.
>>
>> Do we really need a class for this?  If we want to avoid using a global
>> variable, 'cmd' can be passed as a parameter to all the inner functions
>> that use it (check(), convert(), create_test(), etc).
>
> Actually before using 'global' to declare it, I was passing 'cmd' as a parameter
> to all inner functions of subcommand. Anyway, I could change it back. :)
>
>>
>> > > > +        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)
>>
>> Aren't you missing a () -- eval("%s_test()" % subcommand)?
>>
>> BTW, Yolkfull, have you tried running the test and verifying that it
>> works?
>
> Oh, really missed it. I tested it when using method of passing 'cmd' as a parameter
> to subcommand functions and it worked fine. So introduced this mistake when changing
> to use 'global'. Will fix it in next version which will also add 'rebase' subcommand test.
>
> Thanks for comments, Michael.
>
>>
>> > >
>> > > 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
>> > --
>> > 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
> _______________________________________________
> Autotest mailing list
> Autotest@test.kernel.org
> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
>



-- 
Sudhir Kumar