On Fri, 2011-02-25 at 14:17 +0800, Suqin Huang wrote: > convert_image and rebase_image are used frequently, add them to > kvm_vm.py and kvm_preprocessing.py
Nice patch. However we are moving from an API that relied a lot on function return values to a model more exception based. So I ask you to respin your patch taking into account my comments below: > cfg example: > > - boot: install setup > type = boot > kill_vm_on_error = yes > variants: > - base: > - convert: > images +=" convert" > convert_image = yes > image_name_convert = converted_image > image_format_convert = qcow2 > force_create_image_convert = yes > boot_drive_image1 = no > - back: > variants: > - snapshot: > create_snapshot = yes > snapshot_name = snapshot > snapshot_format = qcow2 > - rebase: > images = " snapshot" > rebase_image = yes > image_name_snapshot = snapshot > image_format_snapshot = qcow2 > - commit: > images = " snapshot" > commit_image = yes > image_name_snapshot = snapshot > image_format_snapshot = qcow2 > > Signed-off-by: Suqin Huang <[email protected]> > --- > client/tests/kvm/kvm_preprocessing.py | 38 +++++++++ > client/tests/kvm/kvm_vm.py | 139 > +++++++++++++++++++++++++++++++++ > 2 files changed, 177 insertions(+), 0 deletions(-) > > diff --git a/client/tests/kvm/kvm_preprocessing.py > b/client/tests/kvm/kvm_preprocessing.py > index 515e3a5..2defb09 100644 > --- a/client/tests/kvm/kvm_preprocessing.py > +++ b/client/tests/kvm/kvm_preprocessing.py > @@ -56,6 +56,44 @@ def preprocess_vm(test, params, env, name): > vm = kvm_vm.VM(name, params, test.bindir, env.get("address_cache")) > env.register_vm(name, vm) > > + # Convert vm > + convert_image = False > + > + if params.get("convert_image") == "yes": > + logging.debug("Converting image...") > + convert_image = True > + > + if convert_image and not kvm_vm.convert_image(params, test.bindir): > + raise error.TestFail("Could nt convert image") ^ Make convert_image to raise VMConvertImageError or something similar and the code above can be re-written as: + if params.get("convert_image") == "yes": + kvm_vm.convert_image(params, test.bindir) Same for snapshot and rebase below. > + # Create snapshot > + create_snapshot = False > + > + if params.get("create_snapshot") == "yes": > + logging.debug("Create snapshot...") > + create_snapshot = True > + if create_snapshot and not kvm_vm.create_image(params, test.bindir): > + raise error.TestFail("Could not create snapshot") > + > + # Rebase vm > + rebase_image = False > + > + if params.get("rebase_image") == "yes": > + logging.debug("Rebase snapshot to backing image...") > + rebase_image = True > + if rebase_image and not kvm_vm.rebase_image(params, test.bindir): > + raise error.TestFail("Could not rebase snapshot to backing image") > + > + # Commit vm > + commit_image = False > + > + if params.get("commit_image") == "yes": > + logging.debug("Commit snapshot to backing image...") > + commit_image = True > + if commit_image and not kvm_vm.commit_image(params, test.bindir): > + raise error.TestFail("Could not commmit snapshot to backing image") > + > + # Start vm > start_vm = False > > if params.get("restart_vm") == "yes": > diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py > index 41f7491..44bdb9d 100755 > --- a/client/tests/kvm/kvm_vm.py > +++ b/client/tests/kvm/kvm_vm.py > @@ -215,12 +215,32 @@ def create_image(params, root_dir): > "qemu-img")) > qemu_img_cmd += " create" > > + if params.get("encrypted") == "yes": > + qemu_img_cmd += " -e" > + > + if params.get("create_snapshot") == "yes": > + snapshot_name = params.get("snapshot_name") > + snapshot_format = params.get("snapshot_format", "qcow2") > + base_format = params.get("image_format") > + base_filename = get_image_filename(params, root_dir) > + params['image_name'] = snapshot_name > + params['image_format'] = snapshot_format > + qemu_img_cmd += " -b %s -F %s" % (base_filename, base_format) > + > format = params.get("image_format", "qcow2") > qemu_img_cmd += " -f %s" % format > > image_filename = get_image_filename(params, root_dir) > qemu_img_cmd += " %s" % image_filename > > + preallocated = params.get("preallocated") > + if preallocated is not None: > + qemu_img_cmd += " -o preallocation=%s" % preallocated > + > + cluster_size = params.get("cluster_size") > + if cluster_size is not None: > + qemu_img_cmd += " -o cluster_size=%s" % cluster_size > + > size = params.get("image_size", "10G") > qemu_img_cmd += " %s" % size > > @@ -229,6 +249,125 @@ def create_image(params, root_dir): > return image_filename > > > +def convert_image(params, root_dir): > + """ > + Convert image > + > + @param params: A dict > + @param root_dir: Base directory for relative filenames. > + > + @note: Params should contain: > + image_name -- the name of the image file to be converted > + image_format -- the format of the the image to be convertged > + convert_name -- the name of the image after convert > + convert_format -- the format of the image after convert > + """ > + qemu_img_cmd = kvm_utils.get_path(root_dir, params.get("qemu_img_binary", > + "qemu-img")) > + qemu_img_cmd += " convert" > + if params.get("compressed") == "yes": > + qemu_img_cmd += " -c" > + if params.get("encrypted") == "yes": > + qemu_img_cmd += " -e" > + > + image_format = params.get("image_format", "qcow2") > + qemu_img_cmd += " -f %s" % image_format > + > + params_convert = kvm_utils.get_sub_dict(params, > + params.get("images").split()[-1]) > + convert_format = params_convert.get("image_format", "qcow2") > + qemu_img_cmd += " -O %s" % convert_format > + > + base_filename = get_image_filename(params, root_dir) > + convert_filename = get_image_filename(params_convert, root_dir) > + > + qemu_img_cmd += " %s %s" % (base_filename, convert_filename) > + > + try: > + utils.system(qemu_img_cmd) > + except error.CmdError, e: > + logging.error("Could not convert image, convert failed:\n%s", str(e)) > + return None ^ Like I commented above, this can throw a new exception type. Please check the other VM exceptions to have a idea of how to model the new exceptions. Same goes for the other functions below. > + logging.info("Convert %s to format %s" % (base_filename, convert_format)) > + return convert_filename > + > + > +def rebase_image(params, root_dir): > + """ > + Rebase image: > + @param params: A dict > + @param root_dir: Base directory for relative filename > + > + @note: params should contain: > + snapshot_name -- image name to be rebased > + image_name -- the base image name > + image_format -- the format of base image > + """ > + qemu_img_cmd = kvm_utils.get_path(root_dir, params.get("qemu_img_binary", > + "qemu-img")) > + qemu_img_cmd += " rebase" > + > + mode = params.get("rebase_mode", "unsafe") > + if mode == "unsafe": > + qemu_img_cmd += " -u" > + > + base_name = params.get("image_name") > + base_format = params.get("image_format", "qcow2") > + base_filename = get_image_filename(params, root_dir) > + qemu_img_cmd += " -b %s -F %s" % (base_filename, base_format) > + > + params_sn = kvm_utils.get_sub_dict(params, > params.get("images").split()[-1]) > + snapshot_name = params_sn.get("image_name") > + snapshot_format = params_sn.get("image_format", "qcow2") > + snapshot_filename = get_image_filename(params_sn, root_dir) > + > + qemu_img_cmd += " -f %s %s" % (snapshot_format, snapshot_filename) > + > + try: > + utils.system(qemu_img_cmd) > + except error.CmdError, e: > + logging.error("Rebase image failed\n%s", str(e)) > + return None > + > + logging.info("Image is rebase to %s" % base_name) > + > + return base_filename > + > + > +def commit_image(params, root_dir): > + """ > + Commit snapshot to base file > + > + @param params: A dict > + @param root_dir: Base directory for relative filenames. > + > + @note: params should contain: > + image_name -- the name of the snapshot > + image_format -- the format of the snapshot > + """ > + qemu_img_cmd = kvm_utils.get_path(root_dir, params.get("qemu_img_binary", > + "qemu-img")) > + qemu_img_cmd += " commit" > + > + params_sn = kvm_utils.get_sub_dict(params, > params.get("images").split()[-1]) > + snapshot_format = params_sn.get("image_format", "qcow2") > + snapshot_filename = get_image_filename(params_sn, root_dir) > + qemu_img_cmd += " -f %s %s" % (snapshot_format, snapshot_filename) > + > + base_filename = get_image_filename(params, root_dir) > + > + try: > + utils.system(qemu_img_cmd) > + except error.CmdError, e: > + logging.error("Commit image failed\n%s", str(e)) > + return None > + > + logging.info("Image commit to backing file") > + > + return base_filename > + > + > def remove_image(params, root_dir): > """ > Remove an image file. > > _______________________________________________ > Autotest mailing list > [email protected] > http://test.kernel.org/cgi-bin/mailman/listinfo/autotest _______________________________________________ Autotest mailing list [email protected] http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
