[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