[Kimchi-devel] [Kimchi 4/4] Identify installation media while creating template

Ramon Medeiros ramonn at linux.vnet.ibm.com
Mon Dec 28 12:22:46 UTC 2015



On 12/24/2015 11:20 AM, Rodrigo Trujillo wrote:
>
>
> 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
yes. As you can see, the next block will point to self.info: graphics = 
dict(self.info['graphics'])
>> +
>> +        # 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
We are checking if:

os.path.exists

Then, if not, we check if it's starts with http or ftp. This case will 
not match in any of the checkings
>> +
>> +    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'
>>
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>

-- 

Ramon Nunes Medeiros
Kimchi Developer
Linux Technology Center Brazil
IBM Systems & Technology Group
Phone : +55 19 2132 7878
ramonn at br.ibm.com




More information about the Kimchi-devel mailing list