[Kimchi-devel] [PATCH] Handle transient VM requests differently
Aline Manera
alinefm at linux.vnet.ibm.com
Mon Jan 19 13:41:18 UTC 2015
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)
> ---
> 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