On 08/19/2016 02:12 PM, archus@linux.vnet.ibm.com wrote:
From: Archana Singh <archus@linux.vnet.ibm.com>
Below are the changes done for s390x architecture to only add default networks to template if template.conf has network configuration uncommented otherwise do not add any network by default.
1) Only on s390x, osinfo._get_tmpl_defaults() does not add default network in case template.conf has commented out network configuration. 2) Only on s390x, NetworksModel._check_default_networks() does not throw key error if networks not found in tmpl_defaults. 3) Only on s390x, VMTemplate.validate_integrity() does not throw key error if networks not found in the template. 4) Only on s390x, LibvirtVMTemplate._network_validate() does not throw key error on s390x if networks not found in tmpl_default, as on s390x networks is optional. 5) Only on s390x, VMTemplate._get_network_xml() does return empty string and does not throw key error if networks not found in template object. 6) Only on s390x, control/templates.py gets networks as [] if not found self.info. 7) Added method is_running_on_s390x in kimchi's utils.
Signed-off-by: Archana Singh <archus@linux.vnet.ibm.com> --- control/templates.py | 2 +- model/networks.py | 7 ++++++- model/templates.py | 6 +++++- osinfo.py | 14 +++++++++++++- utils.py | 11 +++++++++++ vmtemplate.py | 17 ++++++++++++++--- 6 files changed, 50 insertions(+), 7 deletions(-)
diff --git a/control/templates.py b/control/templates.py index bb2e068..2dd8601 100644 --- a/control/templates.py +++ b/control/templates.py @@ -67,7 +67,7 @@ class Template(Resource): 'memory': self.info['memory'], 'cdrom': self.info.get('cdrom', None), 'disks': self.info['disks'], - 'networks': self.info['networks'], + 'networks': self.info.get('networks', []), 'folder': self.info.get('folder', []), 'graphics': self.info['graphics'], 'cpu_info': self.info.get('cpu_info') diff --git a/model/networks.py b/model/networks.py index 35431d4..603a31c 100644 --- a/model/networks.py +++ b/model/networks.py @@ -35,6 +35,7 @@ from wok.plugins.kimchi import network as knetwork from wok.plugins.kimchi.config import kimchiPaths from wok.plugins.kimchi.model.config import CapabilitiesModel from wok.plugins.kimchi.osinfo import defaults as tmpl_defaults +from wok.plugins.kimchi.utils import is_running_on_s390x from wok.plugins.kimchi.xmlutils.interface import get_iface_xml from wok.plugins.kimchi.xmlutils.network import create_linux_bridge_xml from wok.plugins.kimchi.xmlutils.network import create_vlan_tagged_bridge_xml @@ -55,7 +56,11 @@ class NetworksModel(object): self.caps = CapabilitiesModel(**kargs)
def _check_default_networks(self): - networks = list(set(tmpl_defaults['networks']))
+ if is_running_on_s390x(): + networks = list(set(tmpl_defaults.get('networks', []))) + else: + networks = list(set(tmpl_defaults['networks'])) +
I don't think we need to distinguish s390x here. We can use networks = list(set(tmpl_defaults.get('networks', []))) In both cases and let tmpl_defaults proper set the value for network according to host arch.
conn = self.conn.get()
for net_name in networks: diff --git a/model/templates.py b/model/templates.py index a299c85..387e4f1 100644 --- a/model/templates.py +++ b/model/templates.py @@ -35,6 +35,7 @@ from wok.plugins.kimchi.config import get_kimchi_version from wok.plugins.kimchi.kvmusertests import UserTests from wok.plugins.kimchi.model.cpuinfo import CPUInfoModel from wok.plugins.kimchi.utils import is_libvirtd_up, pool_name_from_uri +from wok.plugins.kimchi.utils import is_running_on_s390x from wok.plugins.kimchi.vmtemplate import VMTemplate
ISO_TYPE = "ISO 9660 CD-ROM" @@ -358,7 +359,10 @@ class LibvirtVMTemplate(VMTemplate): return sorted(map(lambda x: x.decode('utf-8'), names))
def _network_validate(self):
- names = self.info['networks'] + if is_running_on_s390x(): + names = self.info.get('networks', []) + else: + names = self.info['networks']
The same I commented before.
for name in names: try: conn = self.conn.get() diff --git a/osinfo.py b/osinfo.py index 2c59312..af37c72 100644 --- a/osinfo.py +++ b/osinfo.py @@ -138,10 +138,22 @@ def _get_tmpl_defaults(): 'pool': '/plugins/kimchi/storagepools/default'}}, 'processor': {'vcpus': '1', 'maxvcpus': 1}, 'graphics': {'type': 'spice', 'listen': '127.0.0.1'}} + + The default values on s390x architecture: + + {'memory': {'current': 1024, 'maxmemory': 1024}, + 'storage': { 'disk.0': {'format': 'qcow2', 'size': '10', + 'pool': '/plugins/kimchi/storagepools/default'}}, + 'processor': {'vcpus': '1', 'maxvcpus': 1}, + 'graphics': {'type': 'spice', 'listen': '127.0.0.1'}} """ # Create dict with default values tmpl_defaults = defaultdict(dict) - tmpl_defaults['main']['networks'] = ['default'] + + host_arch = _get_arch() + if host_arch not in ['s390x']: + tmpl_defaults['main']['networks'] = ['default'] + tmpl_defaults['memory'] = {'current': _get_default_template_mem(), 'maxmemory': _get_default_template_mem()} tmpl_defaults['storage']['disk.0'] = {'size': 10, 'format': 'qcow2', diff --git a/utils.py b/utils.py index 26d3cf6..b88740a 100644 --- a/utils.py +++ b/utils.py @@ -20,6 +20,7 @@
import contextlib import json +import os import platform import re import sqlite3 @@ -272,3 +273,13 @@ def is_libvirtd_up():
output, error, rc = run_command(cmd, silent=True) return True if output == 'active\n' else False + + +def is_running_on_s390x(): + """ + Checks if running on s390x. + """ + if os.uname()[4] == 's390x': + return True
+ else: + return False
Tip: you don't need the 'else' statement when you have a return in the if statement. Eliminating the else statement reduce 1 indentation level. In that case, it does not affect too much, but in bigger blocks the indentation level may make a difference.
diff --git a/vmtemplate.py b/vmtemplate.py index 7ac0541..f5405fa 100644 --- a/vmtemplate.py +++ b/vmtemplate.py @@ -288,7 +288,13 @@ class VMTemplate(object): networks = "" params = {'type': 'network', 'model': self.info['nic_model']} - for nw in self.info['networks']: +
+ if self.info.get('arch') == 's390x': + info_networks = self.info.get('networks', []) + else: + info_networks = self.info['networks'] +
The same I commented before. As tmpl_defaults has the right data according to host arch, you can user + info_networks = self.info.get('networks', []) For every case.
+ for nw in info_networks: params['network'] = nw networks += get_iface_xml(params, self.info['arch'], self.info['os_distro'], @@ -353,7 +359,7 @@ class VMTemplate(object): graphics.update(kwargs.get('graphics', {})) # Graphics is not supported on s390x, this check will # not add graphics tag in domain xml. - if params['arch'] not in ['s390x']: + if params.get('arch') != 's390x': params['graphics'] = get_graphics_xml(graphics)
libvirt_stream_protocols = kwargs.get('libvirt_stream_protocols', []) @@ -487,7 +493,12 @@ class VMTemplate(object): def validate_integrity(self): invalid = {} # validate networks integrity - invalid_networks = list(set(self.info['networks']) -
+ if self.info.get('arch') == 's390x': + networks = self.info.get('networks', []) + else: + networks = self.info['networks'] +
Same here.
+ invalid_networks = list(set(networks) - set(self._get_all_networks_name())) if invalid_networks: invalid['networks'] = invalid_networks