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

[Sheldon]

On 02/25/2014 08:23 AM, 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))
just test other's permission is not enough.
1. when the path user is qemu. why we need S_IXOTH?
2. as you said "libvirt requires that all parent dirs have o+x"
you has adding 'X' to the ACL for each directory to try it.
I wander have check your ACL enable?
3. have you check add qemu in all the paths group?
and open set g+x?


can you try my patch, seen it can works?
add a method to probe the permission as qemu user
> 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('/'):


-- 
Thanks and best regards!

Sheldon Feng(冯少合)<shaohef at linux.vnet.ibm.com>
IBM Linux Technology Center