[Kimchi-devel] [PATCH V2] [Kimchi] Only on s390x add default networks to template if template.conf has default network configuration uncommented.
Aline Manera
alinefm at linux.vnet.ibm.com
Mon Aug 29 18:47:02 UTC 2016
On 08/19/2016 02:12 PM, archus at linux.vnet.ibm.com wrote:
> From: Archana Singh <archus at 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 at 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
More information about the Kimchi-devel
mailing list