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@linux.vnet.ibm.com wrote:

From: Aline Manera <alinefm@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@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'}))