On 04/06/2016 01:18 AM, 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(a)linux.vnet.ibm.com>
---
model/templates.py | 17 +++------
vmtemplate.py | 104 ++++++++++++++++++++++++++++++++++++++++++++---------
2 files changed, 92 insertions(+), 29 deletions(-)
diff --git a/model/templates.py b/model/templates.py
index 28c8a08..ca540cd 100644
--- a/model/templates.py
+++ b/model/templates.py
@@ -51,25 +51,12 @@ class TemplatesModel(object):
def create(self, params):
name = params.get('name', '').strip()
- iso = params.get('cdrom')
Join this patch with 2/4
# 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})
- # 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})
-
conn = self.conn.get()
for net_name in params.get(u'networks', []):
try:
@@ -198,6 +185,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")
+
WHY? cdrom will not be on params! Now the only way to create a Template
is through source_media!
new_t.update(params)
for net_name in params.get(u'networks', []):
diff --git a/vmtemplate.py b/vmtemplate.py
index 653ad02..7073d3a 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,10 @@ 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"
+
class VMTemplate(object):
def __init__(self, args, scan=False):
@@ -54,20 +59,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:
The 'scan' is not related to new records! It is only to identify if the
source_media will be scanned to get its OS and OS version. All the
validation should continue to happen in all cases.
+ 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']
Why did you remove the above lines?
Where the template name is generated if no one is passed?
+ # template already exists: load it
+ else:
+ self.info.update(args)
# Merge graphics settings
graph_args = args.get('graphics')
@@ -76,8 +74,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 +83,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.name = args['name']
+
+ def _validate_disks(self, default_disk):
disks = self.info.get('disks')
basic_disk = ['index', 'format', 'pool',
'size']
@@ -95,7 +127,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 +149,47 @@ class VMTemplate(object):
disk_info['index'] = disk_info.get('index', index)
self.info['disks'][index] = disk_info
+ 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)
It is better to check for http/https/tftp/ftp/ftps and already add it to
cdrom before starting using magic.
Example:
if path.startswith(http/https/tftp/ftp/ftps):
args[cdrom] = path
return
if not os.path.exists(path):
raise InvalidParameter(Unable to find file %(path)s)
# Otherwise, use magic to identify the file
# 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:
+ args["cdrom"] = path
+ return
+
# if it is not a cdrom, assume it is an image
else:
disks[0][base] = path
+ # disk
+ for types in DISK_TYPE.keys():
+ if types in ftype:
+
We don't need to do that as it is an easy point of failure due the
amount of disk format we have.
+ # no disk list: create it
+ if args.get("disks") is None:
+ args["disks"] = []
+
+ osinfo_disk = osinfo.lookup("unknow",
"unknow")["disks"]
+
+ # os_info has default pool: get it
+ if osinfo_disk is not None:
+ args["disks"] = osinfo_disk
+
Do not do this! The osinfo information will be get in other place. Only
set disk[base] value as you did below.
+ args["disks"][0]["base"] =
path
+
+ # not local image: set as remote ISO
+ elif check_url_path(path):
+ args["cdrom"] = path
+ return
+
isoinfo will do that verification! Here you can check the path starts
with some patterns as I described below.
def _get_os_info(self, args, scan):
distro = version = 'unknown'