[Kimchi-devel] [PATCH] Handle transient VM requests differently
Christy Perez
christy at linux.vnet.ibm.com
Mon Jan 19 16:14:33 UTC 2015
On 01/19/2015 07:41 AM, Aline Manera wrote:
>
> On 14/01/2015 21:24, Christy Perez wrote:
>> If a transient (or non-persistent) VM is created outside of
>> Kimchi, it will (expectedly) be deleted when shutdown. However,
>> Kimchi calls a GET after shutdown and other POST requests. When
>> the GET fails because the VM has been deleted, a user will get
>> an error. However, this is not an error condition.
>>
>> This patch checks to see if a VM is transient, and if so, does not
>> call the subsequent GET on the VM.
>
> The same scenario exists for networks and storage pools.
> Could you please also update them?
>
> Also as it is common to multiple resources we could move the
> verification to framework
>
> At control/base.py in in generate_action_handler_base():
>
> is_persistent = None
> if hasattr(model, model_fn(self, 'is_persistent')):
> is_persistent = getattr(self.model, model_fn(self, 'is_persistent'))
>
> (...)
>
> except NotFoundError, e:
> # only raise the exception if the resource is persistent
> if is_persistent is not None and is_persistent():
> raise cherrypy.HTTPError(404, e.message)
Sounds good. I'll get a v2 out ASAP with your suggestions.
Thanks!
>
>> ---
>> src/kimchi/control/base.py | 13 +++++++++----
>> src/kimchi/control/vms.py | 17 +++++++++++++----
>> src/kimchi/i18n.py | 1 +
>> src/kimchi/model/vms.py | 9 ++++++++-
>> 4 files changed, 31 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py
>> index 97e789f..8048370 100644
>> --- a/src/kimchi/control/base.py
>> +++ b/src/kimchi/control/base.py
>> @@ -63,7 +63,8 @@ def _redirect(self, ident, code=303):
>> uri_params += [urllib2.quote(ident.encode('utf-8'),
>> safe="")]
>> raise cherrypy.HTTPRedirect(self.uri_fmt %
>> tuple(uri_params), code)
>>
>> - def generate_action_handler(self, action_name, action_args=None):
>> + def generate_action_handler(self, action_name, action_args=None,
>> + ignore_not_found=False):
>> def _render_element(self, ident):
>> self._redirect(ident)
>> uri_params = []
>> @@ -75,7 +76,7 @@ def _render_element(self, ident):
>> raise internal_redirect(self.uri_fmt % tuple(uri_params))
>>
>> return self._generate_action_handler_base(action_name,
>> _render_element,
>> - action_args)
>> + ignore_not_found,
>> action_args)
>>
>> def generate_action_handler_task(self, action_name,
>> action_args=None):
>> def _render_task(self, task):
>> @@ -86,7 +87,7 @@ def _render_task(self, task):
>> action_args)
>>
>> def _generate_action_handler_base(self, action_name, render_fn,
>> - action_args=None):
>> + ignore_not_found,
>> action_args=None):
>> def wrapper(*args, **kwargs):
>> validate_method(('POST'), self.role_key,
>> self.admin_methods)
>> try:
>> @@ -111,7 +112,11 @@ def wrapper(*args, **kwargs):
>> except UnauthorizedError, e:
>> raise cherrypy.HTTPError(403, e.message)
>> except NotFoundError, e:
>> - raise cherrypy.HTTPError(404, e.message)
>> + if ignore_not_found is True:
>> + # Should something be returned instead?
>> + pass
>> + else:
>> + raise cherrypy.HTTPError(404, e.message)
>> except OperationFailed, e:
>> raise cherrypy.HTTPError(500, e.message)
>> except KimchiException, e:
>> diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py
>> index 1a84b6c..82682af 100644
>> --- a/src/kimchi/control/vms.py
>> +++ b/src/kimchi/control/vms.py
>> @@ -21,7 +21,6 @@
>> from kimchi.control.utils import internal_redirect, UrlSubNode
>> from kimchi.control.vm import sub_nodes
>>
>> -
>> @UrlSubNode('vms', True)
>> class VMs(Collection):
>> def __init__(self, model):
>> @@ -37,12 +36,22 @@ def __init__(self, model, ident):
>> self.role_key = 'guests'
>> self.screenshot = VMScreenShot(model, ident)
>> self.uri_fmt = '/vms/%s'
>> + _ignore_not_found = False
>> + try:
>> + # the test driver causes an exception in is_persistent()
>> + _ignore_not_found = \
>> + not model.vm_is_persistent(self.ident.decode('utf-8'))
>> + except:
>> + pass
>> for ident, node in sub_nodes.items():
>> setattr(self, ident, node(model, self.ident))
>> self.start = self.generate_action_handler('start')
>> - self.poweroff = self.generate_action_handler('poweroff')
>> - self.shutdown = self.generate_action_handler('shutdown')
>> - self.reset = self.generate_action_handler('reset')
>> + self.poweroff = self.generate_action_handler('poweroff',
>> + ignore_not_found = _ignore_not_found)
>> + self.shutdown = self.generate_action_handler('shutdown',
>> + ignore_not_found = _ignore_not_found)
>> + self.reset = self.generate_action_handler('reset',
>> + ignore_not_found = _ignore_not_found)
>> self.connect = self.generate_action_handler('connect')
>> self.clone = self.generate_action_handler_task('clone')
>>
>> diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
>> index 4eccc3e..4ab08d1 100644
>> --- a/src/kimchi/i18n.py
>> +++ b/src/kimchi/i18n.py
>> @@ -106,6 +106,7 @@
>> "KCHVM0033E": _("Virtual machine '%(name)s' must be stopped
>> before cloning it."),
>> "KCHVM0034E": _("Insufficient disk space to clone virtual
>> machine '%(name)s'"),
>> "KCHVM0035E": _("Unable to clone VM '%(name)s'. Details: %(err)s"),
>> + "KCHVM0036E": _("Invalid operation for non-persistent virtual
>> machine %(name)s"),
>>
>> "KCHVMHDEV0001E": _("VM %(vmid)s does not contain directly
>> assigned host device %(dev_name)s."),
>> "KCHVMHDEV0002E": _("The host device %(dev_name)s is not allowed
>> to directly assign to VM."),
>> diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py
>> index bae27c1..d3b4f1f 100644
>> --- a/src/kimchi/model/vms.py
>> +++ b/src/kimchi/model/vms.py
>> @@ -760,9 +760,13 @@ def lookup(self, name):
>> 'users': users,
>> 'groups': groups,
>> 'access': 'full',
>> - 'persistent': True if dom.isPersistent() else False
>> + 'persistent': self.is_persistent(name)
>> }
>>
>> + def is_persistent(self, name):
>> + dom = self.get_vm(name, self.conn)
>> + return True if dom.isPersistent() else False
>> +
>> def _vm_get_disk_paths(self, dom):
>> xml = dom.XMLDesc(0)
>> xpath = "/domain/devices/disk[@device='disk']/source/@file"
>> @@ -784,6 +788,9 @@ def get_vm(name, conn):
>> def delete(self, name):
>> conn = self.conn.get()
>> dom = self.get_vm(name, self.conn)
>> + if not dom.isPersistent():
>> + raise InvalidOperation("KCHVM0036E", {'name': name})
>> +
>> self._vmscreenshot_delete(dom.UUIDString())
>> paths = self._vm_get_disk_paths(dom)
>> info = self.lookup(name)
>
More information about the Kimchi-devel
mailing list