[Kimchi-devel] [PATCH] Handle transient VM requests differently

[Christy Perez]

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.
---
 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)
-- 
1.9.3