[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 16:56:05 UTC 2014


On 03/12/2014 04:34 AM, Royce Lv wrote:
> On 2014年03月12日 12:00, Aline Manera wrote:
>> 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)
> Though we need to check network delete fails because it is referred by 
> template.
> We still need to clear env (network) after this test finished by 
> disconnecting this template with network.

Good point.
I forgot it.
I will update on V2.

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




More information about the Kimchi-devel mailing list