[Kimchi-devel] [PATCH 2/3 V3] Raw volumes validation: back-end and front-end

pvital at linux.vnet.ibm.com pvital at linux.vnet.ibm.com
Thu Sep 17 20:45:43 UTC 2015


From: Paulo Vital <pvital at 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 at 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




More information about the Kimchi-devel mailing list