[Kimchi-devel] [PATCH] Do not auto create default network
Royce Lv
lvroyce at linux.vnet.ibm.com
Mon Jul 28 09:23:22 UTC 2014
For me the initial intention of default setup is to let first time kvm
user to start a vm with network access very easily without understanding
of virtualizaiton.
But at this point we want to address the problem that advanced users
want their customized network and storage,
we can stop bringing up default network and storage on default if
configed to do so.
So I would like we follow the bug owner's advice to add a
"auto_default_storage = True" tag in configuration.
Or we can expose an API of /host to disable default network/storage.
Another simple solution is we just check the presence without bringing
the default storage/network up.
If users turn it off deliberately, we will leave it off.
What's your opinion?
On 2014年07月26日 02:28, alinefm at linux.vnet.ibm.com wrote:
> From: Aline Manera <alinefm at linux.vnet.ibm.com>
>
> Kimchi does not depends on default network as a template and a guest can
> be created without any network interface.
> So do not auto create the default network when starting up Kimchi and
> let users create and configure their networks and templates as they
> want.
> It is part of issue #392
>
> Signed-off-by: Aline Manera <alinefm at linux.vnet.ibm.com>
> ---
> src/kimchi/mockmodel.py | 8 +-------
> src/kimchi/model/networks.py | 44 --------------------------------------------
> src/kimchi/osinfo.py | 2 +-
> tests/test_model.py | 19 +++++++------------
> tests/test_osinfo.py | 2 +-
> tests/test_rest.py | 19 +++++--------------
> tests/test_vmtemplate.py | 2 +-
> 7 files changed, 16 insertions(+), 80 deletions(-)
>
> diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py
> index 1584471..2a74044 100644
> --- a/src/kimchi/mockmodel.py
> +++ b/src/kimchi/mockmodel.py
> @@ -81,7 +81,7 @@ def reset(self):
> self._mock_screenshots = {}
> self._mock_templates = {}
> self._mock_storagepools = {'default': MockStoragePool('default')}
> - self._mock_networks = {'default': MockNetwork('default')}
> + self._mock_networks = {}
> self._mock_interfaces = self.dummy_interfaces()
> self._mock_swupdate = MockSoftwareUpdate()
> self.next_taskid = 1
> @@ -659,12 +659,6 @@ def _is_network_used_by_template(self, network):
> 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._is_network_used_by_template(name)
>
> diff --git a/src/kimchi/model/networks.py b/src/kimchi/model/networks.py
> index bc93da5..45659ea 100644
> --- a/src/kimchi/model/networks.py
> +++ b/src/kimchi/model/networks.py
> @@ -41,44 +41,6 @@
> class NetworksModel(object):
> def __init__(self, **kargs):
> self.conn = kargs['conn']
> - if 'qemu:///' in self.conn.get().getURI():
> - self._default_network_check()
> -
> - def _default_network_check(self):
> - def create_default_network():
> - try:
> - subnet = self._get_available_address(knetwork.DefaultNetsPool)
> - params = {"name": "default", "connection": "nat",
> - "subnet": subnet}
> - self.create(params)
> - return conn.networkLookupByName("default")
> - except Exception as e:
> - kimchi_log.error("Fatal: Cannot create default network "
> - "because of %s, exit kimchid", e.message)
> - sys.exit(1)
> -
> - conn = self.conn.get()
> - try:
> - net = conn.networkLookupByName("default")
> - except libvirt.libvirtError:
> - net = create_default_network()
> -
> - if net.isActive() == 0:
> - try:
> - net.create()
> - except libvirt.libvirtError as e:
> - # FIXME we can not distinguish this error from other internal
> - # error by error code.
> - if ("network is already in use by interface"
> - in e.message.lower()):
> - # libvirt do not support update IP element, so delete the
> - # the network and create new one.
> - net.undefine()
> - create_default_network()
> - else:
> - kimchi_log.error("Fatal: Cannot activate default network "
> - "because of %s, exit kimchid", e.message)
> - sys.exit(1)
>
> def create(self, params):
> conn = self.conn.get()
> @@ -267,12 +229,6 @@ def lookup(self, name):
> 'persistent': True if network.isPersistent() else 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._is_network_used_by_template(name)
>
> diff --git a/src/kimchi/osinfo.py b/src/kimchi/osinfo.py
> index 39c9163..cec2c50 100644
> --- a/src/kimchi/osinfo.py
> +++ b/src/kimchi/osinfo.py
> @@ -91,7 +91,7 @@
> },
> }
>
> -defaults = {'networks': ['default'],
> +defaults = {'networks': [],
> 'storagepool': '/storagepools/default',
> 'domain': 'kvm', 'arch': os.uname()[4],
> 'graphics': {'type': 'vnc', 'listen': '0.0.0.0'}}
> diff --git a/tests/test_model.py b/tests/test_model.py
> index da9dae5..e717872 100644
> --- a/tests/test_model.py
> +++ b/tests/test_model.py
> @@ -154,12 +154,7 @@ def test_vm_ifaces(self):
> rollback.prependDefer(inst.network_deactivate, net_name)
>
> ifaces = inst.vmifaces_get_list('kimchi-ifaces')
> - self.assertEquals(1, len(ifaces))
> -
> - iface = inst.vmiface_lookup('kimchi-ifaces', ifaces[0])
> - self.assertEquals(17, len(iface['mac']))
> - self.assertEquals("default", iface['network'])
> - self.assertIn("model", iface)
> + self.assertEquals(0, len(ifaces))
>
> # attach network interface to vm
> iface_args = {"type": "network",
> @@ -176,11 +171,11 @@ def test_vm_ifaces(self):
> self.assertEquals("virtio", iface["model"])
>
> # update vm interface
> - iface_args = {"network": "default",
> + iface_args = {"network": "test-network",
> "model": "e1000"}
> inst.vmiface_update('kimchi-ifaces', mac, iface_args)
> iface = inst.vmiface_lookup('kimchi-ifaces', mac)
> - self.assertEquals("default", iface['network'])
> + self.assertEquals("test-network", iface['network'])
> self.assertEquals("e1000", iface["model"])
>
> @unittest.skipUnless(utils.running_as_root(), 'Must be run as root')
> @@ -565,14 +560,14 @@ def test_template_create(self):
> info = inst.template_lookup('test')
> for key in params.keys():
> self.assertEquals(params[key], info[key])
> - self.assertEquals("default", info["networks"][0])
> + self.assertEquals(0, len(info["networks"]))
>
> # create template with non-existent network
> params['name'] = 'new-test'
> params['networks'] = ["no-exist"]
> self.assertRaises(InvalidParameter, inst.templates_create, params)
>
> - params['networks'] = ['default', 'test-network']
> + params['networks'] = ['test-network']
> inst.templates_create(params)
> rollback.prependDefer(inst.template_delete, params['name'])
> info = inst.template_lookup(params['name'])
> @@ -667,10 +662,10 @@ def test_template_update(self):
> info = inst.template_lookup('new-test')
> for key in params.keys():
> self.assertEquals(params[key], info[key])
> - self.assertEquals("default", info["networks"][0])
> + self.assertEquals(0, len(info["networks"]))
>
> params = {'name': 'new-test', 'memory': 1024, 'cpus': 1,
> - 'networks': ['default', 'test-network']}
> + 'networks': ['test-network']}
> inst.template_update('new-test', params)
> info = inst.template_lookup('new-test')
> for key in params.keys():
> diff --git a/tests/test_osinfo.py b/tests/test_osinfo.py
> index 104e7b8..7524f84 100644
> --- a/tests/test_osinfo.py
> +++ b/tests/test_osinfo.py
> @@ -28,7 +28,7 @@ def test_default_lookup(self):
> entry = lookup(None, None)
> self.assertEquals('unknown', entry['os_distro'])
> self.assertEquals('unknown', entry['os_version'])
> - self.assertEquals(['default'], entry['networks'])
> + self.assertEquals([], entry['networks'])
>
> def test_fedora_lookup(self):
> cd = ('http://fedora.mirrors.tds.net/pub/fedora/releases/17/Live/'
> diff --git a/tests/test_rest.py b/tests/test_rest.py
> index 3ea1927..306064c 100644
> --- a/tests/test_rest.py
> +++ b/tests/test_rest.py
> @@ -603,14 +603,7 @@ def test_vm_iface(self):
> '/networks/test-network', '{}', 'DELETE')
>
> ifaces = json.loads(self.request('/vms/test-vm/ifaces').read())
> - self.assertEquals(1, len(ifaces))
> -
> - for iface in ifaces:
> - res = json.loads(self.request('/vms/test-vm/ifaces/%s' %
> - iface['mac']).read())
> - self.assertEquals('default', res['network'])
> - self.assertEquals(17, len(res['mac']))
> - self.assertEquals('virtio', res['model'])
> + self.assertEquals(0, len(ifaces))
>
> # attach network interface to vm
> req = json.dumps({"type": "network",
> @@ -794,7 +787,7 @@ def test_template_customise_network(self):
> '/templates/test', '{}', 'DELETE')
> tmpl_res = json.loads(resp.read())
> self.assertTrue(type(tmpl_res['networks']) is list)
> - self.assertEquals("default", tmpl_res['networks'][0])
> + self.assertEquals(0, len(tmpl_res['networks']))
>
> tmpl['name'] = "failed_tmpl"
> # Create a Template with non-array network fails with 400
> @@ -832,7 +825,7 @@ def test_template_customise_network(self):
> self.assertEquals(400, resp.status)
>
> # Update a Template with existent network, successful
> - tmpl['networks'] = ["default", "test-network"]
> + tmpl['networks'] = ["test-network"]
> req = json.dumps(tmpl)
> resp = self.request('/templates/test', req, 'PUT')
> self.assertEquals(200, resp.status)
> @@ -1407,9 +1400,7 @@ def test_interfaces(self):
>
> def test_get_networks(self):
> networks = json.loads(request(host, ssl_port, '/networks').read())
> - self.assertEquals(1, len(networks))
> - self.assertEquals('default', networks[0]['name'])
> - self.assertEquals([], networks[0]['vms'])
> + self.assertEquals(0, len(networks))
>
> # Now add a couple of Networks to the mock model
> for i in xrange(5):
> @@ -1424,7 +1415,7 @@ def test_get_networks(self):
> self.assertEquals([], network["vms"])
>
> networks = json.loads(request(host, ssl_port, '/networks').read())
> - self.assertEquals(6, len(networks))
> + self.assertEquals(5, len(networks))
>
> network = json.loads(request(host, ssl_port,
> '/networks/network-1').read())
> diff --git a/tests/test_vmtemplate.py b/tests/test_vmtemplate.py
> index 821ca24..8dceced 100644
> --- a/tests/test_vmtemplate.py
> +++ b/tests/test_vmtemplate.py
> @@ -29,7 +29,7 @@ class VMTemplateTests(unittest.TestCase):
> def test_minimal_construct(self):
> fields = (('name', 'test'), ('os_distro', 'unknown'),
> ('os_version', 'unknown'), ('cpus', 1),
> - ('memory', 1024), ('cdrom', ''), ('networks', ['default']),
> + ('memory', 1024), ('cdrom', ''), ('networks', []),
> ('disk_bus', 'ide'), ('nic_model', 'e1000'),
> ('graphics', {'type': 'vnc', 'listen': '0.0.0.0'}))
>
More information about the Kimchi-devel
mailing list