[Kimchi-devel] [PATCH 1/3] Remove ISO permission check when creating a template

[Aline Manera]

As we have talked offline, we will keep using this check but I agree it 
needs to be improved by calling setfacl before checking the file 
permissions.

On 18/03/2015 23:51, Crístian Viana wrote:
> In some cases, the permission check was not allowing access to an image
> when the image was perfectly readable by libvirt.
>
> Signed-off-by: Crístian Viana <vianac at linux.vnet.ibm.com>
> ---
>   src/kimchi/i18n.py            |  5 ---
>   src/kimchi/kvmusertests.py    | 75 -------------------------------------------
>   src/kimchi/model/templates.py | 12 -------
>   3 files changed, 92 deletions(-)
>   delete mode 100644 src/kimchi/kvmusertests.py
>
> diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
> index df5422f..a43c72d 100644
> --- a/src/kimchi/i18n.py
> +++ b/src/kimchi/i18n.py
> @@ -62,11 +62,6 @@ messages = {
>       "KCHISO0005E": _("Invalid El Torito boot indicator in ISO %(filename)s"),
>       "KCHISO0006E": _("Unexpected volume type for primary volume in ISO %(filename)s"),
>       "KCHISO0007E": _("Bad format while reading volume descriptor in ISO %(filename)s"),
> -    "KCHISO0008E": _("The hypervisor doesn't have permission to use this ISO %(filename)s. "
> -                     "Consider moving it under /var/lib/libvirt,  or set the search permission "
> -                     "to file access control lists for '%(user)s' user if possible, or add the "
> -                     "'%(user)s' to the ISO path group, or (not recommended) 'chmod -R o+x 'path_to_iso'."
> -                     "Details: %(err)s" ),
>   
>       "KCHIMG0001E": _("An error occurred when probing image OS information."),
>       "KCHIMG0002E": _("No OS information found in given image."),
> diff --git a/src/kimchi/kvmusertests.py b/src/kimchi/kvmusertests.py
> deleted file mode 100644
> index 37a80d7..0000000
> --- a/src/kimchi/kvmusertests.py
> +++ /dev/null
> @@ -1,75 +0,0 @@
> -# Project Kimchi
> -#
> -# Copyright IBM, Corp. 2014-2015
> -#
> -# This library is free software; you can redistribute it and/or
> -# modify it under the terms of the GNU Lesser General Public
> -# License as published by the Free Software Foundation; either
> -# version 2.1 of the License, or (at your option) any later version.
> -#
> -# This library is distributed in the hope that it will be useful,
> -# but WITHOUT ANY WARRANTY; without even the implied warranty of
> -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -# Lesser General Public License for more details.
> -#
> -# You should have received a copy of the GNU Lesser General Public
> -# License along with this library; if not, write to the Free Software
> -# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> -
> -import platform
> -import psutil
> -
> -import libvirt
> -
> -from kimchi.rollbackcontext import RollbackContext
> -
> -KVMUSERTEST_VM_NAME = "KVMUSERTEST_VM"
> -
> -
> -class UserTests(object):
> -    SIMPLE_VM_XML = """
> -    <domain type='kvm'>
> -      <name>%(name)s</name>
> -      <memory unit='KiB'>262144</memory>
> -      <os>
> -        <type arch='%(arch)s'>hvm</type>
> -        <boot dev='hd'/>
> -      </os>
> -    </domain>"""
> -    user = None
> -
> -    @classmethod
> -    def probe_user(cls):
> -        if cls.user:
> -            return cls.user
> -
> -        arch = 'ppc64' if platform.machine() == 'ppc64le' \
> -            else platform.machine()
> -
> -        xml = cls.SIMPLE_VM_XML % {'name': KVMUSERTEST_VM_NAME, 'arch': arch}
> -
> -        with RollbackContext() as rollback:
> -            conn = libvirt.open(None)
> -            rollback.prependDefer(conn.close)
> -            dom = conn.createXML(xml,
> -                                 flags=libvirt.VIR_DOMAIN_START_AUTODESTROY)
> -            rollback.prependDefer(dom.destroy)
> -            filename = '/var/run/libvirt/qemu/%s.pid' % KVMUSERTEST_VM_NAME
> -            with open(filename) as f:
> -                pidStr = f.read()
> -            p = psutil.Process(int(pidStr))
> -
> -            # bug fix #357
> -            # in psutil 2.0 and above versions, username will be a method,
> -            # not a string
> -            if callable(p.username):
> -                cls.user = p.username()
> -            else:
> -                cls.user = p.username
> -
> -        return cls.user
> -
> -
> -if __name__ == '__main__':
> -    ut = UserTests()
> -    print ut.probe_user()
> diff --git a/src/kimchi/model/templates.py b/src/kimchi/model/templates.py
> index e91636b..df0b551 100644
> --- a/src/kimchi/model/templates.py
> +++ b/src/kimchi/model/templates.py
> @@ -18,16 +18,13 @@
>   # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
>   
>   import copy
> -import os
>   
>   import libvirt
>   
>   from kimchi.exception import InvalidOperation, InvalidParameter
>   from kimchi.exception import NotFoundError, OperationFailed
> -from kimchi.kvmusertests import UserTests
>   from kimchi.model.cpuinfo import CPUInfoModel
>   from kimchi.utils import pool_name_from_uri
> -from kimchi.utils import probe_file_permission_as_user
>   from kimchi.vmtemplate import VMTemplate
>   from kimchi.xmlutils.utils import xpath_get_text
>   
> @@ -39,15 +36,6 @@ class TemplatesModel(object):
>   
>       def create(self, params):
>           name = params.get('name', '').strip()
> -        iso = params.get('cdrom')
> -        # check search permission
> -        if iso and iso.startswith('/') and os.path.isfile(iso):
> -            user = UserTests().probe_user()
> -            ret, excp = probe_file_permission_as_user(iso, user)
> -            if ret is False:
> -                raise InvalidParameter('KCHISO0008E',
> -                                       {'filename': iso, 'user': user,
> -                                        'err': excp})
>   
>           cpu_info = params.get('cpu_info')
>           if cpu_info: