[Kimchi-devel] [PATCH 2/2] Change 'cdrom' to a optional param

Aline Manera alinefm at linux.vnet.ibm.com
Thu Jul 10 12:36:07 UTC 2014



On 07/09/2014 07:03 AM, lvroyce at linux.vnet.ibm.com wrote:
> From: Royce Lv <lvroyce at linux.vnet.ibm.com>
>
> Multiple files modification for change 'cdrom' to optional param.
>
> Signed-off-by: Royce Lv <lvroyce at linux.vnet.ibm.com>
> ---
>   src/kimchi/control/templates.py |  2 +-
>   src/kimchi/i18n.py              |  2 +-
>   src/kimchi/mockmodel.py         | 26 ++++++++++++++++++++------
>   src/kimchi/model/templates.py   | 13 ++++++++-----
>   src/kimchi/vmtemplate.py        | 12 ++++++++++--
>   tests/test_rest.py              |  2 +-
>   6 files changed, 41 insertions(+), 16 deletions(-)
>
> diff --git a/src/kimchi/control/templates.py b/src/kimchi/control/templates.py
> index a535960..e16cc1a 100644
> --- a/src/kimchi/control/templates.py
> +++ b/src/kimchi/control/templates.py
> @@ -47,7 +47,7 @@ class Template(Resource):
>                   'os_version': self.info['os_version'],
>                   'cpus': self.info['cpus'],
>                   'memory': self.info['memory'],
> -                'cdrom': self.info['cdrom'],
> +                'cdrom': self.info.get('cdrom', None),
>                   'disks': self.info['disks'],
>                   'storagepool': self.info['storagepool'],
>                   'networks': self.info['networks'],
> diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
> index 7a7b675..3ba1e23 100644
> --- a/src/kimchi/i18n.py
> +++ b/src/kimchi/i18n.py
> @@ -118,7 +118,7 @@ messages = {
>       "KCHTMPL0013E": _("Amount of memory (MB) must be an integer greater than 512"),
>       "KCHTMPL0014E": _("Template CDROM must be a local or remote ISO file"),
>       "KCHTMPL0015E": _("Invalid storage pool URI %(value)s specified for template"),
> -    "KCHTMPL0016E": _("Specify an ISO image as CDROM to create a template"),
> +    "KCHTMPL0016E": _("Specify an ISO image as CDROM or a base image to create a template"),
>       "KCHTMPL0017E": _("All networks for the template must be specified in a list."),
>       "KCHTMPL0018E": _("Must specify a volume to a template, when storage pool is iscsi or scsi"),
>       "KCHTMPL0019E": _("The volume: %(volume)s in not in storage pool %(pool)s"),
> diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py
> index a0920e0..e14990e 100644
> --- a/src/kimchi/mockmodel.py
> +++ b/src/kimchi/mockmodel.py
> @@ -200,8 +200,9 @@ class MockModel(object):
>               index += 1
>
>           cdrom = "hd" + string.ascii_lowercase[index + 1]
> -        cdrom_params = {'dev': cdrom, 'path': t_info['cdrom'], 'type': 'cdrom'}
> -        vm.storagedevices[cdrom] = MockVMStorageDevice(cdrom_params)
> +        if t_info.get('cdrom'):
> +            cdrom_params = {'dev': cdrom, 'path': t_info['cdrom'], 'type': 'cdrom'}
> +            vm.storagedevices[cdrom] = MockVMStorageDevice(cdrom_params)
>
>           self._mock_vms[name] = vm
>           return name
> @@ -237,11 +238,24 @@ class MockModel(object):
>
>       def templates_create(self, params):
>           name = params.get('name', '').strip()
> +        iso = params.get('cdrom')
>           if not name:
> -            iso = params['cdrom']
> -            iso_name = os.path.splitext(iso[iso.rfind('/') + 1:])[0]
> -            name = iso_name + str(int(time.time() * 1000))
> -            params['name'] = name
> +            if iso:
> +                iso_name = os.path.splitext(iso[iso.rfind('/') + 1:])[0]
> +                name = iso_name + str(int(time.time() * 1000))
> +                params['name'] = name
> +            else:
> +                raise InvalidParameter("KCHTMPL0008E")
> +


> +        base_tmpl = False
> +        if not iso:
> +            if 'disks' in params:
> +                for d in args['disks']:
> +                    if 'base' in d:
> +                        base_tmpl = True
> +                        break
> +            if not base_tmpl:
> +                raise InvalidParameter("KCHTMPL0016E")

Why should that code be added here?

>
>           if name in self._mock_templates:
>               raise InvalidOperation("KCHTMPL0001E", {'name': name})
> diff --git a/src/kimchi/model/templates.py b/src/kimchi/model/templates.py
> index 60f4de5..0e07a78 100644
> --- a/src/kimchi/model/templates.py
> +++ b/src/kimchi/model/templates.py
> @@ -40,9 +40,9 @@ class TemplatesModel(object):
>
>       def create(self, params):
>           name = params.get('name', '').strip()
> -        iso = params['cdrom']
> +        iso = params.get('cdrom')
>           # check search permission
> -        if iso.startswith('/') and os.path.isfile(iso):
> +        if iso and iso.startswith('/') and os.path.isfile(iso):
>               user = UserTests().probe_user()
>               ret, excp = probe_file_permission_as_user(iso, user)
>               if ret is False:
> @@ -51,9 +51,12 @@ class TemplatesModel(object):
>                                           'err': excp})
>
>           if not name:
> -            iso_name = os.path.splitext(iso[iso.rfind('/') + 1:])[0]
> -            name = iso_name + str(int(time.time() * 1000))
> -            params['name'] = name
> +            if iso:
> +                iso_name = os.path.splitext(iso[iso.rfind('/') + 1:])[0]
> +                name = iso_name + str(int(time.time() * 1000))
> +                params['name'] = name
> +            else:
> +                raise InvalidParameter('KCHTMPL0008E')
>
>           conn = self.conn.get()
>           pool_uri = params.get(u'storagepool', '')
> diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py
> index 8d5217a..91617d9 100644
> --- a/src/kimchi/vmtemplate.py
> +++ b/src/kimchi/vmtemplate.py
> @@ -54,6 +54,14 @@ class VMTemplate(object):
>           iso_distro = iso_version = 'unknown'
>           iso = args.get('cdrom', '')
>


> +        # if ISO not specified and base disk image specified,
> +        # prevent cdrom from filling automatically
> +        if len(iso) == 0 and 'disks' in args:
> +            for d in args['disks']:
> +                if 'base' in d:
> +                    args['cdrom'] = ''
> +                    break
> +


Why is that code needed?
If the ISO was not specified the "cdrom" parameter won't exist in the 
dict - so we should not add a cdrom automatically

We should only verify if a ISO or a base disk is specified to create a 
template and raise a error if none of them exists



>           if scan and len(iso) > 0:
>               iso_distro, iso_version = self.get_iso_info(iso)
>               if not iso.startswith('/'):
> @@ -414,8 +422,8 @@ drive=drive-%(bus)s0-1-0,id=%(bus)s0-1-0'/>
>
>           # validate iso integrity
>           # FIXME when we support multiples cdrom devices
> -        iso = self.info['cdrom']
> -        if not (os.path.isfile(iso) or check_url_path(iso)):
> +        iso = self.info.get('cdrom')
> +        if iso and not (os.path.isfile(iso) or check_url_path(iso)):
>               invalid['cdrom'] = [iso]
>
>           self.info['invalid'] = invalid
> diff --git a/tests/test_rest.py b/tests/test_rest.py
> index ad8fc72..5d84e85 100644
> --- a/tests/test_rest.py
> +++ b/tests/test_rest.py
> @@ -1068,7 +1068,7 @@ class RestTests(unittest.TestCase):
>           self.assertEquals(200, resp.status)
>           self.assertEquals(0, len(json.loads(resp.read())))
>
> -        # Create a template without cdrom fails with 400
> +        # Create a template without cdrom and disk specified fails with 400
>           t = {'name': 'test', 'os_distro': 'ImagineOS',
>                'os_version': '1.0', 'memory': 1024, 'cpus': 1,
>                'storagepool': '/storagepools/alt'}
>




More information about the Kimchi-devel mailing list