[Kimchi-devel] [PATCH][Kimchi 3/4] Identify installation media while creating template

Rodrigo Trujillo rodrigo.trujillo at linux.vnet.ibm.com
Wed Mar 30 15:43:23 UTC 2016


Comments below

On 03/29/2016 04:27 PM, Ramon Medeiros wrote:
> First, improve functions inside class. This will improve code reading,
> also, make more functions to split procedures like: validating disks,
> graphics xml and distro checking.
>
> Add separate function to identify the installation media.
>
> Signed-off-by: Ramon Medeiros <ramonn at linux.vnet.ibm.com>
> ---
>   model/templates.py |   4 ++
>   vmtemplate.py      | 111 +++++++++++++++++++++++++++++++++++++++++++++--------
>   2 files changed, 99 insertions(+), 16 deletions(-)
>
> diff --git a/model/templates.py b/model/templates.py
> index 28c8a08..30d403e 100644
> --- a/model/templates.py
> +++ b/model/templates.py
> @@ -198,6 +198,10 @@ class TemplateModel(object):
>               params['memory'].update(new_mem)
>               validate_memory(params['memory'])
>
> +        # params are cdrom or disk: add it to source_media
> +        if "cdrom" in params:
> +            params["source_media"] = params.get("cdrom")
> +
So, here , you are keeping "cdrom", because you os guest/template update.
If you are changing the API to create a new resource, you also must 
change API  that edits it,
for consistence.

>           new_t.update(params)
>
>           for net_name in params.get(u'networks', []):
> diff --git a/vmtemplate.py b/vmtemplate.py
> index 653ad02..c0effc6 100644
> --- a/vmtemplate.py
> +++ b/vmtemplate.py
> @@ -17,6 +17,7 @@
>   # License along with this library; if not, write to the Free Software
>   # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
>
> +import magic
>   import os
>   import platform
>   import stat
> @@ -40,6 +41,11 @@ from wok.plugins.kimchi.xmlutils.interface import get_iface_xml
>   from wok.plugins.kimchi.xmlutils.qemucmdline import get_qemucmdline_xml
>   from wok.plugins.kimchi.xmlutils.serial import get_serial_xml
>
> +DISK_TYPE = {"QEMU QCOW Image": "qcow2",
> +             "data": "raw"}
> +ISO_TYPE = "ISO 9660 CD-ROM"
> +SOURCE_MEDIA = "source_media"
> +
I am still against this global variable
>   class VMTemplate(object):
>       def __init__(self, args, scan=False):
> @@ -54,20 +60,13 @@ class VMTemplate(object):
>           self.info = {}
>           self.fc_host_support = args.get('fc_host_support')
>
> -        # Fetch defaults based on the os distro and version
> -        try:
> -            distro, version = self._get_os_info(args, scan)
> -        except ImageFormatError as e:
> -            raise OperationFailed('KCHTMPL0020E', {'err': e.message})
> -        os_distro = args.get('os_distro', distro)
> -        os_version = args.get('os_version', version)
> -        entry = osinfo.lookup(os_distro, os_version)
> -        self.info.update(entry)
> +        # no record found: create template
> +        if scan:
> +            self._create_template(args, scan)
>
> -        # Auto-generate a template name if no one is passed
> -        if 'name' not in args or args['name'] == '':
> -            args['name'] = self._gen_name(distro, version)
> -        self.name = args['name']
> +        # template already exists: load it
> +        else:
> +            self.info.update(args)
>
>           # Merge graphics settings
>           graph_args = args.get('graphics')
> @@ -76,8 +75,6 @@ class VMTemplate(object):
>               graphics.update(graph_args)
>               args['graphics'] = graphics
>
> -        default_disk = self.info['disks'][0]
> -
>           # Complete memory args, because dict method update is not recursive
>           if 'memory' in args:
>               if 'current' not in args['memory']:
> @@ -87,6 +84,42 @@ class VMTemplate(object):
>
>           # Override template values according to 'args'
>           self.info.update(args)
> +
> +        if self.info.get("disks") is not None and len(self.info["disks"]) >= 1:
> +            default_disk = self.info['disks'][0]
> +
> +            # validate disks
> +            self._validate_disks(default_disk)
> +
> +    def _create_template(self, args, scan=False):
> +        """
> +        Creates a new template
> +        """
> +        # no source_media argument: raise error
> +        if args.get(SOURCE_MEDIA) is None:
> +            raise MissingParameter('KCHTMPL0016E')
> +
> +        # identify source media
> +        self._identify_installation_media(args)
> +
> +        # Fetch defaults based on the os distro and version
> +        try:
> +            distro, version = self._get_os_info(args, scan)
> +        except ImageFormatError as e:
> +            raise OperationFailed('KCHTMPL0020E', {'err': e.message})
> +        os_distro = args.get('os_distro', distro)
> +        os_version = args.get('os_version', version)
> +        entry = osinfo.lookup(os_distro, os_version)
> +
> +        # update info
> +        self.info.update(entry)
> +
> +        # Auto-generate a template name if no one is passed
> +        if 'name' not in args or args['name'] == '':
> +            args['name'] = self._gen_name(distro, version)
> +        self.info["name"] = args['name']
> +
> +    def _validate_disks(self, default_disk):
>           disks = self.info.get('disks')
>
>           basic_disk = ['index', 'format', 'pool', 'size']
> @@ -95,7 +128,6 @@ class VMTemplate(object):
>
>           for index, disk in enumerate(disks):
>               disk_info = dict(default_disk)
> -
>               pool_type = self._get_storage_type(disk['pool']['name'])
>               if pool_type in ['iscsi', 'scsi']:
>                   disk_info = {'index': 0, 'format': 'raw', 'volume': None}
> @@ -118,6 +150,53 @@ class VMTemplate(object):
>               disk_info['index'] = disk_info.get('index', index)
>               self.info['disks'][index] = disk_info
ok, leave this function here.
> +    def _identify_installation_media(self, args):
> +        path = args.get(SOURCE_MEDIA)
> +
> +        # user did not passed source media: return
> +        if path is None:
> +            return
> +
> +        # create magic object to discover file type
> +        file_type = magic.open(magic.MAGIC_NONE)
> +        file_type.load()
> +
> +        # file is local and exists: check if it's disk or cdrom
> +        if path.startswith("/") and os.path.exists(path):
> +            ftype = file_type.file(path)
> +
> +            # cdrom
> +            if ISO_TYPE in ftype:
> +                args["cdrom"] = path
> +                return
> +
> +            # disk
> +            for types in DISK_TYPE.keys():
> +                if types in ftype:
> +
> +                    # no disk list: create it
> +                    if args.get("disks") is None:
> +                        args["disks"] = []
> +
> +                    # append disk to the list
> +                    default_disk = {
> +                        "base": path, "format": DISK_TYPE[types], "index": 0,
> +                        "pool": {
> +                            "name": "/plugins/kimchi/storagepools/default",
> +                            "type": "dir"}}
> +                    args["disks"].insert(0, default_disk)
> +                    osi = osinfo.lookup("unknow", "unknow")["disks"][0]["pool"]
> +
> +                    # os_info has default pool: get it
> +                    if osi is None:
> +                        args["disks"][0]["pool"] = osi
> +                        return
> +
> +        # not local image: set as remote ISO
> +        elif check_url_path(path):
> +            args["cdrom"] = path
> +            return
> +
>       def _get_os_info(self, args, scan):
>           distro = version = 'unknown'
>




More information about the Kimchi-devel mailing list