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

Ramon Medeiros ramonn at linux.vnet.ibm.com
Tue Mar 29 16:15:58 UTC 2016



On 03/29/2016 12:37 PM, Rodrigo Trujillo wrote:
>
>
> On 03/28/2016 03:34 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>
>> -
>>
>> +        # params are cdrom or disk: add it to source_media
>> +        if "cdrom" in params:
>> +            params["source_media"] = params.get("cdrom")
>> +
> Is this part to support old API ?
> Have you tested with old templates ? Did you think about update 
> objectstore to new API stile ?
this case is used when the user updates the disk of cdrom.
>
>>           new_t.update(params)
>>
>>           for net_name in params.get(u'networks', []):
>> diff --git a/vmtemplate.py b/vmtemplate.py
>> index 653ad02..de69eed 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,11 @@ 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"
>> +SOURCE_MEDIA = "source_media"
> I think that a global variable for a parameter field is not necessary. 
> Its strange.
>> +
>>
>>   class VMTemplate(object):
>>       def __init__(self, args, scan=False):
>> @@ -54,20 +60,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 == True:
>> +            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']
>> +        # template already exists: load it
>> +        else:
>> +            self.info.update(args)
>>
>>           # Merge graphics settings
>>           graph_args = args.get('graphics')
>> @@ -76,7 +75,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:
>> @@ -87,6 +85,42 @@ class VMTemplate(object):
>>
>>           # Override template values according to 'args'
>>           self.info.update(args)
>> +
>> +        if self.info.get("disks") != 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) == None:
>> +            raise OperationFailed('KCHTMPL0016E')
> This is going to return error 500 ... please replace by 
> MissingParameter which returns 400
ok
>> +
>> +        # 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})
> Same here
>
ok
>> +        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.info["name"] = args['name']
>> +
>> +    def _validate_disks(self, default_disk):
>>           disks = self.info.get('disks')
>>
>>           basic_disk = ['index', 'format', 'pool', 'size']
>> @@ -95,7 +129,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 +151,55 @@ class VMTemplate(object):
>>               disk_info['index'] = disk_info.get('index', index)
>>               self.info['disks'][index] = disk_info
>
> I am not sure about have the function below in vmtemplates.
> Wouldn't it fit in osinfo.py  ?
>
why did not fit in vmteplates?
>>
>> +    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)
>> +
>> +            # 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") is None:
>> +                        args["disks"] = []
>> +
>> +                    # append disk to the list
>> +                    args["disks"].insert(0,
>> +                        {"base": path,
>> +                         "format": DISK_TYPE[types],
>> +                         "index": 0,
>> +                         "pool": {"name":
>> + "/plugins/kimchi/storagepools/default",
>> +                                  "type":"dir"}})
>> +
>> +                    os_info = osinfo.lookup("unknow", 
>> "unknow")["disks"][0]["pool"]
>> +
>> +                    # os_info has default pool: get it
>> +                    if os_info != None:
>> +                        args["disks"][0]["pool"] = os_info
>> +                        return
>> +
>> +        # not local image: set as remote ISO
>> +        elif check_url_path(path):
>> +            args["cdrom"] = path
>> +            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