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