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

于 2014/3/12 12:00, Aline Manera 写道: > From: Aline Manera <alinefm(a)br.ibm.com&gt; > > 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(a)br.ibm.com&gt; > --- > 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 Nits. For a uniform name style as the other bool function, "_is_network_usedby_template()".

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