[Kimchi-devel] [PATCH] [Kimchi] Kimchi kills Wokd due to sys.exit() calls in files networks.py and storagepools.py

[Ramon Medeiros]

Would be nice to create a custom exception, like happens on 
src/wok/exception.py. You can also set the http status

On 07/22/2016 08:14 PM, bianca at linux.vnet.ibm.com wrote:
> From: Bianca Carvalho <bianca at linux.vnet.ibm.com>
>
> Removed all sys.exit(1) calls from both files to avoid killing Wokd.
> Also added an Exception to not load kimchi plugin in case network or
> storagepool does not exist or is not active.
>
> Signed-off-by: Bianca Carvalho <bianca at linux.vnet.ibm.com>
> ---
>   model/networks.py     | 16 +++++++---------
>   model/storagepools.py | 19 ++++++++-----------
>   2 files changed, 15 insertions(+), 20 deletions(-)
>
> diff --git a/model/networks.py b/model/networks.py
> index 4b722d3..35431d4 100644
> --- a/model/networks.py
> +++ b/model/networks.py
> @@ -20,7 +20,6 @@
>   import copy
>   import ipaddr
>   import libvirt
> -import sys
>   import time
>   from libvirt import VIR_INTERFACE_XML_INACTIVE
>
> @@ -59,19 +58,19 @@ class NetworksModel(object):
>           networks = list(set(tmpl_defaults['networks']))
>           conn = self.conn.get()
>
> -        error_msg = ("Please, check the configuration in %s/template.conf to "
> -                     "ensure it lists only valid networks." %
> -                     kimchiPaths.sysconf_dir)
> -
>           for net_name in networks:
> +            error_msg = ("Network %s does not exist or is not "
> +                         "active. Please, check the configuration in "
> +                         "%s/template.conf to ensure it lists only valid "
> +                         "networks." % (net_name, kimchiPaths.sysconf_dir))
> +
>               try:
>                   net = conn.networkLookupByName(net_name)
>               except libvirt.libvirtError, e:
>                   msg = "Fatal: Unable to find network %s."
>                   wok_log.error(msg, net_name)
> -                wok_log.error(error_msg)
>                   wok_log.error("Details: %s", e.message)
> -                sys.exit(1)
> +                raise Exception(error_msg)
>
>               if net.isActive() == 0:
>                   try:
> @@ -79,9 +78,8 @@ class NetworksModel(object):
>                   except libvirt.libvirtError as e:
>                       msg = "Fatal: Unable to activate network %s."
>                       wok_log.error(msg, net_name)
> -                    wok_log.error(error_msg)
>                       wok_log.error("Details: %s", e.message)
> -                    sys.exit(1)
> +                    raise Exception(error_msg)
>
>       def create(self, params):
>           conn = self.conn.get()
> diff --git a/model/storagepools.py b/model/storagepools.py
> index 3f6a734..a2dbaec 100644
> --- a/model/storagepools.py
> +++ b/model/storagepools.py
> @@ -19,7 +19,6 @@
>
>   import libvirt
>   import lxml.etree as ET
> -import sys
>   from lxml.builder import E
>
>   from wok.exception import InvalidOperation, MissingParameter
> @@ -82,21 +81,21 @@ class StoragePoolsModel(object):
>           if config.get('kimchi', {}).get('create_iso_pool', False):
>               pools['ISO'] = {'path': '/var/lib/kimchi/isos'}
>
> -        error_msg = ("Please, check the configuration in %s/template.conf to "
> -                     "ensure it has a valid storage pool." %
> -                     kimchiPaths.sysconf_dir)
> -
>           conn = self.conn.get()
>           for pool_name in pools:
> +            error_msg = ("Storage pool %s does not exist or is not "
> +                         "active. Please, check the configuration in "
> +                         "%s/template.conf to ensure it lists only valid "
> +                         "networks." % (pool_name, kimchiPaths.sysconf_dir))
>               try:
>                   pool = conn.storagePoolLookupByName(pool_name)
>               except libvirt.libvirtError, e:
>                   pool_path = pools[pool_name].get('path')
>                   if pool_path is None:
> -                    msg = "Fatal: Unable to find storage pool %s. " + error_msg
> +                    msg = "Fatal: Unable to find storage pool %s. "
>                       wok_log.error(msg % pool_name)
>                       wok_log.error("Details: %s", e.message)
> -                    sys.exit(1)
> +                    raise Exception(error_msg)
>
>                   # Try to create the pool
>                   pool = E.pool(E.name(pool_name), type='dir')
> @@ -106,10 +105,9 @@ class StoragePoolsModel(object):
>                       pool = conn.storagePoolDefineXML(xml, 0)
>                   except libvirt.libvirtError, e:
>                       msg = "Fatal: Unable to create storage pool %s. "
> -                    msg += error_msg
>                       wok_log.error(msg % pool_name)
>                       wok_log.error("Details: %s", e.message)
> -                    sys.exit(1)
> +                    raise Exception(error_msg)
>
>                   # Build and set autostart value to pool
>                   # Ignore error as the pool was already successfully created
> @@ -127,10 +125,9 @@ class StoragePoolsModel(object):
>                       pool.create(0)
>                   except libvirt.libvirtError, e:
>                       msg = "Fatal: Unable to craete storage pool %s. "
> -                    msg += error_msg
>                       wok_log.error(msg % pool_name)
>                       wok_log.error("Details: %s", e.message)
> -                    sys.exit(1)
> +                    raise Exception(error_msg)
>
>       def get_list(self):
>           try:

-- 

Ramon Nunes Medeiros
Kimchi Developer
Linux Technology Center Brazil
IBM Systems & Technology Group
Phone : +55 19 2132 7878
ramonn at br.ibm.com