Hi Paulo,
I understood your approach and think that it works and will fix the issue.
However, I would like to make some suggestions/questions:
1- Does ['dos/mbr boot sector', 'data'] work for all types of images
supported by Kimchi ?
2- Could ['dos/mbr boot sector', 'data'] be defined as global/constant ?
3- Instead of return and check "isvalid" for each file/image inside the
pool, I think a possible approach would be
just don't return it in the API. Then the traffic and the UI checking
are lesser.
What do you think ?
Rodrigo Trujillo
On 09/15/2015 02:30 PM, pvital(a)linux.vnet.ibm.com wrote:
From: Paulo Vital <pvital(a)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
pass the information to front-end to make a better list of volume
options on VM's edition screen.
This solves the issue #731.
Signed-off-by: Paulo Vital <pvital(a)linux.vnet.ibm.com>
---
docs/API.md | 1 +
src/kimchi/control/storagevolumes.py | 3 ++-
src/kimchi/mockmodel.py | 6 ++++--
src/kimchi/model/storagevolumes.py | 15 ++++++++++++++-
ui/js/src/kimchi.guest_storage_add.main.js | 2 +-
5 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/docs/API.md b/docs/API.md
index 18d15ac..e94e69a 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: 1 if it's a valid volume, 0 otherwise.
* **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..33115ef 100644
--- a/src/kimchi/mockmodel.py
+++ b/src/kimchi/mockmodel.py
@@ -471,13 +471,15 @@ class MockStorageVolumes(object):
'allocation': 512,
'type': 'block',
'path': base_path + '1',
- 'used_by': []},
+ 'used_by': [],
+ 'isvalid': 1},
'unit:0:0:2': {'capacity': 2048,
'format': 'unknown',
'allocation': 512,
'type': 'block',
'path': base_path + '2',
- 'used_by': []}}
+ 'used_by': [],
+ 'isvalid': 1}}
class MockPartitions(object):
diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py
index 22856ff..27d4a4e 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
@@ -310,13 +311,25 @@ 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 = 1
+ valid_format_content = ['dos/mbr boot sector', 'data']
+ if fmt == 'raw':
+ ms = magic.open(magic.NONE)
+ ms.load()
+ if ms.file(path).lower() not in valid_format_content:
+ isvalid = 0
+ ms.close()
+
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/ui/js/src/kimchi.guest_storage_add.main.js
b/ui/js/src/kimchi.guest_storage_add.main.js
index 3581c34..9a03656 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 == 1
&& (value.type != 'file' || validVolType[selectType].test(value.format)))
{
options.push({
label: value.name,
value: value.name