[PATCH v2 0/3] Fix storage volume issues
This is the changelog between this and the previous patchset: - Instead of removing the ISO permission check at all, update it so the file's ACL is correctly updated before checking its permission; Crístian Viana (3): issue #565: Allow a template's ISO to be a block device issue #564: Parse logical volumes to find out their actual formats Update ISO file's ACL before checking its permission src/kimchi/isoinfo.py | 7 +++++-- src/kimchi/model/storagevolumes.py | 19 ++++++++++++++++++- src/kimchi/model/templates.py | 20 ++++++++++++-------- src/kimchi/vmtemplate.py | 10 ++++++++-- 4 files changed, 43 insertions(+), 13 deletions(-) -- 2.1.0
The current template code checks whether an ISO path is a regular file (i.e. not a directory, not a char device, not a link). However, a block device may also be a valid ISO path because storage volumes belonging to logical pools are always block devices (e.g. /dev/mypool/myvol.iso), and they should also be used to create templates. Instead of allowing only regular files to be used as template ISOs, allow block devices as well. Fix issue #565 (Allow creating templates with device files). Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/isoinfo.py | 7 +++++-- src/kimchi/model/templates.py | 17 ++++++++++------- src/kimchi/vmtemplate.py | 10 ++++++++-- 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/src/kimchi/isoinfo.py b/src/kimchi/isoinfo.py index b5a1769..badb002 100644 --- a/src/kimchi/isoinfo.py +++ b/src/kimchi/isoinfo.py @@ -22,6 +22,7 @@ import glob import platform import os import re +import stat import struct import sys import urllib2 @@ -151,8 +152,10 @@ class IsoImage(object): self._scan() def _is_iso_remote(self): - if os.path.isfile(self.path): - return False + if os.path.exists(self.path): + st_mode = os.stat(self.path).st_mode + if stat.S_ISREG(st_mode) or stat.S_ISBLK(st_mode): + return False if check_url_path(self.path): return True diff --git a/src/kimchi/model/templates.py b/src/kimchi/model/templates.py index e91636b..4ea1c0e 100644 --- a/src/kimchi/model/templates.py +++ b/src/kimchi/model/templates.py @@ -19,6 +19,7 @@ import copy import os +import stat import libvirt @@ -41,13 +42,15 @@ class TemplatesModel(object): name = params.get('name', '').strip() iso = params.get('cdrom') # check search permission - if iso and iso.startswith('/') and os.path.isfile(iso): - user = UserTests().probe_user() - ret, excp = probe_file_permission_as_user(iso, user) - if ret is False: - raise InvalidParameter('KCHISO0008E', - {'filename': iso, 'user': user, - 'err': excp}) + if iso and iso.startswith('/') and os.path.exists(iso): + st_mode = os.stat(iso).st_mode + if stat.S_ISREG(st_mode) or stat.S_ISBLK(st_mode): + user = UserTests().probe_user() + ret, excp = probe_file_permission_as_user(iso, user) + if ret is False: + raise InvalidParameter('KCHISO0008E', + {'filename': iso, 'user': user, + 'err': excp}) cpu_info = params.get('cpu_info') if cpu_info: diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py index ef41d0a..ec477dd 100644 --- a/src/kimchi/vmtemplate.py +++ b/src/kimchi/vmtemplate.py @@ -19,6 +19,7 @@ import os import socket +import stat import time import urlparse import uuid @@ -403,8 +404,13 @@ class VMTemplate(object): # validate iso integrity # FIXME when we support multiples cdrom devices iso = self.info.get('cdrom') - if iso and not (os.path.isfile(iso) or check_url_path(iso)): - invalid['cdrom'] = [iso] + if iso: + if os.path.exists(iso): + st_mode = os.stat(iso).st_mode + if not (stat.S_ISREG(st_mode) or stat.S_ISBLK(st_mode)): + invalid['cdrom'] = [iso] + elif not check_url_path(iso): + invalid['cdrom'] = [iso] self.info['invalid'] = invalid -- 2.1.0
All storage volumes belonging to a logical storage pool are reported by libvirt as 'raw', even when they are ISO images. When creating a template, only the volumes listed as 'iso' are displayed, so it's currently not possible to create a template using a volume from a logical storage pool. Instead of relying only on the volume format reported by libvirt, open and inspect the storage volumes which belong to a logical storage pool in order to find out whether they're ISO images. Fix issue #564 (List volumes other than "format=iso" when creating a template). Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/model/storagevolumes.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py index 0480496..f02efca 100644 --- a/src/kimchi/model/storagevolumes.py +++ b/src/kimchi/model/storagevolumes.py @@ -294,6 +294,7 @@ class StorageVolumeModel(object): self.objstore = kargs['objstore'] self.task = TaskModel(**kargs) self.storagevolumes = StorageVolumesModel(**kargs) + self.storagepool = StoragePoolModel(**kargs) @staticmethod def get_storagevolume(poolname, name, conn): @@ -321,6 +322,21 @@ class StorageVolumeModel(object): # infomation. When there is no format information, we assume # it's 'raw'. fmt = 'raw' + + iso_img = None + + # 'raw' volumes from 'logical' pools may actually be 'iso'; + # libvirt always reports them as 'raw' + pool_info = self.storagepool.lookup(pool) + if pool_info['type'] == 'logical' and fmt == 'raw': + try: + iso_img = IsoImage(path) + except IsoFormatError: + # not 'iso' afterall + pass + else: + fmt = 'iso' + ref_cnt = get_disk_ref_cnt(self.objstore, self.conn, path) res = dict(type=VOLUME_TYPE_MAP[info[0]], capacity=info[1], @@ -333,7 +349,8 @@ class StorageVolumeModel(object): path = os.path.join(os.path.dirname(path), os.readlink(path)) os_distro = os_version = 'unknown' try: - iso_img = IsoImage(path) + if iso_img is None: + iso_img = IsoImage(path) os_distro, os_version = iso_img.probe() bootable = True except IsoFormatError: -- 2.1.0
A virtual machine is executed by the system user "qemu", so we we need to make sure that user is able to read an ISO image. Add the user "qemu" to the ISO file's ACL before checking if that user is able to read that file. Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/model/templates.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/kimchi/model/templates.py b/src/kimchi/model/templates.py index 4ea1c0e..6bc8aca 100644 --- a/src/kimchi/model/templates.py +++ b/src/kimchi/model/templates.py @@ -28,7 +28,7 @@ from kimchi.exception import NotFoundError, OperationFailed from kimchi.kvmusertests import UserTests from kimchi.model.cpuinfo import CPUInfoModel from kimchi.utils import pool_name_from_uri -from kimchi.utils import probe_file_permission_as_user +from kimchi.utils import probe_file_permission_as_user, run_setfacl_set_attr from kimchi.vmtemplate import VMTemplate from kimchi.xmlutils.utils import xpath_get_text @@ -46,6 +46,7 @@ class TemplatesModel(object): st_mode = os.stat(iso).st_mode if stat.S_ISREG(st_mode) or stat.S_ISBLK(st_mode): user = UserTests().probe_user() + run_setfacl_set_attr(iso, user=user) ret, excp = probe_file_permission_as_user(iso, user) if ret is False: raise InvalidParameter('KCHISO0008E', -- 2.1.0
participants (2)
-
Aline Manera -
Crístian Viana