[Kimchi-devel] [PATCH][Kimchi 3/4] Identify installation media while creating template
Rodrigo Trujillo
rodrigo.trujillo at linux.vnet.ibm.com
Tue Mar 29 15:37:12 UTC 2016
On 03/28/2016 03:34 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 | 112 ++++++++++++++++++++++++++++++++++++++++++++++-------
> 2 files changed, 101 insertions(+), 15 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")
> +
Is this part to support old API ?
Have you tested with old templates ? Did you think about update
objectstore to new API stile ?
> new_t.update(params)
>
> for net_name in params.get(u'networks', []):
> diff --git a/vmtemplate.py b/vmtemplate.py
> index 653ad02..de69eed 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 think that a global variable for a parameter field is not necessary.
Its strange.
> +
>
> 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 == True:
> + 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,7 +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:
> @@ -87,6 +85,42 @@ class VMTemplate(object):
>
> # Override template values according to 'args'
> self.info.update(args)
> +
> + if self.info.get("disks") != 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) == None:
> + raise OperationFailed('KCHTMPL0016E')
This is going to return error 500 ... please replace by MissingParameter
which returns 400
> +
> + # 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})
Same here
> + 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 +129,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 +151,55 @@ class VMTemplate(object):
> disk_info['index'] = disk_info.get('index', index)
> self.info['disks'][index] = disk_info
I am not sure about have the function below in vmtemplates.
Wouldn't it fit in osinfo.py ?
>
> + 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
> + args["disks"].insert(0,
> + {"base": path,
> + "format": DISK_TYPE[types],
> + "index": 0,
> + "pool": {"name":
> + "/plugins/kimchi/storagepools/default",
> + "type":"dir"}})
> +
> + os_info = osinfo.lookup("unknow", "unknow")["disks"][0]["pool"]
> +
> + # os_info has default pool: get it
> + if os_info != None:
> + args["disks"][0]["pool"] = os_info
> + 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