[Kimchi-devel] [PATCH][Kimchi 2/4] Identify installation media while creating template
Aline Manera
alinefm at linux.vnet.ibm.com
Fri Apr 8 16:29:41 UTC 2016
On 04/07/2016 03:47 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.
>
> Signed-off-by: Ramon Medeiros <ramonn at linux.vnet.ibm.com>
> ---
> model/templates.py | 75 +++++++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 54 insertions(+), 21 deletions(-)
>
> diff --git a/model/templates.py b/model/templates.py
> index 92705b6..65d04b7 100644
> --- a/model/templates.py
> +++ b/model/templates.py
> @@ -19,23 +19,28 @@
>
> import copy
> import libvirt
> +import magic
> import os
> import platform
> import psutil
> import stat
> +import urlparse
>
> -from wok.exception import InvalidOperation, InvalidParameter
> +from wok.exception import InvalidOperation, InvalidParameter, MissingParameter
> from wok.exception import NotFoundError, OperationFailed
> from wok.utils import probe_file_permission_as_user, run_setfacl_set_attr
> from wok.xmlutils.utils import xpath_get_text
>
> +from wok.plugins.kimchi import osinfo
> from wok.plugins.kimchi.config import get_kimchi_version
> from wok.plugins.kimchi.kvmusertests import UserTests
> from wok.plugins.kimchi.model.cpuinfo import CPUInfoModel
> from wok.plugins.kimchi.utils import pool_name_from_uri
> from wok.plugins.kimchi.vmtemplate import VMTemplate
>
> -
> +DISK_TYPE = {"QEMU QCOW Image": "qcow2",
> + "data": "raw"}
> +ISO_TYPE = "ISO 9660 CD-ROM"
> # In PowerPC, memories must be aligned to 256 MiB
> PPC_MEM_ALIGN = 256
> # Max memory 16TB for PPC and 4TiB for X (according to Red Hat), in KiB
> @@ -51,18 +56,11 @@ class TemplatesModel(object):
>
> def create(self, params):
> name = params.get('name', '').strip()
> - iso = params.get('cdrom')
> - # check search permission
> - if iso and iso.startswith('/') and os.path.exists(iso):
> - st_mode = os.stat(iso).st_mode
> - if stat.S_ISREG(st_mode) or stat.S_ISBLK(st_mode):
> - user = UserTests().probe_user()
> - run_setfacl_set_attr(iso, user=user)
> - ret, excp = probe_file_permission_as_user(iso, user)
> - if ret is False:
> - raise InvalidParameter('KCHISO0008E',
> - {'filename': iso, 'user': user,
> - 'err': excp})
> +
> + # template with the same name already exists: raise exception
> + with self.objstore as session:
> + if name in session.get_list('template'):
> + raise InvalidOperation("KCHTMPL0001E", {'name': name})
It is too early to verify the name here.
Let's say user did not pass any name. So according to the line above it
will be an empty string. You will check if a template with an empty
string name already exists.
Also after calling VMTemplate, a new name will be automatically
generated BUT there will be no confirmation it is unique.
>
> conn = self.conn.get()
> for net_name in params.get(u'networks', []):
> @@ -71,9 +69,46 @@ class TemplatesModel(object):
> except Exception:
> raise InvalidParameter("KCHTMPL0003E", {'network': net_name,
> 'template': name})
> +
> + # no source_media argument: raise error
> + if params.get("source_media") is None:
> + raise MissingParameter('KCHTMPL0016E')
As you add source_media as a required parameter in API.json you don't
need to verify it again as it will be always present at this point.
> +
> + path = params.pop("source_media")
> +
> + # not local image: set as remote ISO
> + if urlparse.urlparse(path).scheme in ["http", "https", "tftp", "ftp",
> + "ftps"]:
> + params["cdrom"] = path
> +
> + # image does not exists: raise error
> + if not os.path.exists(path):
> + raise InvalidParameter("Unable to find file %(path)s" %
> + {"path": path})
> +
It needs to be a 'elif' otherwise you will raise an error for remote ISO.
> + # create magic object to discover file type
> + file_type = magic.open(magic.MAGIC_NONE)
> + file_type.load()
> + ftype = file_type.file(path)
> +
> + # cdrom
> + if ISO_TYPE in ftype:
> + params["cdrom"] = path
> +
> + # disk
> + else:
> + for types in DISK_TYPE.keys():
> + if types in ftype:
> + # get default disk pool
> + params["disks"] = osinfo.lookup("unknow",
> + "unknow")["disks"]
> + params["disks"][0]["base"] = path
> +
Do not call osinfo here. You don't even know which OS it is in use and
you are assuming 'unknow'.
Only set the disk[0][base] to whatever you want.
Something like:
disks = params.get(disks, None)
if disks is None:
disks = [{'base': path}]
else:;
disks[0]['base'] = path
> + return self.validate_template_config(params)
> +
> + def validate_template_config(self, params):
> +
> # Creates the template class with necessary information
> - # Checkings will be done while creating this class, so any exception
> - # will be raised here
> t = LibvirtVMTemplate(params, scan=True, conn=self.conn)
>
> # Validate cpu info
> @@ -93,8 +128,6 @@ class TemplatesModel(object):
> name = params['name']
> try:
> with self.objstore as session:
> - if name in session.get_list('template'):
> - raise InvalidOperation("KCHTMPL0001E", {'name': name})
You need to keep it here to ensure the name generated is unique.
> session.store('template', name, t.info,
> get_kimchi_version())
> except InvalidOperation:
> @@ -158,7 +191,7 @@ class TemplateModel(object):
>
> temp = self.lookup(name)
> temp['name'] = clone_name
> - ident = self.templates.create(temp)
> + ident = self.templates.validate_template_config(temp)
As the validate_template_config is storing the data in the objecstore, I
suggest to rename it to "save_template()"
> return ident
>
> def delete(self, name):
> @@ -207,9 +240,9 @@ class TemplateModel(object):
>
> self.delete(name)
> try:
> - ident = self.templates.create(new_t)
> + ident = self.templates.validate_template_config(new_t)
> except:
> - ident = self.templates.create(old_t)
> + ident = self.templates.validate_template_config(old_t)
> raise
> return ident
>
More information about the Kimchi-devel
mailing list