[PATCH] [Kimchi 0/2] Fix issues 920 and 923
This patchset fixes the issues 920 and 923. Depends on patches: Issue #919: Deactivate a storagepool makes the list of templates blank [Wok] Implement update methods for object store Jose Ricardo Ziviani (2): Issue #920: Template is removed if an ISO doesn't exist Issue #923: Template always shows VNC, even if I save Spice in my template i18n.py | 2 ++ model/templates.py | 42 +++++++++++++++++++++++----------- ui/js/src/kimchi.template_edit_main.js | 12 ++++++++-- 3 files changed, 41 insertions(+), 15 deletions(-) -- 1.9.1
- 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@linux.vnet.ibm.com> --- i18n.py | 2 ++ model/templates.py | 42 +++++++++++++++++++++++++++++------------- 2 files changed, 31 insertions(+), 13 deletions(-) diff --git a/i18n.py b/i18n.py index 1061aeb..f10133e 100644 --- a/i18n.py +++ b/i18n.py @@ -157,6 +157,7 @@ messages = { "KCHTMPL0001E": _("Template %(name)s already exists"), "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"), @@ -182,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 9294be2..1e6ad30 100644 --- a/model/templates.py +++ b/model/templates.py @@ -49,7 +49,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() iso = params.get('cdrom') # check search permission @@ -89,13 +89,18 @@ class TemplatesModel(object): if 'volume' in disk.keys(): self.template_volume_validate(volume, disk['pool']) + 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: if name in session.get_list('template'): raise InvalidOperation("KCHTMPL0001E", {'name': name}) - session.store('template', name, t.info, + session.store('template', name, template.info, get_kimchi_version()) except InvalidOperation: raise @@ -171,31 +176,30 @@ 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) # 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: @@ -205,13 +209,25 @@ 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.create(new_t) - except: - ident = self.templates.create(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: + 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): -- 1.9.1
Reviewed-by: Paulo Vital <pvital@linux.vnet.ibm.com> On Apr 07 04:07PM, 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@linux.vnet.ibm.com> --- i18n.py | 2 ++ model/templates.py | 42 +++++++++++++++++++++++++++++------------- 2 files changed, 31 insertions(+), 13 deletions(-)
diff --git a/i18n.py b/i18n.py index 1061aeb..f10133e 100644 --- a/i18n.py +++ b/i18n.py @@ -157,6 +157,7 @@ messages = { "KCHTMPL0001E": _("Template %(name)s already exists"), "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"), @@ -182,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 9294be2..1e6ad30 100644 --- a/model/templates.py +++ b/model/templates.py @@ -49,7 +49,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() iso = params.get('cdrom') # check search permission @@ -89,13 +89,18 @@ class TemplatesModel(object): if 'volume' in disk.keys(): self.template_volume_validate(volume, disk['pool'])
+ 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: if name in session.get_list('template'): raise InvalidOperation("KCHTMPL0001E", {'name': name}) - session.store('template', name, t.info, + session.store('template', name, template.info, get_kimchi_version()) except InvalidOperation: raise @@ -171,31 +176,30 @@ 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)
# 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: @@ -205,13 +209,25 @@ 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.create(new_t) - except: - ident = self.templates.create(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: + 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): -- 1.9.1
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
-- Paulo Ricardo Paz Vital Linux Technology Center, IBM Systems http://www.ibm.com/linux/ltc/
Hi Ziviani, I was not able to apply this patch in the current master branch. Could you, please, rebase and resend? Thanks, Aline Manera On 04/07/2016 04:07 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@linux.vnet.ibm.com> --- i18n.py | 2 ++ model/templates.py | 42 +++++++++++++++++++++++++++++------------- 2 files changed, 31 insertions(+), 13 deletions(-)
diff --git a/i18n.py b/i18n.py index 1061aeb..f10133e 100644 --- a/i18n.py +++ b/i18n.py @@ -157,6 +157,7 @@ messages = { "KCHTMPL0001E": _("Template %(name)s already exists"), "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"), @@ -182,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 9294be2..1e6ad30 100644 --- a/model/templates.py +++ b/model/templates.py @@ -49,7 +49,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() iso = params.get('cdrom') # check search permission @@ -89,13 +89,18 @@ class TemplatesModel(object): if 'volume' in disk.keys(): self.template_volume_validate(volume, disk['pool'])
+ 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: if name in session.get_list('template'): raise InvalidOperation("KCHTMPL0001E", {'name': name}) - session.store('template', name, t.info, + session.store('template', name, template.info, get_kimchi_version()) except InvalidOperation: raise @@ -171,31 +176,30 @@ 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)
# 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: @@ -205,13 +209,25 @@ 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.create(new_t) - except: - ident = self.templates.create(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: + 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):
Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> --- ui/js/src/kimchi.template_edit_main.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/ui/js/src/kimchi.template_edit_main.js b/ui/js/src/kimchi.template_edit_main.js index eda83b2..de72452 100644 --- a/ui/js/src/kimchi.template_edit_main.js +++ b/ui/js/src/kimchi.template_edit_main.js @@ -65,14 +65,22 @@ kimchi.template_edit_main = function() { $('#template-edit-memory-textbox').val(template.memory.current); $('#template-edit-max-memory-textbox').val(template.memory.maxmemory); - $('#template-edit-graphics').append('<option value="vnc" selected="selected">VNC</option>'); + if (template.graphics.type == 'vnc') { + $('#template-edit-graphics').append('<option value="vnc" selected="selected">VNC</option>'); + } else { + $('#template-edit-graphics').append('<option value="vnc">VNC</option>'); + } var enableSpice = function() { if (kimchi.capabilities == undefined) { setTimeout(enableSpice, 2000); return; } if (kimchi.capabilities.qemu_spice == true) { - $('#template-edit-graphics').append('<option value="spice">Spice</option>'); + if (template.graphics.type == 'spice') { + $('#template-edit-graphics').append('<option value="spice" selected="selected">Spice</option>'); + } else { + $('#template-edit-graphics').append('<option value="spice">Spice</option>'); + } } }; var isImageBasedTemplate = function() { -- 1.9.1
Reviewed-by: Paulo Vital <pvital@linux.vnet.ibm.com> On Apr 07 04:07PM, Jose Ricardo Ziviani wrote:
Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> --- ui/js/src/kimchi.template_edit_main.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/ui/js/src/kimchi.template_edit_main.js b/ui/js/src/kimchi.template_edit_main.js index eda83b2..de72452 100644 --- a/ui/js/src/kimchi.template_edit_main.js +++ b/ui/js/src/kimchi.template_edit_main.js @@ -65,14 +65,22 @@ kimchi.template_edit_main = function() { $('#template-edit-memory-textbox').val(template.memory.current); $('#template-edit-max-memory-textbox').val(template.memory.maxmemory);
- $('#template-edit-graphics').append('<option value="vnc" selected="selected">VNC</option>'); + if (template.graphics.type == 'vnc') { + $('#template-edit-graphics').append('<option value="vnc" selected="selected">VNC</option>'); + } else { + $('#template-edit-graphics').append('<option value="vnc">VNC</option>'); + } var enableSpice = function() { if (kimchi.capabilities == undefined) { setTimeout(enableSpice, 2000); return; } if (kimchi.capabilities.qemu_spice == true) { - $('#template-edit-graphics').append('<option value="spice">Spice</option>'); + if (template.graphics.type == 'spice') { + $('#template-edit-graphics').append('<option value="spice" selected="selected">Spice</option>'); + } else { + $('#template-edit-graphics').append('<option value="spice">Spice</option>'); + } } }; var isImageBasedTemplate = function() { -- 1.9.1
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
-- Paulo Ricardo Paz Vital Linux Technology Center, IBM Systems http://www.ibm.com/linux/ltc/
participants (3)
-
Aline Manera -
Jose Ricardo Ziviani -
Paulo Ricardo Paz Vital