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

[Yolkfull Chow <yzhou@redhat.com>]

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