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

[Aline Manera]

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?

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