[Kimchi-devel] [PATCH V3] Issue #992 : Create template on s390x without libvirt storage

Aline Manera alinefm at linux.vnet.ibm.com
Tue Sep 6 13:00:31 UTC 2016



On 09/01/2016 09:10 AM, harshalp at linux.vnet.ibm.com wrote:
> From: Harshal Patil <harshalp at linux.vnet.ibm.com>
>
> V1:
>   This patch adds support for creating templates on s390x arch
>   without using libvirt related storage calls
>
> V2:
>   Review comments
>
> V3:
>   Review comments and refactoring
>
> Signed-off-by: Harshal Patil <harshalp at linux.vnet.ibm.com>
> ---
>   docs/API.md             |   2 +
>   i18n.py                 |   3 ++
>   model/storagepools.py   |  13 ++++--
>   model/storagevolumes.py |   1 -
>   model/templates.py      |  27 +++++++++---
>   model/vms.py            |   9 +++-
>   osinfo.py               |  35 +++++++++++++--
>   utils.py                |  34 +++++++++++++++
>   vmtemplate.py           | 111 +++++++++++++++++++++++++++++++-----------------
>   9 files changed, 179 insertions(+), 56 deletions(-)
>
> diff --git a/docs/API.md b/docs/API.md
> index 7bd677f..d8d191a 100644
> --- a/docs/API.md
> +++ b/docs/API.md
> @@ -427,6 +427,7 @@ A interface represents available network interface on VM.
>             over current will be used exclusively for memory hotplug
>       * cdrom: A volume name or URI to an ISO image
>       * storagepool: URI of the storagepool where template allocates vm storage.
> +    * path : Storage path to store virtual disks without libvirt
>       * networks *(optional)*: list of networks will be assigned to the new VM.
>       * disks: An array of requested disks with the following optional fields
>         (either *size* or *volume* must be specified):
> @@ -481,6 +482,7 @@ A interface represents available network interface on VM.
>           * format: Format of the image. Valid formats: qcow, qcow2, qed, raw, vmdk, vpc.
>           * pool: Storage pool information
>               * name: URI of the storagepool where template allocates vm disk.
> +        * path (optional): Either pool or path to store the virtual disks should be specified
>       * graphics *(optional)*: A dict of graphics paramenters of this template
>           * type: The type of graphics. It can be VNC or spice or None.
>               * vnc: Graphical display using the Virtual Network
> diff --git a/i18n.py b/i18n.py
> index ea2c4ab..7e60079 100644
> --- a/i18n.py
> +++ b/i18n.py
> @@ -190,6 +190,9 @@ messages = {
>       "KCHTMPL0031E": _("Memory value (%(mem)sMiB) must be equal or lesser than maximum memory value (%(maxmem)sMiB)"),
>       "KCHTMPL0032E": _("Unable to update template due error: %(err)s"),
>       "KCHTMPL0033E": _("Parameter 'disks' requires at least one disk object"),
> +    "KCHTMPL0034E": _("Storage without libvirt pool is not supported on this architecture"),
> +    "KCHTMPL0035E": _("Error while creating the virtual disk for the guest. Details: %(err)s"),
> +    "KCHTMPL0036E": _("When setting template disks without libvirt, following parameters are required: 'index', 'format', 'path', 'size'"),
>
>       "KCHPOOL0001E": _("Storage pool %(name)s already exists"),
>       "KCHPOOL0002E": _("Storage pool %(name)s does not exist"),
> diff --git a/model/storagepools.py b/model/storagepools.py
> index af92ec9..5942b31 100644
> --- a/model/storagepools.py
> +++ b/model/storagepools.py
> @@ -33,7 +33,7 @@ from wok.plugins.kimchi.model.host import DeviceModel
>   from wok.plugins.kimchi.model.libvirtstoragepool import StoragePoolDef
>   from wok.plugins.kimchi.osinfo import defaults as tmpl_defaults
>   from wok.plugins.kimchi.scan import Scanner
> -from wok.plugins.kimchi.utils import pool_name_from_uri
> +from wok.plugins.kimchi.utils import pool_name_from_uri, is_s390x
>
>
>   ISO_POOL_NAME = u'kimchi_isos'
> @@ -57,6 +57,7 @@ STORAGE_SOURCES = {'netfs': {'addr': '/pool/source/host/@name',
>
>
>   class StoragePoolsModel(object):
> +
>       def __init__(self, **kargs):
>           self.conn = kargs['conn']
>           self.objstore = kargs['objstore']
> @@ -72,6 +73,9 @@ class StoragePoolsModel(object):
>       def _check_default_pools(self):
>           pools = {}
>
> +        if is_s390x():
> +            return
> +
>           default_pool = tmpl_defaults['disks'][0]['pool']['name']
>           default_pool = default_pool.split('/')[-1]
>
> @@ -437,9 +441,10 @@ class StoragePoolModel(object):
>               for tmpl in templates:
>                   t_info = session.get('template', tmpl)
>                   for disk in t_info['disks']:
> -                    t_pool = disk['pool']['name']
> -                    if pool_name_from_uri(t_pool) == pool_name:
> -                        return True
> +                    if 'pool' in disk:
> +                        t_pool = disk['pool']['name']
> +                        if pool_name_from_uri(t_pool) == pool_name:
> +                            return True
>               return False
>
>       def deactivate(self, name):
> diff --git a/model/storagevolumes.py b/model/storagevolumes.py
> index 4708674..d721d3b 100644
> --- a/model/storagevolumes.py
> +++ b/model/storagevolumes.py
> @@ -44,7 +44,6 @@ from wok.plugins.kimchi.model.diskutils import set_disk_used_by
>   from wok.plugins.kimchi.model.storagepools import StoragePoolModel
>   from wok.plugins.kimchi.utils import get_next_clone_name
>
> -
>   VOLUME_TYPE_MAP = {0: 'file',
>                      1: 'block',
>                      2: 'directory',
> diff --git a/model/templates.py b/model/templates.py
> index 8df8c3b..c749f4b 100644
> --- a/model/templates.py
> +++ b/model/templates.py
> @@ -28,13 +28,15 @@ import urlparse
>
>   from wok.exception import InvalidOperation, InvalidParameter
>   from wok.exception import NotFoundError, OperationFailed
> -from wok.utils import probe_file_permission_as_user, run_setfacl_set_attr
> +from wok.utils import probe_file_permission_as_user
> +from wok.utils import run_setfacl_set_attr
>   from wok.xmlutils.utils import xpath_get_text
>
>   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 create_disk_image
>   from wok.plugins.kimchi.vmtemplate import VMTemplate
>
>   ISO_TYPE = "ISO 9660 CD-ROM"
> @@ -400,15 +402,26 @@ class LibvirtVMTemplate(VMTemplate):
>
>       def fork_vm_storage(self, vm_uuid):
>           # Provision storages:
> -        vol_list = self.to_volume_list(vm_uuid)
> +        disk_and_vol_list = self.to_volume_list(vm_uuid)
>           try:
> -            for v in vol_list:
> -                pool = self._get_storage_pool(v['pool'])
> -                # outgoing text to libvirt, encode('utf-8')
> -                pool.createXML(v['xml'].encode('utf-8'), 0)
> +            for v in disk_and_vol_list:
> +                if v['pool'] is not None:
> +                    pool = self._get_storage_pool(v['pool'])
> +                    # outgoing text to libvirt, encode('utf-8')
> +                    pool.createXML(v['xml'].encode('utf-8'), 0)
> +                else:
> +                    capacity = v['capacity']
> +                    format_type = v['format']
> +                    path = v['path']
> +                    create_disk_image(
> +                        format_type=format_type,
> +                        path=path,
> +                        capacity=capacity)
> +
>           except libvirt.libvirtError as e:
>               raise OperationFailed("KCHVMSTOR0008E", {'error': e.message})
> -        return vol_list
> +
> +        return disk_and_vol_list
>
>       def set_cpu_info(self):
>           # undefined topology: consider these values to calculate maxvcpus
> diff --git a/model/vms.py b/model/vms.py
> index 7f607f5..3ad722e 100644
> --- a/model/vms.py
> +++ b/model/vms.py
> @@ -61,7 +61,7 @@ from wok.plugins.kimchi.model.utils import remove_metadata_node
>   from wok.plugins.kimchi.model.utils import set_metadata_node
>   from wok.plugins.kimchi.osinfo import defaults, MEM_DEV_SLOTS
>   from wok.plugins.kimchi.screenshot import VMScreenshot
> -from wok.plugins.kimchi.utils import get_next_clone_name
> +from wok.plugins.kimchi.utils import get_next_clone_name, is_s390x
>   from wok.plugins.kimchi.utils import template_name_from_uri
>   from wok.plugins.kimchi.xmlutils.bootorder import get_bootorder_node
>   from wok.plugins.kimchi.xmlutils.bootorder import get_bootmenu_node
> @@ -1395,6 +1395,13 @@ class VMModel(object):
>               except libvirt.libvirtError as e:
>                   wok_log.error('Unable to get storage volume by path: %s' %
>                                 e.message)
> +                try:
> +                    if is_s390x() and os.path.exists(path):
> +                        os.remove(path)
> +                except Exception as e:
> +                    wok_log.error('Unable to delete storage path: %s' %
> +                                  e.message)
> +
>               except Exception as e:
>                   raise OperationFailed('KCHVOL0017E', {'err': e.message})
>
> diff --git a/osinfo.py b/osinfo.py
> index 528cf14..b177830 100644
> --- a/osinfo.py
> +++ b/osinfo.py
> @@ -158,6 +158,12 @@ def _get_tmpl_defaults():
>                                  'maxmemory': _get_default_template_mem()}
>       tmpl_defaults['storage']['disk.0'] = {'size': 10, 'format': 'qcow2',
>                                             'pool': 'default'}

> +    is_on_s390x = True if _get_arch() == 's390x' else False
> +

You created a function is_s390x function in utils.py so it is better to 
reuse it instead of create a new variable.

> +    if is_on_s390x:
> +        tmpl_defaults['storage']['disk.0']['path'] = '/var/lib/libvirt/images/'
> +        del tmpl_defaults['storage']['disk.0']['pool']
> +
>       tmpl_defaults['processor']['vcpus'] = 1
>       tmpl_defaults['processor']['maxvcpus'] = 1
>       tmpl_defaults['graphics'] = {'type': 'vnc', 'listen': '127.0.0.1'}
> @@ -165,7 +171,12 @@ def _get_tmpl_defaults():
>       default_config = ConfigObj(tmpl_defaults)
>
>       # Load template configuration file
> -    config_file = os.path.join(kimchiPaths.sysconf_dir, 'template.conf')
> +    if is_on_s390x:
> +        config_file = os.path.join(
> +            kimchiPaths.sysconf_dir,
> +            'template_s390x.conf')
> +    else:
> +        config_file = os.path.join(kimchiPaths.sysconf_dir, 'template.conf')
>       config = ConfigObj(config_file)
>
>       # Merge default configuration with file configuration
> @@ -186,11 +197,27 @@ def _get_tmpl_defaults():
>       # Parse storage section to get disks values
>       storage_section = default_config.pop('storage')
>       defaults['disks'] = []
> -    for disk in storage_section.keys():
> +
> +    for index, disk in enumerate(storage_section.keys()):
>           data = storage_section[disk]
>           data['index'] = int(disk.split('.')[1])
> -        data['pool'] = {"name": '/plugins/kimchi/storagepools/' +
> -                        storage_section[disk].pop('pool')}
> +        # Right now 'Path' is only supported on s390x
> +        if storage_section[disk].get('path') and is_on_s390x:
> +            data['path'] = storage_section[disk].pop('path')
> +            if 'size' not in storage_section[disk]:
> +                data['size'] = tmpl_defaults['storage']['disk.0']['size']
> +            else:
> +                data['size'] = storage_section[disk].pop('size')
> +
> +            if 'format' not in storage_section[disk]:
> +                data['format'] = tmpl_defaults['storage']['disk.0']['format']
> +            else:
> +                data['format'] = storage_section[disk].pop('format')
> +        else:
> +            storage_section[disk].get('pool')
> +            data['pool'] = {"name": '/plugins/kimchi/storagepools/' +
> +                                    storage_section[disk].pop('pool')}
> +
>           defaults['disks'].append(data)
>
>       # Parse processor section to get vcpus and cpu_topology values
> diff --git a/utils.py b/utils.py
> index 26d3cf6..0fca191 100644
> --- a/utils.py
> +++ b/utils.py
> @@ -24,6 +24,7 @@ import platform
>   import re
>   import sqlite3
>   import time
> +import os
>   import urllib2
>   from httplib import HTTPConnection, HTTPException
>   from urlparse import urlparse
> @@ -31,6 +32,7 @@ from urlparse import urlparse
>   from wok.exception import InvalidParameter, OperationFailed
>   from wok.plugins.kimchi import config
>   from wok.plugins.kimchi.osinfo import get_template_default
> +from wok.stringutils import encode_value
>   from wok.utils import run_command, wok_log
>   from wok.xmlutils.utils import xpath_get_text
>
> @@ -272,3 +274,35 @@ def is_libvirtd_up():
>
>       output, error, rc = run_command(cmd, silent=True)
>       return True if output == 'active\n' else False
> +
> +
> +def is_s390x():
> +    """
> +    Check if current arch is 's390x'
> +    Returns:
> +    """
> +    if os.uname()[4] == 's390x':
> +        return True
> +
> +    return False
> +
> +
> +def create_disk_image(format_type, path, capacity):
> +    """
> +    Create a disk image for the Guest
> +    Args:
> +        format: Format of the storage. e.g. qcow2
> +        path: Path where the virtual disk will be created
> +        capacity: Capacity of the virtual disk in GBs
> +
> +    Returns:
> +
> +    """
> +    out, err, rc = run_command(
> +        ["/usr/bin/qemu-img", "create", "-f", format_type, "-o",
> +         "preallocation=metadata", path, encode_value(capacity) + "G"])
> +
> +    if rc != 0:
> +        raise OperationFailed("KCHTMPL0035E", {'err': err})
> +
> +    return
> \ No newline at end of file
> diff --git a/vmtemplate.py b/vmtemplate.py
> index babf050..2d3a351 100644
> --- a/vmtemplate.py
> +++ b/vmtemplate.py
> @@ -31,7 +31,8 @@ from wok.exception import MissingParameter, OperationFailed
>   from wok.plugins.kimchi import imageinfo
>   from wok.plugins.kimchi import osinfo
>   from wok.plugins.kimchi.isoinfo import IsoImage
> -from wok.plugins.kimchi.utils import check_url_path, pool_name_from_uri
> +from wok.plugins.kimchi.utils import check_url_path, is_s390x
> +from wok.plugins.kimchi.utils import pool_name_from_uri
>   from wok.plugins.kimchi.xmlutils.bootorder import get_bootorder_xml
>   from wok.plugins.kimchi.xmlutils.cpu import get_cpu_xml
>   from wok.plugins.kimchi.xmlutils.disk import get_disk_xml
> @@ -42,6 +43,7 @@ from wok.plugins.kimchi.xmlutils.serial import get_serial_xml
>
>
>   class VMTemplate(object):
> +
>       def __init__(self, args, scan=False, netboot=False):
>           """
>           Construct a VM Template from a widely variable amount of information.
> @@ -95,35 +97,55 @@ class VMTemplate(object):
>           disks = self.info.get('disks')
>
>           basic_disk = ['index', 'format', 'pool', 'size']
> +        basic_path_disk = ['index', 'format', 'path', 'size']
>           ro_disk = ['index', 'format', 'pool', 'volume']
>           base_disk = ['index', 'base', 'pool', 'size', 'format']
>
>           for index, disk in enumerate(disks):
>               disk_info = dict(default_disk)
> -
> -            pool = disk.get('pool', default_disk['pool'])
> -            pool_type = self._get_storage_type(pool['name'])
> -
> -            if pool_type in ['iscsi', 'scsi']:
> -                disk_info = {'index': 0, 'format': 'raw', 'volume': None}
> -
> -            disk_info.update(disk)
> -            pool_name = disk_info.get('pool', {}).get('name')
> -            if pool_name is None:
> -                raise MissingParameter('KCHTMPL0028E')
> -
> -            keys = sorted(disk_info.keys())
> -            if ((keys != sorted(basic_disk)) and (keys != sorted(ro_disk)) and
> -                    (keys != sorted(base_disk))):
> -                raise MissingParameter('KCHTMPL0028E')
> -
> -            if pool_type in ['logical', 'iscsi', 'scsi']:
> -                if disk_info['format'] != 'raw':
> -                    raise InvalidParameter('KCHTMPL0029E')
> -
> -            disk_info['pool']['type'] = pool_type
> -            disk_info['index'] = disk_info.get('index', index)
> -            self.info['disks'][index] = disk_info
> +            if disk.get('pool'):
> +                pool = disk.get('pool', default_disk.get('pool'))
> +                pool_type = self._get_storage_type(pool['name'])
> +                if pool_type in ['iscsi', 'scsi']:
> +                    disk_info = {'index': 0, 'format': 'raw', 'volume': None}
> +
> +                # This check is required where 'path' disk
> +                # exists and is replaced by 'pool' disk during
> +                # template update
> +                if 'path' in disk_info:
> +                    del disk_info['path']
> +
> +                disk_info.update(disk)
> +                pool_name = disk_info.get('pool', {}).get('name')
> +                if pool_name is None:
> +                    raise MissingParameter('KCHTMPL0028E')
> +
> +                keys = sorted(disk_info.keys())
> +                if ((keys != sorted(basic_disk)) and
> +                        (keys != sorted(ro_disk)) and
> +                        (keys != sorted(base_disk)) and
> +                        (keys != sorted(basic_path_disk))):
> +                    raise MissingParameter('KCHTMPL0028E')
> +
> +                if pool_type in ['logical', 'iscsi', 'scsi']:
> +                    if disk_info['format'] != 'raw':
> +                        raise InvalidParameter('KCHTMPL0029E')
> +
> +                disk_info['pool']['type'] = pool_type
> +                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['path'])
> +                disk_info.update(disk)
> +                keys = sorted(disk_info.keys())
> +                if keys != sorted(basic_path_disk):
> +                    raise MissingParameter('KCHTMPL0036E')
> +                disk_info['path'] = path
> +                disk_info['index'] = disk_info.get('index', index)
> +                self.info['disks'][index] = disk_info
> +            else:
> +                raise InvalidParameter('KCHTMPL0034E')
>
>       def _get_os_info(self, args, scan):
>           distro = version = 'unknown'
> @@ -217,8 +239,9 @@ class VMTemplate(object):
>               params = dict(base_disk_params)
>               params['format'] = disk['format']
>               params['index'] = index
> -            params.update(locals().get('%s_disk_params' %
> -                                       disk['pool']['type'], {}))
> +            if disk.get('pool'):
> +                params.update(locals().get('%s_disk_params' %
> +                                           disk['pool']['type'], {}))
>
>               volume = disk.get('volume')
>               if volume is not None:
> @@ -226,9 +249,13 @@ class VMTemplate(object):
>                                                          volume)
>               else:
>                   img = "%s-%s.img" % (vm_uuid, params['index'])
> -                storage_path = self._get_storage_path(disk['pool']['name'])
> +                if disk.get('pool'):
> +                    storage_path = self._get_storage_path(disk['pool']['name'])
> +                    params['pool_type'] = disk['pool']['type']
> +                elif disk.get('path'):
> +                    storage_path = disk.get('path')
> +                    params['pool_type'] = None
>                   params['path'] = os.path.join(storage_path, img)
> -                params['pool_type'] = disk['pool']['type']
>               disks_xml += get_disk_xml(params)[1]
>
>           return unicode(disks_xml, 'utf-8')
> @@ -237,20 +264,24 @@ class VMTemplate(object):
>           ret = []
>           for i, d in enumerate(self.info['disks']):
>               # Create only .img. If storagepool is (i)SCSI, volumes will be LUNs
> -            if d['pool']['type'] in ["iscsi", "scsi"]:
> +            if 'pool' in d and d['pool']['type'] in ["iscsi", "scsi"]:
>                   continue
>
>               index = d.get('index', i)
>               volume = "%s-%s.img" % (vm_uuid, index)
>
> -            storage_path = self._get_storage_path(d['pool']['name'])
> +            if 'path' in d:
> +                storage_path = d['path']
> +            else:
> +                storage_path = self._get_storage_path(d['pool']['name'])
> +
>               info = {'name': volume,
>                       'capacity': d['size'],
>                       'format': d['format'],
>                       'path': '%s/%s' % (storage_path, volume),
> -                    'pool': d['pool']['name']}
> +                    'pool': d['pool']['name'] if 'pool' in d else None}
>
> -            if 'logical' == d['pool']['type'] or \
> +            if ('pool' in d and 'logical' == d['pool']['type']) or \
>                  info['format'] not in ['qcow2', 'raw']:
>                   info['allocation'] = info['capacity']
>               else:
> @@ -447,8 +478,9 @@ class VMTemplate(object):
>
>       def validate(self):
>           for disk in self.info.get('disks'):
> -            pool_uri = disk.get('pool', {}).get('name')
> -            self._get_storage_pool(pool_uri)
> +            if 'pool' in disk:
> +                pool_uri = disk.get('pool', {}).get('name')
> +                self._get_storage_pool(pool_uri)
>           self._network_validate()
>           self._iso_validate()
>           self.cpuinfo_validate()
> @@ -499,10 +531,11 @@ class VMTemplate(object):
>
>           # validate storagepools and image-based templates integrity
>           for disk in self.info['disks']:
> -            pool_uri = disk['pool']['name']
> -            pool_name = pool_name_from_uri(pool_uri)
> -            if pool_name not in self._get_active_storagepools_name():
> -                invalid['storagepools'] = [pool_name]
> +            if 'pool' in disk:
> +                pool_uri = disk['pool']['name']
> +                pool_name = pool_name_from_uri(pool_uri)
> +                if pool_name not in self._get_active_storagepools_name():
> +                    invalid['storagepools'] = [pool_name]
>
>               if disk.get("base") is None:
>                   continue




More information about the Kimchi-devel mailing list