Hi Harshal,
Some inline comments below. Please feel free to reply back if it
does not make sense to you.
Thanks,
Archana Singh
From: Harshal Patil <harshalp@linux.vnet.ibm.com>
This patch adds support for creating templates on s390x arch
without using libvirt related storage calls
Signed-off-by: Harshal Patil <harshalp@linux.vnet.ibm.com>
---
docs/API.md | 2 +
i18n.py | 2 +
model/storagepools.py | 15 +++++--
model/storagevolumes.py | 10 +++--
model/templates.py | 56 +++++++++++++++++++++-----
model/vms.py | 38 ++++++++++++------
osinfo.py | 47 ++++++++++++++++++++--
utils.py | 13 ++++++
vmtemplate.py | 105 ++++++++++++++++++++++++++++++------------------
9 files changed, 213 insertions(+), 75 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 e8d9c05..8e55e01 100644
--- a/i18n.py
+++ b/i18n.py
@@ -190,6 +190,8 @@ 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"),
"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 a2dbaec..cbd756b 100644
--- a/model/storagepools.py
+++ b/model/storagepools.py
@@ -32,7 +32,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'
@@ -56,6 +56,7 @@ STORAGE_SOURCES = {'netfs': {'addr': '/pool/source/host/@name',
class StoragePoolsModel(object):
+
def __init__(self, **kargs):
self.conn = kargs['conn']
self.objstore = kargs['objstore']
@@ -71,6 +72,11 @@ class StoragePoolsModel(object):
def _check_default_pools(self):
pools = {}
+ if 'disks' not in tmpl_defaults or len(tmpl_defaults['disks']) == 0 \
+ or (not tmpl_defaults.get('disks')[0].get(
+ 'pool') and is_s390x()): # s390x specific handling
Can we have some more details in comments on s390x checks?
+ return
+ tmpl_defaults.get('disks')
default_pool = tmpl_defaults['disks'][0]['pool']['name']
default_pool = default_pool.split('/')[-1]
@@ -436,9 +442,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 7b2272b..1e4a2af 100644
--- a/model/storagevolumes.py
+++ b/model/storagevolumes.py
@@ -41,8 +41,7 @@ from wok.plugins.kimchi.kvmusertests import UserTests
from wok.plugins.kimchi.model.diskutils import get_disk_used_by
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
-
+from wok.plugins.kimchi.utils import get_next_clone_name, is_s390x
VOLUME_TYPE_MAP = {0: 'file',
1: 'block',
@@ -275,8 +274,11 @@ class StorageVolumeModel(object):
self.task = TaskModel(**kargs)
self.storagevolumes = StorageVolumesModel(**kargs)
self.storagepool = StoragePoolModel(**kargs)
- if self.conn.get() is not None:
- self.libvirt_user = UserTests().probe_user()
+ if is_s390x(): # s390x specific handling
+ if self.conn.get() is not None:
+ self.libvirt_user = UserTests().probe_user()
+ else:
+ self.libvirt_user = None
else:
self.libvirt_user = None
In case of architecture other than s390x also
UserTests().probe_user() is called incase self.conn.get() is not
None, otherwise self.libvirt_user is assigned to None. Any specific
reason of having architecture check and doing the same thing?
diff --git a/model/templates.py b/model/templates.py
index a299c85..4edd98e 100644
--- a/model/templates.py
+++ b/model/templates.py
@@ -28,7 +28,8 @@ 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, run_command
+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
@@ -47,6 +48,7 @@ if os.uname()[4] in ['ppc', 'ppc64', 'ppc64le']:
class TemplatesModel(object):
+
def __init__(self, **kargs):
self.objstore = kargs['objstore']
self.conn = kargs['conn']
@@ -142,7 +144,7 @@ class TemplatesModel(object):
get_kimchi_version())
except InvalidOperation:
raise
- except Exception, e:
+ except Exception as e:
raise OperationFailed('KCHTMPL0020E', {'err': e.message})
return name
@@ -171,6 +173,7 @@ class TemplatesModel(object):
class TemplateModel(object):
+
def __init__(self, **kargs):
self.objstore = kargs['objstore']
self.conn = kargs['conn']
@@ -198,10 +201,9 @@ class TemplateModel(object):
# set default name
subfixs = [v[len(name):] for v in self.templates.get_list()
if v.startswith(name)]
- indexs = [int(v.lstrip("-clone")) for v in subfixs
- if v.startswith("-clone") and
- v.lstrip("-clone").isdigit()]
- indexs.sort()
+ indexs = sorted([int(v.lstrip("-clone")) for v in subfixs
+ if v.startswith("-clone") and
+ v.lstrip("-clone").isdigit()])
index = "1" if not indexs else str(indexs[-1] + 1)
clone_name = name + "-clone" + index
@@ -267,7 +269,7 @@ class TemplateModel(object):
except InvalidOperation:
raise
- except Exception, e:
+ except Exception as e:
raise OperationFailed('KCHTMPL0032E', {'err': e.message})
return params['name']
@@ -290,7 +292,7 @@ def validate_memory(memory):
# memory limit
if (current > (MAX_MEM_LIM >> 10)) or (maxmem > (MAX_MEM_LIM >> 10)):
raise InvalidParameter("KCHVM0079E",
- {'value': str(MAX_MEM_LIM / (1024**3))})
+ {'value': str(MAX_MEM_LIM / (1024 ** 3))})
if (current > host_memory) or (maxmem > host_memory):
raise InvalidParameter("KCHVM0078E", {'memHost': host_memory})
@@ -316,6 +318,7 @@ def validate_memory(memory):
class LibvirtVMTemplate(VMTemplate):
+
def __init__(self, args, scan=False, conn=None):
self.conn = conn
netboot = True if 'netboot' in args.keys() else False
@@ -398,16 +401,47 @@ class LibvirtVMTemplate(VMTemplate):
raise NotFoundError("KCHVOL0002E", {'name': vol,
'pool': pool})
+ def _create_disk_image(self, format_type, path, capacity):
This should not be part of libvirtVMTemplate class as name says I
guess we should not have any non libvirt related handling here. What
do you think?str(capacity) will throw error in case of unicode characters. I will suggest to ensure that its integer and use encode or decode methods to do str conversion.+ """ + 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, str(capacity) + "G"])
+
+ if rc != 0:
+ raise OperationFailed("KCHTMPL0035E", {'err': err})
+
+ return
+
def fork_vm_storage(self, vm_uuid):
# Provision storages:
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)
+ if v['pool'] is not None:
If 'pool' key will not present, it will throw key error. Is this
expected to be the behaviour?
+ 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']
+ self._create_disk_image(
+ format_type=format_type,
+ path=path,
+ capacity=capacity)
I think as name suggested to_volume_list, it should look correct
from naming point of view as iterating to volume and handing non
volume related stuff. What do you think?
+
except libvirt.libvirtError as e:
raise OperationFailed("KCHVMSTOR0008E", {'error': e.message})
+
return vol_list
def set_cpu_info(self):
diff --git a/model/vms.py b/model/vms.py
index 3380278..5d1eebb 100644
--- a/model/vms.py
+++ b/model/vms.py
@@ -60,13 +60,13 @@ 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
from wok.plugins.kimchi.xmlutils.cpu import get_topology_xml
from wok.plugins.kimchi.xmlutils.disk import get_vm_disk_info, get_vm_disks
-from utils import has_cpu_numa, set_numa_memory
+from .utils import has_cpu_numa, set_numa_memory
DOM_STATE_MAP = {0: 'nostate',
@@ -114,6 +114,7 @@ vm_locks = {}
class VMsModel(object):
+
def __init__(self, **kargs):
self.conn = kargs['conn']
self.objstore = kargs['objstore']
@@ -231,6 +232,7 @@ class VMsModel(object):
class VMModel(object):
+
def __init__(self, **kargs):
self.conn = kargs['conn']
self.objstore = kargs['objstore']
@@ -373,7 +375,7 @@ class VMModel(object):
vir_dom = self.get_vm(name, self.conn)
flags = libvirt.VIR_DOMAIN_XML_SECURE
xml = vir_dom.XMLDesc(flags).decode('utf-8')
- except libvirt.libvirtError, e:
+ except libvirt.libvirtError as e:
raise OperationFailed('KCHVM0035E', {'name': name,
'err': e.message})
@@ -407,7 +409,7 @@ class VMModel(object):
vir_conn = self.conn.get()
dom = vir_conn.defineXML(xml)
self._update_metadata_name(dom, nonascii_name)
- except libvirt.libvirtError, e:
+ except libvirt.libvirtError as e:
raise OperationFailed('KCHVM0035E', {'name': name,
'err': e.message})
@@ -480,7 +482,7 @@ class VMModel(object):
orig_pool_name = vir_pool.name().decode('utf-8')
orig_vol_name = vir_orig_vol.name().decode('utf-8')
- except libvirt.libvirtError, e:
+ except libvirt.libvirtError as e:
raise OperationFailed('KCHVM0035E', {'name': domain_name,
'err': e.message})
@@ -1366,7 +1368,7 @@ class VMModel(object):
# "OperationFailed" in that case.
try:
snapshot_names = self.vmsnapshots.get_list(name)
- except OperationFailed, e:
+ except OperationFailed as e:
wok_log.error('cannot list snapshots: %s; '
'skipping snapshot deleting...' % e.message)
else:
@@ -1394,6 +1396,14 @@ class VMModel(object):
except libvirt.libvirtError as e:
wok_log.error('Unable to get storage volume by path: %s' %
e.message)
Below changes will not get called if libvirtError will be thrown for
the path given. I guess we need to ensure that even if libvirt pool
and volume does not exit for path provided below gets executed for
s390x.
+ try:
+ if is_s390x(): # s390x specific handling
+ if 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})
@@ -1538,7 +1548,7 @@ class VMModel(object):
raise OperationFailed("KCHVM0081E",
{'dir': serialconsole.BASE_DIRECTORY})
- websocket.add_proxy_token(name.encode('utf-8')+'-console',
+ websocket.add_proxy_token(name.encode('utf-8') + '-console',
os.path.join(serialconsole.BASE_DIRECTORY,
name.encode('utf-8')), True)
@@ -1596,7 +1606,7 @@ class VMModel(object):
try:
vir_dom.suspend()
- except libvirt.libvirtError, e:
+ except libvirt.libvirtError as e:
raise OperationFailed('KCHVM0038E', {'name': name,
'err': e.message})
@@ -1617,7 +1627,7 @@ class VMModel(object):
try:
vir_dom.resume()
- except libvirt.libvirtError, e:
+ except libvirt.libvirtError as e:
raise OperationFailed('KCHVM0040E', {'name': name,
'err': e.message})
@@ -1657,7 +1667,7 @@ class VMModel(object):
'destarch': dest_arch
}
)
- except Exception, e:
+ except Exception as e:
raise OperationFailed("KCHVM0066E", {'error': e.message})
finally:
@@ -1768,7 +1778,7 @@ class VMModel(object):
ssh_client,
id_rsa_data
)
- except Exception, e:
+ except Exception as e:
raise OperationFailed(
"KCHVM0068E",
{'host': remote_host, 'user': user, 'error': e.message}
@@ -1833,7 +1843,7 @@ class VMModel(object):
conn = self.conn.get()
vol_obj = conn.storageVolLookupByPath(disk_path)
return vol_obj.info()[1]
- except Exception, e:
+ except Exception as e:
raise OperationFailed(
"KCHVM0062E",
{'path': disk_path, 'error': e.message}
@@ -1949,6 +1959,7 @@ class VMModel(object):
class VMScreenshotModel(object):
+
def __init__(self, **kargs):
self.objstore = kargs['objstore']
self.conn = kargs['conn']
@@ -1995,6 +2006,7 @@ class VMScreenshotModel(object):
class LibvirtVMScreenshot(VMScreenshot):
+
def __init__(self, vm_uuid, conn):
VMScreenshot.__init__(self, vm_uuid)
self.conn = conn
@@ -2004,7 +2016,7 @@ class LibvirtVMScreenshot(VMScreenshot):
fd = opaque
os.write(fd, buf)
- fd = os.open(thumbnail, os.O_WRONLY | os.O_TRUNC | os.O_CREAT, 0644)
+ fd = os.open(thumbnail, os.O_WRONLY | os.O_TRUNC | os.O_CREAT, 0o644)
try:
conn = self.conn.get()
dom = conn.lookupByUUIDString(self.vm_uuid)
diff --git a/osinfo.py b/osinfo.py
index 2c59312..524b3bd 100644
--- a/osinfo.py
+++ b/osinfo.py
@@ -146,6 +146,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'}
@@ -153,7 +159,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')
Not sure, but should we handle scenario where in case s390x specific
arch conf file is not present just go with template.conf?
+ else:
+ config_file = os.path.join(kimchiPaths.sysconf_dir, 'template.conf')
config = ConfigObj(config_file)
# Merge default configuration with file configuration
@@ -174,11 +185,39 @@ 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():
+
+ pool_exists = False
+ path_exists = False
+ 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:
+ path_exists = True
+ 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')
+
+ if storage_section[disk].get('pool'):
+ pool_exists = True
+ data['pool'] = {"name": '/plugins/kimchi/storagepools/' +
+ storage_section[disk].pop('pool')}
For one disk either it can be pool or path, both should not be
added. Do you agree?
+
+ # If both pool and path don't exist, pick the defaults
+ if index == len(storage_section.keys()) - 1 and \
+ (not pool_exists and not path_exists):
+ if is_on_s390x: # Special handling for s390x
+ data['path'] = tmpl_defaults['storage']['disk.0']['path']
+ else:
+ data['pool'] = {"name": '/plugins/kimchi/storagepools/default'}
+
defaults['disks'].append(data)
# Parse processor section to get vcpus and cpu_topology values
diff --git a/utils.py b/utils.py
index 26d3cf6..b8f81ca 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
@@ -272,3 +273,15 @@ 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:
+
+ """
do we need new line in comment?
+ if os.uname()[4] == 's390x':
+ return True
+
+ return False
diff --git a/vmtemplate.py b/vmtemplate.py
index 7ac0541..c0d51a5 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,49 @@ 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}
+
+ 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 != 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
+ else:
+ if is_s390x(): # For now support 'path' only on s390x
any specific reason of keeping if in side else and not just elif?
+ path = disk.get('path', default_disk['path'])
+ disk_info.update(disk)
+ keys = sorted(disk_info.keys())
+ if keys != sorted(basic_path_disk):
+ raise MissingParameter('KCHTMPL0028E')
i18n message has to be changed to convey that either pool or path
has to be given in a disk, as current is "KCHTMPL0028E":
_("When setting
template disks, following parameters are required: 'index', 'pool
name', 'format', 'size' or 'volume' (for scsi/iscsi pools)")
+ 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 +233,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 +243,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 +258,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 not is_s390x() and d['pool']['type'] in ["iscsi", "scsi"]:
continue
in case of s390x also if pool is present and type is iscsi or scsi
we should 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'])
+
I guess as this method is meant for libvirt volume listing, I guess
we should restructure.
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:
@@ -444,8 +469,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 not is_s390x(): # s390x specific handling
+ pool_uri = disk.get('pool', {}).get('name')
+ self._get_storage_pool(pool_uri)
Again in case of s390x also if pool is present, it should validate
it.
self._network_validate()
self._iso_validate()
self.cpuinfo_validate()
@@ -494,10 +520,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