26 Feb
2014
26 Feb
'14
1:26 p.m.
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@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@linux.vnet.ibm.com>
IBM Linux Technology Center