[Kimchi-devel] [PATCH 2/4] Transient VM POST request handling

[Aline Manera]

On 19/01/2015 19:49, Christy Perez wrote:
> Ignore the exception on GET for PowerOff, Shutdown, and
> Reset actioins for VMs.
> ---
>   src/kimchi/control/vms.py | 18 ++++++++++++++----
>   src/kimchi/i18n.py        |  1 +
>   src/kimchi/model/vms.py   |  9 ++++++++-
>   3 files changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py
> index 1a84b6c..68e6e93 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,23 @@ 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 Exception:
> +            pass
> +

You should be handled in the model function (vm_is_persistent)

>           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})
> +

Why is it needed? libvirt retuns any error when trying to delete a 
non-persistent VM?

Should we also update the UI to avoid this kind of operation?

>           self._vmscreenshot_delete(dom.UUIDString())
>           paths = self._vm_get_disk_paths(dom)
>           info = self.lookup(name)