[Kimchi-devel] [PATCH] [Kimchi] s390x specific changes to support storage path and storage pool disk.
Lucio Correia
luciojhc at linux.vnet.ibm.com
Fri Oct 21 12:45:23 UTC 2016
On 21/10/2016 03:18, archus at linux.vnet.ibm.com wrote:
> From: Archana Singh <archus at linux.vnet.ibm.com>
>
> 1) Added s390x specific error message for storage pool and network.
> 2) As on s390x, default is storage path, but if pool explicitly specified in
> template_s390x.conf it takes preference. So added code to ensure that
> if storage pool explicitly specified in file conf then check for default
> storage pool gets called.
> 3) Code changes in tmpl default creation which ensure that on s390x, default
> disk is path but if conf file has explicitly pool specified then defult disk
> is the sepecified pool. And also ensures that disk is either pool or path
> and not both.
> 4) Code changes in vmtemplate, to ensure that on s390x either a path disk
> or a pool disk can be added to template. And if disk to add does not have
> both path nad pool then tmpl default storage is used.
> 5) Added template conf file specific to s390x.
>
> Signed-off-by: Archana Singh <archus at linux.vnet.ibm.com>
> ---
> IBM-license-blacklist | 1 +
> model/networks.py | 7 +++++++
> model/storagepools.py | 13 +++++++++++--
> osinfo.py | 36 ++++++++++++++++++++++++++++++++--
> template_s390x.conf | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++
> vmtemplate.py | 33 +++++++++++++++++++++++---------
> 6 files changed, 130 insertions(+), 13 deletions(-)
> create mode 100644 template_s390x.conf
>
> diff --git a/IBM-license-blacklist b/IBM-license-blacklist
> index 78fd2cb..a0d1407 100644
> --- a/IBM-license-blacklist
> +++ b/IBM-license-blacklist
> @@ -27,6 +27,7 @@ po/Makevars
> po/POTFILES.in
> po/kimchi.pot
> template.conf
> +template_s390x.conf
Why not add a section s390x to the existing template.conf? Is a new file
really necessary?
> ui/config/tab-ext.xml
> ui/images/.*.svg
> ui/pages/help/dita-help.xsl
> diff --git a/model/networks.py b/model/networks.py
> index be4eec2..57ff487 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_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
> @@ -64,6 +65,12 @@ class NetworksModel(object):
> "active. Please, check the configuration in "
> "%s/template.conf to ensure it lists only valid "
> "networks." % (net_name, kimchiPaths.sysconf_dir))
> + if is_s390x():
> + error_msg = ("Network %s does not exist or is not "
> + "active. Please, check the "
> + "configuration in %s/template_s390x.conf "
> + "to ensure it lists only valid "
> + "networks." % (net_name, kimchiPaths.sysconf_dir))
>
> try:
> net = conn.networkLookupByName(net_name)
> diff --git a/model/storagepools.py b/model/storagepools.py
> index 5942b31..a8a2356 100644
> --- a/model/storagepools.py
> +++ b/model/storagepools.py
> @@ -73,7 +73,9 @@ class StoragePoolsModel(object):
> def _check_default_pools(self):
> pools = {}
>
> - if is_s390x():
> + # Don't create default pool if it's not
> + # explicitly specified in template_s390x.conf
> + if is_s390x() and 'pool' not in tmpl_defaults['disks'][0]:
> return
>
> default_pool = tmpl_defaults['disks'][0]['pool']['name']
> @@ -91,7 +93,14 @@ class StoragePoolsModel(object):
> error_msg = ("Storage pool %s does not exist or is not "
> "active. Please, check the configuration in "
> "%s/template.conf to ensure it lists only valid "
> - "networks." % (pool_name, kimchiPaths.sysconf_dir))
> + "storage." % (pool_name, kimchiPaths.sysconf_dir))
> + if is_s390x:
> + error_msg = ("Network %s does not exist or is not "
> + "active. Please, check the "
> + "configuration in %s/template_s390x.conf "
> + "to ensure it lists only valid "
> + "storage." % (pool_name, kimchiPaths.sysconf_dir))
> +
> try:
> pool = conn.storagePoolLookupByName(pool_name)
> except libvirt.libvirtError, e:
> diff --git a/osinfo.py b/osinfo.py
> index c51d6e0..4f0e1fa 100644
> --- a/osinfo.py
> +++ b/osinfo.py
> @@ -27,9 +27,9 @@ from configobj import ConfigObj
> from distutils.version import LooseVersion
>
> from wok.config import PluginPaths
> +from wok.exception import InvalidParameter
> from wok.plugins.kimchi.config import kimchiPaths
>
> -
> SUPPORTED_ARCHS = {'x86': ('i386', 'i686', 'x86_64'),
> 'power': ('ppc', 'ppc64'),
> 'ppc64le': ('ppc64le'),
> @@ -129,7 +129,8 @@ def _get_default_template_mem():
> def _get_tmpl_defaults():
> """
> ConfigObj returns a dict like below when no changes were made in the
> - template configuration file (template.conf)
> + template configuration file (template.conf and in case of s390x its
> + template_s390x.conf)
>
> {'main': {}, 'memory': {}, 'storage': {'disk.0': {}}, 'processor': {},
> 'graphics': {}}
> @@ -184,6 +185,37 @@ def _get_tmpl_defaults():
> config_file = os.path.join(kimchiPaths.sysconf_dir, 'template.conf')
> config = ConfigObj(config_file)
>
> + # File configuration takes preference.
> + # In s390x, file configuration can have storage pool or path.
> + # Default configuration for s390x is storage path.
> + # In case file conf has storage pool then storage pool takes preference.
> + # When conf file has explicitly storage pool: "defaults" should
> + # have storage pool and default configured path should be removed,
> + # as either storage can be path or pool, cannot be both.
> + # When conf file does not explicity storage pool or have explicitly
> + # storage path: "default" should have storage path only and cannot
> + # have default pool.
> + #
> + # Check file conf has storage configured.
> + if is_on_s390x and config.get('storage').get('disk.0'):
> + # remove storage from default_config as file configuration takes
> + # preference.
> + default_config.pop('storage')
> +
> + # Get storage configuration present in conf file
> + config_pool = config.get('storage').get('disk.0').get('pool')
> + config_path = config.get('storage').get('disk.0').get('path')
> +
> + # If storage configured in conf file then it should have either
> + # pool or path.
> + if not config_pool and not config_path:
> + raise InvalidParameter('KCHTMPL0040E')
> +
> + # On s390x if config file has both path and pool uncommented
> + # then path should take preference.
> + if config_pool and config_path:
> + config.get('storage').get('disk.0').pop('pool')
> +
> # Merge default configuration with file configuration
> default_config.merge(config)
>
> diff --git a/template_s390x.conf b/template_s390x.conf
> new file mode 100644
> index 0000000..c4598f1
> --- /dev/null
> +++ b/template_s390x.conf
> @@ -0,0 +1,53 @@
> +#
> +# Configuration file for Kimchi Templates
> +#
> +
> +[main]
> +# List of networks separated by comma
> +# Represents the virtual network interfaces to be assigned to guest
> +#networks = default,
> +
> +[memory]
> +# Memory in MB
> +# current = 1024
> +
> +# Maximum value of memory to be assigned to guest in MB
> +# maxmemory = 1024
> +
> +[storage]
> +
> +# Specify multiple [[disk.X]] sub-sections to add multiples disks to guest
> +# Each disk files will be created in respective storage pool set
> +[[disk.0]]
> +# Disk size in GB
> +#size = 10
> +
> +# Disk format
> +#format = qcow2
> +
> +# Storage pool used to handle the guest disk
> +#pool = default
> +
> +[graphics]
> +# Graphics type
> +# Valid options: vnc | spice
> +#type = vnc
> +
> +# The network which the vnc/spice server listens on
> +#listen = 127.0.0.1
> +
> +[processor]
> +# Number of vcpus
> +# When specifying CPU topology, make sure maxvcpus value is equal to the
> +# product of sockets, cores, and threads.
> +#vcpus = 1
> +#maxvcpus = 1
> +
> +# Number of sockets (not set by default)
> +#sockets =
> +
> +# Number of cores per socket (not set by default)
> +#cores =
> +
> +# Number of threads per core (not set by default)
> +#threads =
> diff --git a/vmtemplate.py b/vmtemplate.py
> index c3390fe..b249873 100644
> --- a/vmtemplate.py
> +++ b/vmtemplate.py
> @@ -106,14 +106,25 @@ class VMTemplate(object):
>
> for index, disk in enumerate(disks):
> disk_info = dict(default_disk)
> - # on s390x/s390 either pool or path should be present in
> - # default disk.
> - if is_s390x() and 'pool' not in default_disk and \
> - 'path' not in default_disk:
> - raise InvalidParameter('KCHTMPL0040E')
>
> - # On s390x/s390 pool is optional attribute for disk.
> - pool = disk.get('pool', default_disk.get('pool'))
> + if is_s390x():
> + # Default disk should have either pool or path.
> + if 'pool' not in default_disk and 'path' not in default_disk:
> + raise InvalidParameter('KCHTMPL0040E')
> +
> + # Each disk should have either pool or path.
> + # if not then use "default_disk" configuration.
> + pool = disk.get('pool')
> + path = disk.get('path')
> + if not path and not pool:
> + # If default disk is path then set disk with default path
> + if default_disk.get('path'):
> + path = default_disk.get('path')
> + # If default disk is pool then set disk with default pool
> + if default_disk.get('pool'):
> + pool = default_disk.get('pool')
> + else:
> + pool = disk.get('pool', default_disk.get('pool'))
>
> if pool:
> pool_type = self._get_storage_type(pool['name'])
> @@ -148,8 +159,12 @@ class VMTemplate(object):
> disk_info['index'] = disk_info.get('index', index)
> self.info['disks'][index] = disk_info
> elif is_s390x():
> - # For now support 'path' only on s390x
> - path = disk.get('path', default_disk.get('path'))
> + # This check is required where 'path' disk
> + # has to be added and hence default pool
> + # has to be removed during template update.
> + if 'pool' in disk_info:
> + del disk_info['pool']
> +
> disk_info.update(disk)
> keys = sorted(disk_info.keys())
> if ((keys != sorted(basic_path_disk)) and
>
--
Lucio Correia
Software Engineer
IBM LTC Brazil
More information about the Kimchi-devel
mailing list