On 12/23/2015 01:12 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(a)linux.vnet.ibm.com>
---
vmtemplate.py | 122 +++++++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 108 insertions(+), 14 deletions(-)
diff --git a/vmtemplate.py b/vmtemplate.py
index d629226..a209578 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 psutil
@@ -28,12 +29,14 @@ from lxml import etree
from lxml.builder import E
from wok.exception import InvalidParameter, ImageFormatError, IsoFormatError
-from wok.exception import MissingParameter, OperationFailed
+from wok.exception import MissingParameter, OperationFailed, InvalidOperation
+from wok.xmlutils.utils import xpath_get_text
from wok.plugins.kimchi import imageinfo
from wok.plugins.kimchi import osinfo
from wok.plugins.kimchi.isoinfo import IsoImage
-from wok.plugins.kimchi.utils import check_url_path, pool_name_from_uri
+from wok.plugins.kimchi.model.libvirtconnection import LibvirtConnection
+from wok.plugins.kimchi.utils import check_url_path, pool_name_from_uri,
get_template_by_name
from wok.plugins.kimchi.xmlutils.cpu import get_cpu_xml
from wok.plugins.kimchi.xmlutils.disk import get_disk_xml
from wok.plugins.kimchi.xmlutils.graphics import get_graphics_xml
@@ -41,6 +44,9 @@ 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"
# In PowerPC, memories must be aligned to 256 MiB
PPC_MEM_ALIGN = 256
@@ -61,6 +67,36 @@ class VMTemplate(object):
self.info = {}
self.fc_host_support = args.get('fc_host_support')
+ # search for template with the same name
+ template = get_template_by_name(args.get("name"))
+
+ # no record found: create template
+ if len(template) == 0:
+ self._create_template(args, scan)
+
+ # template already exists: load it
+ else:
+ self.info.update(args)
Is this else + self.info.update(args) necessary ? I am asking because
there is another info.update right below
+
+ # Merge graphics settings
+ graph_args = args.get('graphics')
+ if graph_args:
+ graphics = dict(self.info['graphics'])
+ graphics.update(graph_args)
+ args['graphics'] = graphics
+
+ # validate disks
+ default_disk = self.info['disks'][0]
+ self.info.update(args)
+ self._validate_disks(default_disk)
+
+ def _create_template(self, args, scan=False):
+ """
+ Creates a new template
+ """
+ # identify installation media
+ self._identify_installation_media(args)
+
# Fetch defaults based on the os distro and version
try:
distro, version = self._get_os_info(args, scan)
@@ -69,23 +105,16 @@ class VMTemplate(object):
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']
-
- # Merge graphics settings
- graph_args = args.get('graphics')
- if graph_args:
- graphics = dict(self.info['graphics'])
- graphics.update(graph_args)
- args['graphics'] = graphics
+ self.info["name"] = args['name']
- default_disk = self.info['disks'][0]
- # Override template values according to 'args'
- self.info.update(args)
+ def _validate_disks(self, default_disk):
disks = self.info.get('disks')
basic_disk = ['index', 'format', 'pool',
'size']
@@ -94,7 +123,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}
@@ -117,6 +145,72 @@ 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("installation_media")
+
+ # user did not passed installation media: return
+ if path == 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") == None:
+ args["disks"] = []
+
+ # append disk to the list
+ args["disks"].insert(0, {"base": path,
+ "format": DISK_TYPE[types],
+ "index": 0
+ })
+ # get storage pool
+ pool = self._get_storagepool_from_storagevolume(path)
+ args["disks"][0]['pool'] = pool
+ return
+
+ # not local image: set as remote ISO
+ elif path.startswith("http") or path.startswith("ftp"):
+ args["cdrom"] = path
+ return
I think you must add an ELSE statement here, because, I am not
sure what
is going to happen if path is something strange, like "43242/4343/232"
Have you searched if there is something similar in other parts ? Or
another way to get the same result ?
If there is not , then I am ok with this function, however , I think you
would have to move it to another place, because I was told that we
should not use Libvirt or call libvirt in vmtemplates.py
+
+ def _get_storagepool_from_storagevolume(self, storagevolume_path):
+ conn = LibvirtConnection('qemu:///system').get()
+ storagepools = conn.listStoragePools()
+ storagepools += conn.listDefinedStoragePools()
+
+ # discover storagepool
+ for storagepool in storagepools:
+ pool = conn.storagePoolLookupByName(storagepool)
+ spath = xpath_get_text(pool.XMLDesc(), "/pool/target/path")[0]
+ pool_type = xpath_get_text(pool.XMLDesc(), "/pool/@type")[0]
+ for storagevolume in sorted(map(lambda x: x.decode('utf-8'),
pool.listVolumes())):
+
+ # path does not exists: continue to the next
+ if not os.path.exists("%s/%s" % (spath, storagevolume)):
+ continue
+
+ # storagepool found: return
+ if os.path.samefile(storagevolume_path,"%s/%s" % (spath,
storagevolume)):
+ return {"name":
"/plugins/kimchi/storagepools/%s" % storagepool,
+ "type": pool_type
+ }
+ return {}
+
def _get_os_info(self, args, scan):
distro = version = 'unknown'