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

archus at linux.vnet.ibm.com archus at linux.vnet.ibm.com
Tue Sep 6 18:54:46 UTC 2016


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

V4:
  Rebase patch to latest upstream.

V5:
 Taken care of make check failed testcases.

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           | 123 +++++++++++++++++++++++++++++++++---------------
 9 files changed, 191 insertions(+), 56 deletions(-)

diff --git a/docs/API.md b/docs/API.md
index 1c20466..cff623e 100644
--- a/docs/API.md
+++ b/docs/API.md
@@ -438,6 +438,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 *(optional and only valid for s390x architecture)*: Storage path to store virtual disks without libvirt.
     * networks *(optional)*: list of networks will be assigned to the new VM.
     * interfaces *(optional)*: list of host network interfaces will be assigned to the new VM. Only applicable for s390x or s390 architecture.
         * type: Type of host network interface. Type should be 'macvtap' for host network interface (Ethernet, Bond, VLAN) to be connected as direct MacVTap; or 'ovs' for openvswitch host network interface to be connected as virtual switch to a VM.
@@ -504,6 +505,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 and only valid for s390x architecture)*: 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 47c829e..b6533d4 100644
--- a/i18n.py
+++ b/i18n.py
@@ -203,6 +203,9 @@ messages = {
     "KCHTMPL0037E": _("Interfaces should be list of interfaces. Each interface should have name, type and mode(optional, only applicable for interfcae type 'macvtap'."),
     "KCHTMPL0038E": _("Interface expects an object with parameters: 'name', 'type' and 'mode'. Name should be name of host network interface (Ethernet, Bond, VLAN) for type 'macvtap' or the name of host openvswitch bridge interface for type 'ovs'. Mode (optional) is only applicable for interface type 'macvtap' to indicates whether packets will be delivered directly to target device (bridge) or to the external bridge (vepa-capable bridge)."),
     "KCHTMPL0039E": _("Interfaces parameter only supported on s390x or s390 architecture."),
+    "KCHTMPL0040E": _("Storage without libvirt pool is not supported on this architecture"),
+    "KCHTMPL0041E": _("Error while creating the virtual disk for the guest. Details: %(err)s"),
+    "KCHTMPL0042E": _("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 a6ce97b..2d68027 100644
--- a/model/storagevolumes.py
+++ b/model/storagevolumes.py
@@ -43,7 +43,6 @@ from wok.plugins.kimchi.model.diskutils import get_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 04e6626..a5d17af 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"
@@ -417,15 +419,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 b889166..6a9b1d1 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
@@ -1405,6 +1405,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 3e56d97..8ec9480 100644
--- a/osinfo.py
+++ b/osinfo.py
@@ -159,6 +159,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
+
+    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'}
@@ -166,7 +172,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
@@ -187,11 +198,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..7129bfe 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("KCHTMPL0041E", {'err': err})
+
+    return
diff --git a/vmtemplate.py b/vmtemplate.py
index 07cebb9..3b5eeac 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,67 @@ 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']
+        base_path_disk = ['index', 'base', 'path', '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
+            # 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')
+
+            if is_s390x():
+                # On s390x/s390 pool is optional attribute for disk.
+                pool = disk.get('pool', default_disk.get('pool'))
+            else:
+                pool = disk.get('pool', default_disk['pool'])
+
+            if 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.get('path'))
+                disk_info.update(disk)
+                keys = sorted(disk_info.keys())
+                if ((keys != sorted(basic_path_disk)) and
+                   (keys != sorted(base_path_disk))):
+                    raise MissingParameter('KCHTMPL0042E')
+
+                disk_info['path'] = path
+                disk_info['index'] = disk_info.get('index', index)
+                self.info['disks'][index] = disk_info
 
     def _get_os_info(self, args, scan):
         distro = version = 'unknown'
@@ -217,8 +251,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 +261,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 +276,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:
@@ -467,8 +510,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()
@@ -519,10 +563,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
-- 
2.7.4




More information about the Kimchi-devel mailing list