[PATCH 0/3 V3] [issue #731] Raw volumes validation
From: Paulo Vital <pvital@linux.vnet.ibm.com> V2 -> V3: - Update model/vmstorages.py to solve issue when using REST API directly. - Changed return of validation to True/False instead of 1/0. - Update tests to reflect REST API changes. - Update MockModel to be correctly used by tests. V1 -> V2: - Add new supported valid Raw content. - Make list of valid Raw content global. Kimchi allows to attach Raw volumes that are not disk images when editing a VM. With this, it's possible attach a XML or PDF file (that qemu-img and libvirt see as raw volumes) as a disk into the guest. This patch-set introduces a check for all 'raw' volumes to see if them are valid disks or not (by using libmagic), and then, pass this information to the front-end to make a correct list of volumes option when attaching a new one into a VM. Paulo Vital (3): Raw volumes validation: update contrib and README Raw volumes validation: back-end and front-end Raw volumes validation: update tests contrib/DEBIAN/control.in | 3 ++- contrib/kimchi.spec.fedora.in | 1 + contrib/kimchi.spec.suse.in | 1 + docs/API.md | 1 + docs/README.md | 11 +++++------ src/kimchi/control/storagevolumes.py | 3 ++- src/kimchi/mockmodel.py | 10 ++++++++-- src/kimchi/model/storagevolumes.py | 21 ++++++++++++++++++++- src/kimchi/model/vmstorages.py | 4 ++++ tests/test_model_storagevolume.py | 2 +- tests/test_rest.py | 2 ++ ui/js/src/kimchi.guest_storage_add.main.js | 2 +- 12 files changed, 48 insertions(+), 13 deletions(-) -- 2.4.3
From: Paulo Vital <pvital@linux.vnet.ibm.com> Update Fedora and Suse spec files, Debian control file and README.md to add python-magic requirement. Signed-off-by: Paulo Vital <pvital@linux.vnet.ibm.com> --- contrib/DEBIAN/control.in | 3 ++- contrib/kimchi.spec.fedora.in | 1 + contrib/kimchi.spec.suse.in | 1 + docs/README.md | 11 +++++------ 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/contrib/DEBIAN/control.in b/contrib/DEBIAN/control.in index a288e9b..1211f1f 100644 --- a/contrib/DEBIAN/control.in +++ b/contrib/DEBIAN/control.in @@ -28,7 +28,8 @@ Depends: python-cherrypy3 (>= 3.2.0), python-guestfs, python-ldap, libguestfs-tools, - spice-html5 + spice-html5, + python-magic Build-Depends: libxslt, openssl, python-lxml diff --git a/contrib/kimchi.spec.fedora.in b/contrib/kimchi.spec.fedora.in index 183fd43..8463930 100644 --- a/contrib/kimchi.spec.fedora.in +++ b/contrib/kimchi.spec.fedora.in @@ -33,6 +33,7 @@ Requires: iscsi-initiator-utils Requires: python-ldap Requires: python-libguestfs Requires: libguestfs-tools +Requires: python-magic BuildRequires: libxslt BuildRequires: openssl BuildRequires: python-lxml diff --git a/contrib/kimchi.spec.suse.in b/contrib/kimchi.spec.suse.in index 54228ae..b6f00d4 100644 --- a/contrib/kimchi.spec.suse.in +++ b/contrib/kimchi.spec.suse.in @@ -33,6 +33,7 @@ Requires: nginx Requires: open-iscsi Requires: python-libguestfs Requires: guestfs-tools +Requires: python-magic BuildRequires: libxslt-tools BuildRequires: openssl BuildRequires: python-lxml diff --git a/docs/README.md b/docs/README.md index 034a591..541b4c8 100644 --- a/docs/README.md +++ b/docs/README.md @@ -55,7 +55,7 @@ Install Dependencies python-ipaddr python-ldap python-lxml nfs-utils \ iscsi-initiator-utils libxslt pyparted nginx \ python-libguestfs libguestfs-tools python-websockify \ - novnc spice-html5 python-configobj + novnc spice-html5 python-configobj python-magic # If using RHEL, install the following additional packages: $ sudo yum install python-unittest2 python-ordereddict @@ -87,7 +87,7 @@ channel at RHN Classic or Red Hat Satellite. sosreport python-ipaddr python-ldap \ python-lxml nfs-common open-iscsi lvm2 xsltproc \ python-parted nginx python-guestfs libguestfs-tools \ - websockify novnc spice-html5 + websockify novnc spice-html5 python-magic Packages version requirement: python-jsonschema >= 1.3.0 @@ -106,7 +106,7 @@ channel at RHN Classic or Red Hat Satellite. python-ipaddr python-ldap python-lxml nfs-client \ open-iscsi libxslt-tools python-xml python-parted \ nginx python-libguestfs python-configobj \ - guestfs-tools python-websockify novnc + guestfs-tools python-websockify novnc python-magic Packages version requirement: python-psutil >= 0.6.0 @@ -117,9 +117,8 @@ channel at RHN Classic or Red Hat Satellite. *Note for openSUSE users*: Some of the above packages are located in different openSUSE repositories. See [this FAQ](http://download.opensuse.org/repositories/home:GRNET:synnefo/) for -python-parted; and -[this FAQ](http://download.opensuse.org/repositories/systemsmanagement:/spacewalk/) -for python-ethtool to get the correct repository based on your openSUSE version. And +python-parted, [this FAQ](http://download.opensuse.org/repositories/systemsmanagement:/spacewalk/) +for python-ethtool, and [this FAQ](http://download.opensuse.org/repositories/home:/Simmphonie:/python/) for python-magic to get the correct repository based on your openSUSE version. And [this FAQ](http://en.opensuse.org/SDB:Add_package_repositories) for more information on how configure your system to access this repository. -- 2.4.3
From: Paulo Vital <pvital@linux.vnet.ibm.com> 'Raw' volumes can not be valid image disks (e.g. XML, PDF, TXT are all raw files), so it's necessary check the 'content' of them and return the information to GET requests. Updated front-end to make a better list of volume options on VM's edition screen based on GET response of storage volumes. Updated MockModel to reflect the changes and use it correctly on tests. This patch solves the issue #731. Signed-off-by: Paulo Vital <pvital@linux.vnet.ibm.com> --- docs/API.md | 1 + src/kimchi/control/storagevolumes.py | 3 ++- src/kimchi/mockmodel.py | 10 ++++++++-- src/kimchi/model/storagevolumes.py | 21 ++++++++++++++++++++- src/kimchi/model/vmstorages.py | 4 ++++ ui/js/src/kimchi.guest_storage_add.main.js | 2 +- 6 files changed, 36 insertions(+), 5 deletions(-) diff --git a/docs/API.md b/docs/API.md index 18d15ac..d860513 100644 --- a/docs/API.md +++ b/docs/API.md @@ -511,6 +511,7 @@ A interface represents available network interface on VM. * os_version *(optional)*: os version of the volume, for iso volume only. * bootable *(optional)*: True if iso image is bootable and not corrupted. * used_by: Name of vms which use this volume. + * isvalid: True if is a valid volume. * **DELETE**: Remove the Storage Volume * **POST**: *See Storage Volume Actions* diff --git a/src/kimchi/control/storagevolumes.py b/src/kimchi/control/storagevolumes.py index 8af7abc..d3532f6 100644 --- a/src/kimchi/control/storagevolumes.py +++ b/src/kimchi/control/storagevolumes.py @@ -57,7 +57,8 @@ class StorageVolume(Resource): 'allocation': self.info['allocation'], 'path': self.info['path'], 'used_by': self.info['used_by'], - 'format': self.info['format']} + 'format': self.info['format'], + 'isvalid': self.info['isvalid']} for key in ('os_version', 'os_distro', 'bootable', 'base'): val = self.info.get(key) diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index 34bcfc9..8a7ac58 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -39,6 +39,7 @@ from kimchi.model.libvirtstoragepool import StoragePoolDef from kimchi.model.model import Model from kimchi.model.storagepools import StoragePoolModel from kimchi.model.storagevolumes import StorageVolumeModel, StorageVolumesModel +from kimchi.model import storagevolumes from kimchi.model.templates import LibvirtVMTemplate from kimchi.model.users import PAMUsersModel from kimchi.model.groups import PAMGroupsModel @@ -52,6 +53,9 @@ fake_user = {'root': 'letmein!'} mockmodel_defaults = {'storagepool': '/storagepools/default-pool', 'domain': 'test', 'arch': 'i686'} +storagevolumes.VALID_RAW_CONTENT = ['dos/mbr boot sector', + 'x86 boot sector', + 'data', 'empty'] class MockModel(Model): _mock_vms = {} @@ -471,13 +475,15 @@ class MockStorageVolumes(object): 'allocation': 512, 'type': 'block', 'path': base_path + '1', - 'used_by': []}, + 'used_by': [], + 'isvalid': True}, 'unit:0:0:2': {'capacity': 2048, 'format': 'unknown', 'allocation': 512, 'type': 'block', 'path': base_path + '2', - 'used_by': []}} + 'used_by': [], + 'isvalid': True}} class MockPartitions(object): diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py index 22856ff..75cab9c 100644 --- a/src/kimchi/model/storagevolumes.py +++ b/src/kimchi/model/storagevolumes.py @@ -19,6 +19,7 @@ import contextlib import lxml.etree as ET +import magic import os import tempfile import threading @@ -48,6 +49,10 @@ VOLUME_TYPE_MAP = {0: 'file', READ_CHUNK_SIZE = 1048576 # 1 MiB REQUIRE_NAME_PARAMS = ['capacity'] +VALID_RAW_CONTENT = ['dos/mbr boot sector', + 'x86 boot sector', + 'data'] + upload_volumes = dict() @@ -310,13 +315,27 @@ class StorageVolumeModel(object): else: fmt = 'iso' + # 'raw' volumes can not be valid image disks (e.g. XML, PDF, TXT are + # raw files), so it's necessary check the 'content' of them + isvalid = True + if fmt == 'raw': + try: + ms = magic.open(magic.NONE) + ms.load() + if ms.file(path).lower() not in VALID_RAW_CONTENT: + isvalid = False + ms.close() + except UnicodeDecodeError: + isvalid = False + used_by = get_disk_used_by(self.objstore, self.conn, path) res = dict(type=VOLUME_TYPE_MAP[info[0]], capacity=info[1], allocation=info[2], path=path, used_by=used_by, - format=fmt) + format=fmt, + isvalid=isvalid) if fmt == 'iso': if os.path.islink(path): path = os.path.join(os.path.dirname(path), os.readlink(path)) diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py index 0beaaf4..21d4f54 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -126,6 +126,10 @@ class VMStoragesModel(object): {"format": vol_info['format'], "type": params['type']}) + if (params['format'] == 'raw' and not vol_info['isvalid']): + message = 'This is not a valid RAW disk image.' + raise OperationFailed('KCHVMSTOR0008E', {'error': message}) + params['path'] = vol_info['path'] params['disk'] = vol_info['type'] diff --git a/ui/js/src/kimchi.guest_storage_add.main.js b/ui/js/src/kimchi.guest_storage_add.main.js index 3581c34..323695e 100644 --- a/ui/js/src/kimchi.guest_storage_add.main.js +++ b/ui/js/src/kimchi.guest_storage_add.main.js @@ -81,7 +81,7 @@ kimchi.guest_storage_add_main = function() { if (result.length) { $.each(result, function(index, value) { // Only unused volume can be attached - if (value.used_by.length == 0 && (value.type != 'file' || validVolType[selectType].test(value.format))) { + if (value.used_by.length == 0 && value.isvalid && (value.type != 'file' || validVolType[selectType].test(value.format))) { options.push({ label: value.name, value: value.name -- 2.4.3
From: Paulo Vital <pvital@linux.vnet.ibm.com> Update tests to handle the changes in new 'raw' volume validation. Signed-off-by: Paulo Vital <pvital@linux.vnet.ibm.com> --- tests/test_model_storagevolume.py | 2 +- tests/test_rest.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_model_storagevolume.py b/tests/test_model_storagevolume.py index 5e76d3d..82c38e2 100644 --- a/tests/test_model_storagevolume.py +++ b/tests/test_model_storagevolume.py @@ -253,7 +253,7 @@ class StorageVolumeTests(unittest.TestCase): self.assertEquals(200, resp.status) keys = [u'name', u'type', u'capacity', u'allocation', u'path', - u'used_by', u'format'] + u'used_by', u'format', u'isvalid'] for vol in json.loads(resp.read()): resp = self.request(uri + '/' + vol['name']) self.assertEquals(200, resp.status) diff --git a/tests/test_rest.py b/tests/test_rest.py index 7e726b0..6c5c96d 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -594,6 +594,7 @@ class RestTests(unittest.TestCase): self.assertEquals(200, resp.status) # 'name' is required for this type of volume + open('/tmp/attach-volume', 'w').close() req = json.dumps({'capacity': 1024, 'allocation': 512, 'type': 'disk', @@ -659,6 +660,7 @@ class RestTests(unittest.TestCase): rollback.prependDefer(self.request, '/vms/test-vm/storages/hdx', '{}', 'DELETE') os.remove('/tmp/existent.iso') + os.remove('/tmp/attach-volume') # Change path of storage cdrom cdrom = u'http://mirrors.kernel.org/fedora/releases/21/Live/'\ -- 2.4.3
participants (2)
-
Aline Manera -
pvital＠linux.vnet.ibm.com