[Kimchi-devel] [PATCHv2 5/5] Create volume based on backing store image

[Aline Manera]

Just a minor comment below

On 07/15/2014 06:11 AM, lvroyce at linux.vnet.ibm.com wrote:
> From: Royce Lv <lvroyce at linux.vnet.ibm.com>
>
> Creating volume base on backing store so that we can create
> vm from this cow volume. Also change volume xml generation method
> to lxml.
>
> Signed-off-by: Royce Lv <lvroyce at linux.vnet.ibm.com>
> ---
>   src/kimchi/i18n.py       |  1 +
>   src/kimchi/model/vms.py  |  1 +
>   src/kimchi/utils.py      | 16 ++++++++++++++++
>   src/kimchi/vmtemplate.py | 33 ++++++++++++++++++++-------------
>   4 files changed, 38 insertions(+), 13 deletions(-)
>
> diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
> index 3ba1e23..b427c5d 100644
> --- a/src/kimchi/i18n.py
> +++ b/src/kimchi/i18n.py
> @@ -126,6 +126,7 @@ messages = {
>       "KCHTMPL0021E": _("Unable to delete template due error: %(err)s"),
>       "KCHTMPL0022E": _("Disk size must be greater than 1GB."),
>       "KCHTMPL0023E": _("Template base image must be a valid local image file"),
> +    "KCHTMPL0024E": _("Cannot identify base image %(path)s format"),
>
>       "KCHPOOL0001E": _("Storage pool %(name)s already exists"),
>       "KCHPOOL0002E": _("Storage pool %(name)s does not exist"),
> diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py
> index 17bda04..55699d0 100644
> --- a/src/kimchi/model/vms.py
> +++ b/src/kimchi/model/vms.py
> @@ -201,6 +201,7 @@ class VMsModel(object):
>           # the user from UI or manually.
>           vol_list = []
>           if t._get_storage_type() in ["iscsi", "scsi"]:
> +        # FIXME: iscsi and scsi storage work with base image needs to be fixed.
>               vol_list = []
>           else:
>               vol_list = t.fork_vm_storage(vm_uuid)
> diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py
> index 97adbf8..0fd59ff 100644
> --- a/src/kimchi/utils.py
> +++ b/src/kimchi/utils.py
> @@ -20,6 +20,7 @@
>
>   import cherrypy
>   import grp
> +import json
>   import os
>   import psutil
>   import pwd
> @@ -213,6 +214,21 @@ def parse_cmd_output(output, output_items):
>       return res
>

> +def probe_img_info(path):
> +    cmd = ["qemu-img", "info", "--output=json", path]
> +    info = dict()
> +    try:
> +        out = run_command(cmd, 10)[0]
> +    except TimeoutExpired:
> +        kimchi_log.warning("Cannot decide format of base img %s", path)
> +        return None
> +
> +    info = json.loads(out)
> +    info['virtual-size'] = info['virtual-size'] >> 30
> +    info['actual-size'] = info['actual-size'] >> 30
> +    return info
> +

Maybe imageinfo.py is a better location for this code.

> +
>   def patch_find_nfs_target(nfs_server):
>       cmd = ["showmount", "--no-headers", "--exports", nfs_server]
>       try:
> diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py
> index d531406..883455c 100644
> --- a/src/kimchi/vmtemplate.py
> +++ b/src/kimchi/vmtemplate.py
> @@ -27,7 +27,7 @@ from kimchi import osinfo
>   from kimchi.exception import InvalidParameter, IsoFormatError, ImageFormatError
>   from kimchi.imageinfo import probe_image
>   from kimchi.isoinfo import IsoImage
> -from kimchi.utils import check_url_path, pool_name_from_uri
> +from kimchi.utils import check_url_path, pool_name_from_uri, probe_img_info
>   from lxml import etree
>   from lxml.builder import E
>
> @@ -65,6 +65,8 @@ class VMTemplate(object):
>                       except ImageFormatError:
>                           pass
>                       args['cdrom'] = ''
> +                    if 'size' not in d:
> +                        d['size'] = probe_img_info(d['base'])['virtual-size']
>                       break
>
>           if scan and len(iso) > 0:
> @@ -254,6 +256,13 @@ drive=drive-%(bus)s0-1-0,id=%(bus)s0-1-0'/>
>           fmt = 'raw' if self._get_storage_type() in ['logical'] else 'qcow2'
>           ret = []
>           for i, d in enumerate(self.info['disks']):
> +            if 'base' in d:
> +                base_fmt = probe_img_info(d['base'])['format']
> +                if base_fmt is None:
> +                    raise InvalidParameter("KCHTMPL0024E", {'path': d['base']})
> +                base_path = d['base']
> +            else:
> +                base_path = None
>               index = d.get('index', i)
>               volume = "%s-%s.img" % (vm_uuid, index)
>
> @@ -262,19 +271,17 @@ drive=drive-%(bus)s0-1-0,id=%(bus)s0-1-0'/>
>                       'type': 'disk',
>                       'format': fmt,
>                       'path': '%s/%s' % (storage_path, volume)}
> -
>               info['allocation'] = 0 if fmt == 'qcow2' else info['capacity']
> -            info['xml'] = """
> -            <volume>
> -              <name>%(name)s</name>
> -              <allocation unit="G">%(allocation)s</allocation>
> -              <capacity unit="G">%(capacity)s</capacity>
> -              <target>
> -                <format type='%(format)s'/>
> -                <path>%(path)s</path>
> -              </target>
> -            </volume>
> -            """ % info
> +            v_tree = E.volume(E.name(info['name']))
> +            v_tree.append(E.allocation(str(info['allocation']), unit='G'))
> +            v_tree.append(E.capacity(str(info['capacity']), unit='G'))
> +            target = E.target(
> +                E.format(type=info['format']), E.path(info['path']))
> +            if base_path:
> +                v_tree.append(E.backingStore(
> +                    E.path(base_path), E.format(type=base_fmt)))
> +            v_tree.append(target)
> +            info['xml'] = etree.tostring(v_tree)
>               ret.append(info)
>           return ret
>