[Kimchi-devel] [PATCH] Don't allow templates to be created with ISOs that won't be usable.

[Aline Manera]

The tests are failing with this patch set

======================================================================
ERROR: test_template_integrity (test_model.ModelTests)
----------------------------------------------------------------------
Traceback (most recent call last):
   File "/home/alinefm/kimchi/tests/test_model.py", line 477, in 
test_template_integrity
     info = inst.template_lookup('test')
   File "/home/alinefm/kimchi/src/kimchi/model/templates.py", line 86, 
in lookup
     t = self.get_template(name, self.objstore, self.conn)
   File "/home/alinefm/kimchi/src/kimchi/model/templates.py", line 83, 
in get_template
     return LibvirtVMTemplate(params, False, conn)
   File "/home/alinefm/kimchi/src/kimchi/model/templates.py", line 143, 
in __init__
     VMTemplate.__init__(self, args, scan)
   File "/home/alinefm/kimchi/src/kimchi/vmtemplate.py", line 59, in 
__init__
     not check_iso_path_perm(os.path.dirname(iso)):
   File "/home/alinefm/kimchi/src/kimchi/utils.py", line 244, in 
check_iso_path_perm
     return os.stat(path).st_mode & stat.S_IXOTH and \
OSError: [Errno 2] No such file or directory: '/tmp/kimchi-iso'

----------------------------------------------------------------------
Ran 157 tests in 129.686s

FAILED (errors=1)
[25/Feb/2014:11:28:02] ENGINE Waiting for child threads to terminate...
make[3]: *** [check-local] Error 1
make[3]: Leaving directory `/home/alinefm/kimchi/tests'
make[2]: *** [check-am] Error 2
make[2]: Leaving directory `/home/alinefm/kimchi/tests'
make[1]: *** [check] Error 2
make[1]: Leaving directory `/home/alinefm/kimchi/tests'
make: *** [check-recursive] Error 1


On 02/24/2014 09:23 PM, Christy Perez wrote:
> qemu can't use an ISO if 'other' can't execute all the directories
> containing the file. Return an error explaning this, and suggest
> the way to fix it.
>
> Signed-off-by: Christy Perez <christy at linux.vnet.ibm.com>
> ---
>   po/en_US.po              | 6 ++++++
>   src/kimchi/exception.py  | 4 ++++
>   src/kimchi/i18n.py       | 2 ++
>   src/kimchi/utils.py      | 9 +++++++++
>   src/kimchi/vmtemplate.py | 7 +++++--
>   5 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/po/en_US.po b/po/en_US.po
> index ca1fe86..38af64c 100644
> --- a/po/en_US.po
> +++ b/po/en_US.po
> @@ -674,6 +674,12 @@ msgid "Bad format while reading volume descriptor in ISO %(filename)s"
>   msgstr ""
>
>   #, python-format
> +msgid "The hypervisor doesn't have permission to use this ISO %(filename)s. \
> +                     Consider moving it under /var/lib/libvirt, or \
> +                     (not recommended) 'chmod -R o+x 'path_to_iso'."
> +msgstr ""
> +
> +#, python-format
>   msgid "Virtual machine %(name)s already exists"
>   msgstr ""
>
> diff --git a/src/kimchi/exception.py b/src/kimchi/exception.py
> index 2d974a8..263bd0c 100644
> --- a/src/kimchi/exception.py
> +++ b/src/kimchi/exception.py
> @@ -75,6 +75,10 @@ class InvalidParameter(KimchiException):
>       pass
>
>
> +class InvalidPermission(KimchiException):
> +    pass
> +
> +
>   class InvalidOperation(KimchiException):
>       pass
>
> diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
> index fea0184..05b347c 100644
> --- a/src/kimchi/i18n.py
> +++ b/src/kimchi/i18n.py
> @@ -57,6 +57,8 @@ 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 (not recommended) 'chmod -R o+x 'path_to_iso'." ),
>
>       "KCHVM0001E": _("Virtual machine %(name)s already exists"),
>       "KCHVM0002E": _("Virtual machine %(name)s does not exist"),
> diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py
> index 6be1c04..c325be3 100644
> --- a/src/kimchi/utils.py
> +++ b/src/kimchi/utils.py
> @@ -27,6 +27,7 @@ import psutil
>   import re
>   import subprocess
>   import urllib2
> +import stat
>   from threading import Timer
>
>   from cherrypy.lib.reprconf import Parser
> @@ -234,3 +235,11 @@ def run_setfacl_set_attr(path, attr="r", user=""):
>       set_user = ["setfacl", "--modify", "user:%s:%s" % (user, attr), path]
>       out, error, ret = run_command(set_user)
>       return ret == 0
> +
> +def check_iso_path_perm(path):
> +    """
> +    libvirt requires that all parent dirs have o+x
> +    """
> +    if path == '/': return True
> +    return os.stat(path).st_mode & stat.S_IXOTH and \
> +                            check_iso_path_perm(os.path.dirname(path))
> diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py
> index af07ee3..76ac772 100644
> --- a/src/kimchi/vmtemplate.py
> +++ b/src/kimchi/vmtemplate.py
> @@ -29,9 +29,9 @@ import urlparse
>
>   from kimchi import isoinfo
>   from kimchi import osinfo
> -from kimchi.exception import InvalidParameter, IsoFormatError
> +from kimchi.exception import InvalidParameter, IsoFormatError, InvalidPermission
>   from kimchi.isoinfo import IsoImage
> -
> +from kimchi.utils import check_iso_path_perm
>
>   QEMU_NAMESPACE = "xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'"
>
> @@ -55,6 +55,9 @@ class VMTemplate(object):
>           iso_distro = iso_version = 'unknown'
>           iso = args.get('cdrom', '')
>
> +        if iso.startswith('/') and \
> +                not check_iso_path_perm(os.path.dirname(iso)):
> +            raise InvalidPermission("KCHISO0008E", {'filename': iso})
>           if scan and len(iso) > 0:
>               iso_distro, iso_version = self.get_iso_info(iso)
>               if not iso.startswith('/'):