[Kimchi-devel] [Kimchi 4/4] Identify installation media while creating template
Rodrigo Trujillo
rodrigo.trujillo at linux.vnet.ibm.com
Thu Dec 24 13:20:42 UTC 2015
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 at 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'
>
More information about the Kimchi-devel
mailing list