[Kimchi-devel] [PATCH v3] [Kimchi] Issue #920: Template is removed if an ISO doesn't exist
Daniel Henrique Barboza
dhbarboza82 at gmail.com
Fri Apr 15 21:40:20 UTC 2016
Reviewed-by: Daniel Barboza <dhbarboza82 at gmail.com>
On 04/15/2016 05:48 PM, Jose Ricardo Ziviani wrote:
> - This commit changes the way the template update works. Now, it
> really updates the existing template instead of creating a new
> one (and deleting the old one)
>
> Signed-off-by: Jose Ricardo Ziviani <joserz at linux.vnet.ibm.com>
> ---
> i18n.py | 3 +++
> model/templates.py | 70 ++++++++++++++++++++++++++++++++------------------
> tests/test_template.py | 6 ++++-
> 3 files changed, 53 insertions(+), 26 deletions(-)
>
> diff --git a/i18n.py b/i18n.py
> index 7cce796..e4656cb 100644
> --- a/i18n.py
> +++ b/i18n.py
> @@ -155,8 +155,10 @@ messages = {
> "KCHVMIF0011E": _("Cannot change MAC address of a running virtual machine"),
>
> "KCHTMPL0001E": _("Template %(name)s already exists"),
> + "KCHTMPL0002E": _("Source media %(path)s not found"),
> "KCHTMPL0003E": _("Network '%(network)s' specified for template %(template)s does not exist"),
> "KCHTMPL0004E": _("Storage pool %(pool)s specified for template %(template)s does not exist"),
> + "KCHTMPL0005E": _("Template %(name)s does not exist."),
> "KCHTMPL0006E": _("Invalid parameter '%(param)s' specified for CDROM."),
> "KCHTMPL0007E": _("Network %(network)s specified for template %(template)s is not active"),
> "KCHTMPL0008E": _("Template name must be a string"),
> @@ -181,6 +183,7 @@ messages = {
> "KCHTMPL0029E": _("Disk format must be 'raw', for logical, iscsi, and scsi pools."),
> "KCHTMPL0030E": _("Memory expects an object with one or both parameters: 'current' and 'maxmemory'"),
> "KCHTMPL0031E": _("Memory value (%(mem)sMiB) must be equal or lesser than maximum memory value (%(maxmem)sMiB)"),
> + "KCHTMPL0032E": _("Unable to update template due error: %(err)s"),
>
> "KCHPOOL0001E": _("Storage pool %(name)s already exists"),
> "KCHPOOL0002E": _("Storage pool %(name)s does not exist"),
> diff --git a/model/templates.py b/model/templates.py
> index a02099c..d0742ca 100644
> --- a/model/templates.py
> +++ b/model/templates.py
> @@ -51,7 +51,7 @@ class TemplatesModel(object):
> self.objstore = kargs['objstore']
> self.conn = kargs['conn']
>
> - def create(self, params):
> + def _validate_template_params(self, params):
> name = params.get('name', '').strip()
>
> conn = self.conn.get()
> @@ -72,8 +72,7 @@ class TemplatesModel(object):
>
> # image does not exists: raise error
> elif not os.path.exists(path):
> - raise InvalidParameter("Unable to find file %(path)s" %
> - {"path": path})
> + raise InvalidParameter("KCHTMPL0002E", {'path': path})
>
> # create magic object to discover file type
> file_type = magic.open(magic.MAGIC_NONE)
> @@ -99,10 +98,6 @@ class TemplatesModel(object):
> params["disks"] = params.get('disks', [])
> params["disks"].append({"base": path})
>
> - return self.save_template(params)
> -
> - def save_template(self, params):
> -
> # Creates the template class with necessary information
> t = LibvirtVMTemplate(params, scan=True, conn=self.conn)
>
> @@ -119,16 +114,18 @@ class TemplatesModel(object):
> if 'volume' in disk.keys():
> self.template_volume_validate(volume, disk['pool'])
>
> - # template with the same name already exists: raise exception
> - name = params['name']
> - with self.objstore as session:
> - if name in session.get_list('template'):
> - raise InvalidOperation("KCHTMPL0001E", {'name': name})
> + return t
> +
> + def create(self, params):
> + template = self._validate_template_params(params)
>
> # Store template on objectstore
> + name = params['name']
> try:
> with self.objstore as session:
> - session.store('template', name, t.info,
> + if name in session.get_list('template'):
> + raise InvalidOperation("KCHTMPL0001E", {'name': name})
> + session.store('template', name, template.info,
> get_kimchi_version())
> except InvalidOperation:
> raise
> @@ -191,7 +188,8 @@ class TemplateModel(object):
>
> temp = self.lookup(name)
> temp['name'] = clone_name
> - ident = self.templates.save_template(temp)
> + temp['source_media'] = temp.get('base', temp['cdrom'])
> + ident = self.templates.create(temp)
> return ident
>
> def delete(self, name):
> @@ -204,31 +202,36 @@ class TemplateModel(object):
> raise OperationFailed('KCHTMPL0021E', {'err': e.message})
>
> def update(self, name, params):
> - old_t = self.lookup(name)
> - new_t = copy.copy(old_t)
> + edit_template = self.lookup(name)
> +
> + edit_template['source_media'] = edit_template.get(
> + 'base', edit_template['cdrom'])
> +
> + if 'cdrom' in params:
> + edit_template['source_media'] = params['cdrom']
>
> # Merge graphics settings
> graph_args = params.get('graphics')
> if graph_args:
> - graphics = dict(new_t['graphics'])
> + graphics = dict(edit_template['graphics'])
> graphics.update(graph_args)
> params['graphics'] = graphics
>
> # Merge cpu_info settings
> new_cpu_info = params.get('cpu_info')
> if new_cpu_info:
> - cpu_info = dict(new_t['cpu_info'])
> + cpu_info = dict(edit_template['cpu_info'])
> cpu_info.update(new_cpu_info)
> params['cpu_info'] = cpu_info
>
> # Fix memory values, because method update does not work recursively
> new_mem = params.get('memory')
> if new_mem is not None:
> - params['memory'] = copy.copy(old_t.get('memory'))
> + params['memory'] = copy.copy(edit_template.get('memory'))
> params['memory'].update(new_mem)
> validate_memory(params['memory'])
>
> - new_t.update(params)
> + edit_template.update(params)
>
> for net_name in params.get(u'networks', []):
> try:
> @@ -238,13 +241,30 @@ class TemplateModel(object):
> raise InvalidParameter("KCHTMPL0003E", {'network': net_name,
> 'template': name})
>
> - self.delete(name)
> + template = self.templates._validate_template_params(edit_template)
> +
> try:
> - ident = self.templates.save_template(new_t)
> - except:
> - ident = self.templates.save_template(old_t)
> + with self.objstore as session:
> + if name not in session.get_list('template'):
> + raise InvalidOperation("KCHTMPL0005E", {'name': name})
> +
> + if template.info['name'] != name and \
> + template.info['name'] in session.get_list('template'):
> + raise InvalidOperation("KCHTMPL0001E",
> + {'name': template.info['name']})
> +
> + if template.info['name'] != name:
> + session.delete('template', name)
> +
> + session.store('template', template.info['name'],
> + template.info, get_kimchi_version())
> +
> + except InvalidOperation:
> raise
> - return ident
> + except Exception, e:
> + raise OperationFailed('KCHTMPL0032E', {'err': e.message})
> +
> + return template.info['name']
>
>
> def validate_memory(memory):
> diff --git a/tests/test_template.py b/tests/test_template.py
> index 158bbeb..f0ef030 100644
> --- a/tests/test_template.py
> +++ b/tests/test_template.py
> @@ -256,7 +256,11 @@ class TemplateTests(unittest.TestCase):
> self.assertEquals(1024, update_tmpl['memory']['maxmemory'])
>
> # Update cdrom
> - cdrom_data = {'cdrom': '/tmp/mock2.iso'}
> + cdrom_data = {'cdrom': 'inexistent.iso'}
> + resp = self.request(new_tmpl_uri, json.dumps(cdrom_data), 'PUT')
> + self.assertEquals(400, resp.status)
> +
> + cdrom_data = {'cdrom': '/tmp/existent.iso'}
> resp = self.request(new_tmpl_uri, json.dumps(cdrom_data), 'PUT')
> self.assertEquals(200, resp.status)
> update_tmpl = json.loads(resp.read())
More information about the Kimchi-devel
mailing list