[Kimchi-devel] [PATCH] [Kimchi 6/6] Improve logic to identify if a network is in use or not

[Aline Manera]

The function get_vms_attach_to_a_network() to identify which virtual
machines are using a given network was being called multiple times
without need. So improve the logic.

Signed-off-by: Aline Manera <alinefm at linux.vnet.ibm.com>
---
 i18n.py           |  4 ++--
 model/networks.py | 42 ++++++++++++++++++++++++++----------------
 2 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/i18n.py b/i18n.py
index f505693..4460ce5 100644
--- a/i18n.py
+++ b/i18n.py
@@ -300,8 +300,8 @@ messages = {
     "KCHNET0014E": _("Network interfaces must be an array."),
     "KCHNET0015E": _("Network VLAN ID must be an integer between 1 and 4094"),
     "KCHNET0016E": _("Specify name and type to create a Network"),
-    "KCHNET0017E": _("Unable to delete or update network %(name)s. There are some virtual machines %(vms)s and/or templates linked to this network."),
-    "KCHNET0018E": _("Unable to deactivate network %(name)s. There are some virtual machines %(vms)s and/or templates linked to this network."),
+    "KCHNET0017E": _("Unable to delete or update network %(name)s as it is linked to some virtual machines (%(vms)s) and/or templates (%(tmpls)s)."),
+    "KCHNET0018E": _("Unable to deactivate network %(name)s as it is linked to are some virtual machines (%(vms)s) and/or templates (%(tmpls)s)."),
     "KCHNET0019E": _("Bridge device %(name)s can not be the trunk device of a VLAN."),
     "KCHNET0020E": _("Failed to activate interface %(iface)s: %(err)s."),
     "KCHNET0021E": _("Failed to activate interface %(iface)s. Please check the physical link status."),
diff --git a/model/networks.py b/model/networks.py
index be4eec2..eb1590e 100644
--- a/model/networks.py
+++ b/model/networks.py
@@ -1,7 +1,7 @@
 #
 # Project Kimchi
 #
-# Copyright IBM Corp, 2015-2016
+# Copyright IBM Corp, 2015-2017
 #
 # This library is free software; you can redistribute it and/or
 # modify it under the terms of the GNU Lesser General Public
@@ -383,12 +383,14 @@ class NetworkModel(object):
         else:
             interfaces = [interface]
 
+        network_in_use, used_by_vms, _ = self._is_network_in_use(name)
+
         return {'connection': connection,
                 'interfaces': interfaces,
                 'subnet': subnet,
                 'dhcp': dhcp,
-                'vms': self._get_vms_attach_to_a_network(name),
-                'in_use': self._is_network_in_use(name),
+                'vms': used_by_vms,
+                'in_use': network_in_use,
                 'autostart': network.autostart() == 1,
                 'state':  network.isActive() and "active" or "inactive",
                 'persistent': True if network.isPersistent() else False}
@@ -397,20 +399,24 @@ class NetworkModel(object):
         # All the networks listed as default in template.conf file should not
         # be deactivate or deleted. Otherwise, we will allow user create
         # inconsistent templates from scratch
+        vms = self._get_vms_attach_to_a_network(name)
+        tmpls = self._is_network_used_by_template(name)
+
         if name in tmpl_defaults['networks']:
-            return True
+            return (True, vms, tmpls)
 
-        vms = self._get_vms_attach_to_a_network(name)
-        return bool(vms) or self._is_network_used_by_template(name)
+        return (bool(vms) or bool(tmpls), vms, tmpls)
 
     def _is_network_used_by_template(self, network):
+        tmpl_list = []
+
         with self.objstore as session:
             templates = session.get_list('template')
             for tmpl in templates:
                 tmpl_net = session.get('template', tmpl)['networks']
                 if network in tmpl_net:
-                    return True
-            return False
+                    tmpl_list.append(tmpl)
+            return tmpl_list
 
     def _get_vms_attach_to_a_network(self, network, filter="all"):
         DOM_STATE_MAP = {'nostate': 0, 'running': 1, 'blocked': 2,
@@ -440,21 +446,25 @@ class NetworkModel(object):
                                                   'err': e.message})
 
     def deactivate(self, name):
-        if self._is_network_in_use(name):
-            vms = self._get_vms_attach_to_a_network(name)
-            vms.sort()
+        in_use, used_by_vms, used_by_tmpls = self._is_network_in_use(name)
+        vms = 'N/A' if len(used_by_vms) == 0 else ', '.join(used_by_vms)
+        tmpls = 'N/A' if len(used_by_tmpls) == 0 else ', '.join(used_by_tmpls)
+        if in_use:
             raise InvalidOperation("KCHNET0018E", {'name': name,
-                                                   'vms': ', '.join(vms)})
+                                                   'vms': vms,
+                                                   'tmpls': tmpls})
 
         network = self.get_network(self.conn.get(), name)
         network.destroy()
 
     def delete(self, name):
-        if self._is_network_in_use(name):
-            vms = self._get_vms_attach_to_a_network(name)
-            vms.sort()
+        in_use, used_by_vms, used_by_tmpls = self._is_network_in_use(name)
+        vms = 'N/A' if len(used_by_vms) == 0 else ', '.join(used_by_vms)
+        tmpls = 'N/A' if len(used_by_tmpls) == 0 else ', '.join(used_by_tmpls)
+        if in_use:
             raise InvalidOperation("KCHNET0017E", {'name': name,
-                                                   'vms': ', '.join(vms)})
+                                                   'vms': vms,
+                                                   'tmpls': tmpls})
 
         network = self.get_network(self.conn.get(), name)
         if network.isActive():
-- 
2.9.3