11:29 a.m.
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(a)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