[Kimchi-devel] [PATCH 1/2] Do not allow user disable/delete a network used by VM or template

Aline Manera alinefm at linux.vnet.ibm.com
Wed Mar 12 04:00:20 UTC 2014


From: Aline Manera <alinefm at br.ibm.com>

Add a new parameter to network resource to identify the network is in
use by a template or VM, and do not allow user disable/delete it in this
case.
The 'default' network is always 'in use' as Kimchi uses it when creating a new
template.

Also update API and test cases to reflect this change

Signed-off-by: Aline Manera <alinefm at br.ibm.com>
---
 docs/API.md                    |    2 ++
 src/kimchi/control/networks.py |    1 +
 src/kimchi/i18n.py             |    4 ++--
 src/kimchi/mockmodel.py        |   24 ++++++++++++++++++++++++
 src/kimchi/model/networks.py   |   31 +++++++++++++++++++++++++++----
 tests/test_model.py            |    3 +--
 tests/test_rest.py             |    6 +++---
 7 files changed, 60 insertions(+), 11 deletions(-)

diff --git a/docs/API.md b/docs/API.md
index b3b6c49..3fca87a 100644
--- a/docs/API.md
+++ b/docs/API.md
@@ -469,6 +469,8 @@ A interface represents available interface on host.
     * interface: The name of a network interface on the host.
                  For bridge network, the interface can be a bridge or nic/bonding
                  device. For isolated or NAT network, the interface is ignored.
+    * in_use: True if network is in use by a template or virtual machine;
+              False, otherwise.
 
 ### Resource: Network
 
diff --git a/src/kimchi/control/networks.py b/src/kimchi/control/networks.py
index dd7e08d..95e8523 100644
--- a/src/kimchi/control/networks.py
+++ b/src/kimchi/control/networks.py
@@ -39,6 +39,7 @@ class Network(Resource):
     def data(self):
         return {'name': self.ident,
                 'vms': self.info['vms'],
+                'in_use': self.info['in_use'],
                 'autostart': self.info['autostart'],
                 'connection': self.info['connection'],
                 'interface': self.info['interface'],
diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
index 1ae3889..0af90a0 100644
--- a/src/kimchi/i18n.py
+++ b/src/kimchi/i18n.py
@@ -180,8 +180,8 @@ messages = {
     "KCHNET0014E": _("Network interface must be a string"),
     "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 network %(name)s. There are still some VMs linked to this network."),
-    "KCHNET0018E": _("Unable to deactivate network %(name)s. There are some VMs running linked to this network."),
+    "KCHNET0017E": _("Unable to delete network %(name)s. There are some virtual machines and/or templates linked to this network."),
+    "KCHNET0018E": _("Unable to deactivate network %(name)s. There are some virtual machines and/or templates linked to this network."),
 
     "KCHDR0001E": _("Debug report %(name)s does not exist"),
     "KCHDR0002E": _("Debug report tool not found in system"),
diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py
index b842b6c..b79aead 100644
--- a/src/kimchi/mockmodel.py
+++ b/src/kimchi/mockmodel.py
@@ -594,18 +594,42 @@ class MockModel(object):
                 vms.append(name)
         return vms
 
+    def _network_used_by_template(self, network):
+        for name, tmpl in self._mock_templates.iteritems():
+            if network in tmpl.info['networks']:
+                return True
+        return False
+
+    def _is_network_in_use(self, name):
+        # The network "default" is used for Kimchi proposal and should not be
+        # deactivate or deleted. Otherwise, we will allow user create
+        # inconsistent templates from scratch
+        if name == 'default':
+            return True
+
+        vms = self._get_vms_attach_to_a_network(name)
+        return bool(vms) or self._network_used_by_template(name)
+
     def network_lookup(self, name):
         network = self._get_network(name)
         network.info['vms'] = self._get_vms_attach_to_a_network(name)
+        network.info['in_use'] = self._is_network_in_use(name)
+
         return network.info
 
     def network_activate(self, name):
         self._get_network(name).info['state'] = 'active'
 
     def network_deactivate(self, name):
+        if self._is_network_in_use(name):
+            raise InvalidOperation("KCHNET0018E", {'name': name})
+
         self._get_network(name).info['state'] = 'inactive'
 
     def network_delete(self, name):
+        if self._is_network_in_use(name):
+            raise InvalidOperation("KCHNET0017E", {'name': name})
+
         # firstly, we should check the network actually exists
         network = self._get_network(name)
         del self._mock_networks[network.name]
diff --git a/src/kimchi/model/networks.py b/src/kimchi/model/networks.py
index 0d6ccec..cfea6ba 100644
--- a/src/kimchi/model/networks.py
+++ b/src/kimchi/model/networks.py
@@ -153,6 +153,7 @@ class NetworksModel(object):
 class NetworkModel(object):
     def __init__(self, **kargs):
         self.conn = kargs['conn']
+        self.objstore = kargs['objstore']
 
     def lookup(self, name):
         network = self.get_network(self.conn.get(), name)
@@ -183,9 +184,29 @@ class NetworkModel(object):
                 'subnet': subnet,
                 'dhcp': dhcp,
                 'vms': self._get_vms_attach_to_a_network(name),
+                'in_use': self._is_network_in_use(name),
                 'autostart': network.autostart() == 1,
                 'state':  network.isActive() and "active" or "inactive"}
 
+    def _is_network_in_use(self, name):
+        # The network "default" is used for Kimchi proposal and should not be
+        # deactivate or deleted. Otherwise, we will allow user create
+        # inconsistent templates from scratch
+        if name == 'default':
+            return True
+
+        vms = self._get_vms_attach_to_a_network(name)
+        return bool(vms) or self._network_used_by_template(name)
+
+    def _network_used_by_template(self, network):
+        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
+
     def _get_vms_attach_to_a_network(self, network, filter="all"):
         DOM_STATE_MAP = {'nostate': 0, 'running': 1, 'blocked': 2,
                          'paused': 3, 'shutdown': 4, 'shutoff': 5,
@@ -210,17 +231,19 @@ class NetworkModel(object):
         network.create()
 
     def deactivate(self, name):
-        network = self.get_network(self.conn.get(), name)
-        if self._get_vms_attach_to_a_network(name, "running"):
+        if self._is_network_in_use(name):
             raise InvalidOperation("KCHNET0018E", {'name': name})
+
+        network = self.get_network(self.conn.get(), name)
         network.destroy()
 
     def delete(self, name):
+        if self._is_network_in_use(name):
+            raise InvalidOperation("KCHNET0017E", {'name': name})
+
         network = self.get_network(self.conn.get(), name)
         if network.isActive():
             raise InvalidOperation("KCHNET0005E", {'name': name})
-        if self._get_vms_attach_to_a_network(name):
-            raise InvalidOperation("KCHNET0017E", {'name': name})
 
         self._remove_vlan_tagged_bridge(network)
         network.undefine()
diff --git a/tests/test_model.py b/tests/test_model.py
index a3d8eff..db0f5f5 100644
--- a/tests/test_model.py
+++ b/tests/test_model.py
@@ -486,12 +486,11 @@ class ModelTests(unittest.TestCase):
             inst.templates_create(params)
             rollback.prependDefer(inst.template_delete, 'test')
 
-            inst.network_delete(net_name)
+            self.assertRaises(InvalidOperation, inst.network_delete, net_name)
             shutil.rmtree(path)
 
             info = inst.template_lookup('test')
             self.assertEquals(info['invalid']['cdrom'], [iso])
-            self.assertEquals(info['invalid']['networks'], [net_name])
 
     @unittest.skipUnless(utils.running_as_root(), 'Must be run as root')
     def test_template_clone(self):
diff --git a/tests/test_rest.py b/tests/test_rest.py
index 65d50e0..4a1b6eb 100644
--- a/tests/test_rest.py
+++ b/tests/test_rest.py
@@ -1109,8 +1109,9 @@ class RestTests(unittest.TestCase):
 
         shutil.rmtree(path)
         # Delete the network
-        resp = request(host, port, '/networks/test-network', '{}', 'DELETE')
-        self.assertEquals(204, resp.status)
+        resp = json.loads(request(host, port, '/networks/test-network',
+                                  '{}', 'DELETE').read())
+        self.assertIn("KCHNET0017E", resp["reason"])
 
         # Delete the storagepool
         resp = request(host, port, '/storagepools/test-storagepool',
@@ -1120,7 +1121,6 @@ class RestTests(unittest.TestCase):
         # Verify the template
         res = json.loads(self.request('/templates/test').read())
         self.assertEquals(res['invalid']['cdrom'], [iso])
-        self.assertEquals(res['invalid']['networks'], ['test-network'])
         self.assertEquals(res['invalid']['storagepools'], ['test-storagepool'])
 
         # Delete the template
-- 
1.7.10.4




More information about the Kimchi-devel mailing list