[Kimchi-devel] [PATCH] [Kimchi] s390x specific changes to support storage path and storage pool disk.
Chandra Shekhar Reddy Potula
chandra at linux.vnet.ibm.com
Mon Oct 24 12:56:01 UTC 2016
On 10/23/16 5:06 PM, Archana Singh wrote:
> Hi All,
>
> As template.conf file and its configuration by default is going to be
> same in both s390x and other architecture. So I guess we can have just
> one template.conf. Please see my comment inline.
>
> And lets discuss this further.
>
> Thanks,
> Archana Singh
>
>
> On 10/21/2016 06:15 PM, Lucio Correia wrote:
>> 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?
> This is true, may be it don't even need new section for s390x also.
> As in case of s390x also it need to read storage and network(i.e main)
> section. But as I remember correctly in RFC it was concluded to have
> separated file for s390x due to the fact that on s390x in addition to
> pool there can be path also. However since by default in both case
> s390x and other architecture the file conf setting is going to be
> same. So I think one file should be good for both.
>
> Please provide your input.
Right now on s390x along with path based storage and network interfaces
(macvtap and ovs), storage pools and networks via libvirt are supported.
Hence in my opinion conf file should be similar to other platforms and
use the same file as default conf file. Later user can choose depend
upon choices whichever way he/she want for storage/networks.
>
>>
>>
>>> 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
>>>
>>
>>
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>
More information about the Kimchi-devel
mailing list