[Kimchi-devel] [PATCH] [Kimchi] Check if qemu/libvirt user has permission to use an ISO

[joserz at linux.vnet.ibm.com]

On Thu, May 12, 2016 at 10:25:06AM -0300, Aline Manera wrote:
> 
> Hi Ziviani,
> 
> Although this feature was designed thinking about ISO files it may be useful
> for Image files as well.
> So I'd suggest to add the has_permission parameter to any storage volume.
> 
> What do you think about it?

Sure, I agree with you. Do not consider this patch because I'm going to
set a v2 with such change.

thank you

> 
> Regards,
> Aline Manera
> 
> On 05/11/2016 12:44 PM, Jose Ricardo Ziviani wrote:
> >  - Based on this check this patch returns a new field to the
> >    interface be able to disable such ISO before creating any
> >    templates with it.
> >
> >Signed-off-by: Jose Ricardo Ziviani <joserz at linux.vnet.ibm.com>
> >---
> >  control/storagevolumes.py         | 3 +++
> >  docs/API.md                       | 2 ++
> >  model/storagevolumes.py           | 9 +++++++--
> >  tests/test_model_storagevolume.py | 3 ++-
> >  tests/test_rest.py                | 1 +
> >  5 files changed, 15 insertions(+), 3 deletions(-)
> >
> >diff --git a/control/storagevolumes.py b/control/storagevolumes.py
> >index a04ad35..08cd989 100644
> >--- a/control/storagevolumes.py
> >+++ b/control/storagevolumes.py
> >@@ -90,6 +90,9 @@ class StorageVolume(Resource):
> >              if val:
> >                  res[key] = val
> >
> >+        if self.info['format'] == 'iso':
> >+            res['has_permission'] = self.info.get('has_permission', False)
> >+
> >          return res
> >
> >
> >diff --git a/docs/API.md b/docs/API.md
> >index 9d42c53..6afa6e4 100644
> >--- a/docs/API.md
> >+++ b/docs/API.md
> >@@ -613,6 +613,8 @@ A interface represents available network interface on VM.
> >      * 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.
> >+    * has_permission *(optional)*: qemu/libvirt user has the right permission to
> >+                                 to use the image
> >
> >  * **DELETE**: Remove the Storage Volume
> >  * **POST**: *See Storage Volume Actions*
> >diff --git a/model/storagevolumes.py b/model/storagevolumes.py
> >index e037e35..9d4ac43 100644
> >--- a/model/storagevolumes.py
> >+++ b/model/storagevolumes.py
> >@@ -30,12 +30,14 @@ from lxml.builder import E
> >
> >  from wok.exception import InvalidOperation, InvalidParameter, IsoFormatError
> >  from wok.exception import MissingParameter, NotFoundError, OperationFailed
> >-from wok.utils import add_task, get_unique_file_name, wok_log
> >+from wok.utils import add_task, get_unique_file_name
> >+from wok.utils import probe_file_permission_as_user, wok_log
> >  from wok.xmlutils.utils import xpath_get_text
> >  from wok.model.tasks import TaskModel
> >
> >  from wok.plugins.kimchi.config import READONLY_POOL_TYPE
> >  from wok.plugins.kimchi.isoinfo import IsoImage
> >+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
> >@@ -347,9 +349,12 @@ class StorageVolumeModel(object):
> >                  bootable = True
> >              except IsoFormatError:
> >                  bootable = False
> >+
> >+            user = UserTests().probe_user()
> >+            ret, _ = probe_file_permission_as_user(path, user)
> >              res.update(
> >                  dict(os_distro=os_distro, os_version=os_version, path=path,
> >-                     bootable=bootable))
> >+                     bootable=bootable, has_permission=ret))
> >          return res
> >
> >      def wipe(self, pool, name):
> >diff --git a/tests/test_model_storagevolume.py b/tests/test_model_storagevolume.py
> >index 7dbda97..bb7758f 100644
> >--- a/tests/test_model_storagevolume.py
> >+++ b/tests/test_model_storagevolume.py
> >@@ -266,7 +266,8 @@ class StorageVolumeTests(unittest.TestCase):
> >              all_keys = keys[:]
> >              vol_info = json.loads(resp.read())
> >              if vol_info['format'] == 'iso':
> >-                all_keys.extend([u'os_distro', u'os_version', u'bootable'])
> >+                all_keys.extend([u'os_distro', u'os_version', u'bootable',
> >+                                 u'has_permission'])
> >
> >              self.assertEquals(sorted(all_keys), sorted(vol_info.keys()))
> >
> >diff --git a/tests/test_rest.py b/tests/test_rest.py
> >index b73d16c..516a38b 100644
> >--- a/tests/test_rest.py
> >+++ b/tests/test_rest.py
> >@@ -1174,6 +1174,7 @@ class RestTests(unittest.TestCase):
> >          self.assertEquals('17', storagevolume['os_version'])
> >          self.assertEquals('fedora', storagevolume['os_distro'])
> >          self.assertEquals(True, storagevolume['bootable'])
> >+        self.assertEquals(True, storagevolume['has_permission'])
> >
> >          # Create a template
> >          # In real model os distro/version can be omitted
>