[Kimchi-devel] [PATCH][Kimchi 3/4] Identify installation media while creating template
Rodrigo Trujillo
rodrigo.trujillo at linux.vnet.ibm.com
Wed Mar 30 15:45:59 UTC 2016
On 03/29/2016 01:18 PM, Ramon Medeiros wrote:
>
>
> 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>
>>> ---
>>> model/templates.py | 4 ++
>>> vmtemplate.py | 112
>>> ++++++++++++++++++++++++++++++++++++++++++++++-------
>>> 2 files changed, 101 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/model/templates.py b/model/templates.py
>>> index 28c8a08..30d403e 100644
>>> --- a/model/templates.py
>>> +++ b/model/templates.py
>>> @@ -198,6 +198,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")
>>> +
>> Is this part to support old API ?
>> Have you tested with old templates ? Did you think about update
>> objectstore to new API stile ?
>>
>>> 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
>>> +
>>> + # 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})
> This one was returning 500 before this patch. Do you really want to
> change it? As you can see, this error sounds more like an failed
> operation then a missed parameter
>> Same here
This seems to be ok keep it 500
>>
>>> + 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 ?
>>
>>>
>>> + 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
>>
>
More information about the Kimchi-devel
mailing list