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 <yz...@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. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html