<html>
  <head>
    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <div class="moz-cite-prefix">On 2014年03月10日 09:52, Sheldon wrote:<br>
    </div>
    <blockquote cite="mid:531D1AF8.9070002@linux.vnet.ibm.com"
      type="cite">
      <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
      <div class="moz-cite-prefix">On 03/08/2014 04:20 AM, Christy Perez
        wrote:<br>
      </div>
      <blockquote cite="mid:1394223617.4878.215.camel@christy-toshiba"
        type="cite">
        <pre wrap="">Sorry this took me a while to get back to you. Answers below.

Do you want to pick this back up in the overall kimchi thread?


On Thu, 2014-03-06 at 13:29 +0800, Sheldon wrote:
</pre>
        <blockquote type="cite">
          <pre wrap="">On 03/04/2014 06:38 AM, Christy Perez wrote:
</pre>
          <blockquote type="cite">
            <pre wrap="">On Wed, 2014-02-26 at 20:26 +0800, Sheldon wrote:
</pre>
            <blockquote type="cite">
              <pre wrap="">On 02/25/2014 08:23 AM, Christy Perez wrote:
</pre>
              <blockquote type="cite">
                <pre wrap="">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 <a moz-do-not-send="true" class="moz-txt-link-rfc2396E" href="mailto:christy@linux.vnet.ibm.com">&lt;christy@linux.vnet.ibm.com&gt;</a>
---
   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 "<a moz-do-not-send="true" class="moz-txt-link-rfc2396E" href="mailto:diff--gita/src/kimchi/exception.pyb/src/kimchi/exception.pyindex2d974a8..263bd0c100644---a/src/kimchi/exception.py+++b/src/kimchi/exception.py@@-75,6+75,10@@classInvalidParameter%28KimchiException%29:pass+classInvalidPermission%28KimchiException%29:+pass++classInvalidOperation%28KimchiException%29:passdiff--gita/src/kimchi/i18n.pyb/src/kimchi/i18n.pyindexfea0184..05b347c100644---a/src/kimchi/i18n.py+++b/src/kimchi/i18n.py@@-57,6+57,8@@messages=%7B">"

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 = {
       "</a>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<a moz-do-not-send="true" class="moz-txt-link-rfc2396E" href="mailto:%29,diff--gita/src/kimchi/utils.pyb/src/kimchi/utils.pyindex6be1c04..c325be3100644---a/src/kimchi/utils.py+++b/src/kimchi/utils.py@@-27,6+27,7@@importpsutilimportreimportsubprocessimporturllib2+importstatfromthreadingimportTimerfromcherrypy.lib.reprconfimportParser@@-234,3+235,11@@defrun_setfacl_set_attr%28path,attr=">"),
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="</a>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 &amp; stat.S_IXOTH and \
+                            check_iso_path_perm(os.path.dirname(path))
</pre>
              </blockquote>
              <pre wrap="">just test other's permission is not enough.
1. when the path user is qemu. why we need S_IXOTH?
</pre>
            </blockquote>
            <pre wrap="">That's what the libvirt doc says, and what I've found is needed: " The
directories containing disk images must either have their ownership set
to match the user/group configured for QEMU, or their UNIX file
permissions must have the 'execute/search' bit enabled for 'others'. "
Source: <a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://libvirt.org/drvqemu.html">http://libvirt.org/drvqemu.html</a>
</pre>
          </blockquote>
          <pre wrap="">so that's means the disk images without S_IXOTH, qemu still can have 
search permissions right?
</pre>
        </blockquote>
        <pre wrap="">Yep.
The images themselves won't have S_IXOTH.  qemu will own them, though:

[root@kop4 kimchi-ginger-pkvm]# ls -lah /var/lib | grep libvirt
drwxr-xr-x.  9 root   root    4.0K Feb 19 11:16 libvirt
[root@kop4 kimchi-ginger-pkvm]# ls -lah /var/lib/libvirt | grep images
drwx--x--x.  2 root root 4.0K Mar  7 06:22 images
[root@kop4 kimchi-ginger-pkvm]# ls -lah /var/lib/libvirt/images/
total 2.9G
drwx--x--x. 2 root root 4.0K Mar  7 06:22 .
drwxr-xr-x. 9 root root 4.0K Feb 19 11:16 ..
-rw-------. 1 qemu qemu 193K Mar  7 06:22
2f7130eb-539c-4401-82eb-cccd18708f56-0.img
-r--r--r--. 1 qemu qemu 2.9G Mar  7 06:20
RHEL-7.0-20140226.0-Server-ppc64-dvd1.iso</pre>
      </blockquote>
    </blockquote>
    I think Sheldon means besides condition  of others have x of ISO's
    ancestor directories,  directory owner/group is qemu with x access
    can also guarantee qemu access ISO directoy success.<br>
    <br>
        directory owner==qemu AND qemu can x OR directory group==qemu
    AND qemu can x OR other can x =&gt; qemu can access the iso file
    directory(check should pass)<br>
    <br>
    So based on these complex check personally I favour sudo 'qemu' and
    try open iso file to validate access. It is simple and avoid
    handling corner case.<br>
    <br>
    <blockquote cite="mid:531D1AF8.9070002@linux.vnet.ibm.com"
      type="cite">
      <blockquote cite="mid:1394223617.4878.215.camel@christy-toshiba"
        type="cite">
        <pre wrap="">

</pre>
        <blockquote type="cite">
          <pre wrap="">o+x is  sufficient but not necessary condition?
</pre>
        </blockquote>
        <pre wrap="">o+x is necessary.
</pre>
        <blockquote type="cite">
          <blockquote type="cite">
            <blockquote type="cite">
              <pre wrap="">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?
</pre>
            </blockquote>
            <pre wrap="">I don't think the ACL will be enough. It looks like it has to have o+x
for all parent paths. And yes, my ACLs were enabled on my filesystem.
</pre>
          </blockquote>
          <pre wrap="">That's strangely.  but the docs  <a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://libvirt.org/drvqemu.html">http://libvirt.org/drvqemu.html</a>
says:
it simple to set o+x,  means this is  the only way.
</pre>
        </blockquote>
        <pre wrap="">Yes, I think so. </pre>
      </blockquote>
      it is strange from what I test, it seems o+x is  not necessary but
      sufficient. <br>
      I really want to know why o+x is necessary and why it is the only
      way <br>
      to check the permission. <br>
      <br>
      I have try 5 test case with kimchi.<br>
      <br>
      1. There's a iso on my laptop, with o+x permission <br>
      $ ls /home/shhfeng/iso/RHEL6.4-20130123.0-Server-x86_64-DVD1.iso <br>
      /home/shhfeng/iso/RHEL6.4-20130123.0-Server-x86_64-DVD1.iso <br>
      $ ls <i class="moz-txt-slash"><span class="moz-txt-tag">/</span>home/shhfeng<span
          class="moz-txt-tag">/</span></i> -l |grep iso <br>
      drwxrwxrwx.  4 shhfeng shhfeng      4096 Oct  3 00:22 iso <br>
      <br>
      I can create a vm and start is successfully. <br>
      <br>
      <br>
      2. Now I remove x permission <br>
      $ sudo chmod o-x /home/shhfeng/iso <br>
      $ ls <i class="moz-txt-slash"><span class="moz-txt-tag">/</span>home/shhfeng<span
          class="moz-txt-tag">/</span></i> -l |grep iso <br>
      drwxrwxrw-.  4 shhfeng shhfeng      4096 Oct  3 00:22 iso <br>
      <br>
      I failed to start it for permission. <br>
      <br>
      3. I change the own of this iso, still without x permission. <br>
      $ sudo chown qemu:qemu /home/shhfeng/iso <br>
      $ ls <i class="moz-txt-slash"><span class="moz-txt-tag">/</span>home/shhfeng<span
          class="moz-txt-tag">/</span></i> -l |grep iso <br>
      drwxrwxrw-.  4 qemu    qemu         4096 Oct  3 00:22 iso <br>
      <br>
      Now I can start it successfully. <br>
      <br>
      <br>
      4. just set the qemu user as search permission. <br>
      $ sudo chown shhfeng:shhfeng <i class="moz-txt-slash"><span
          class="moz-txt-tag">/</span>home/shhfeng/iso<span
          class="moz-txt-tag">/</span></i> <br>
      $ ls -l <i class="moz-txt-slash"><span class="moz-txt-tag">/</span>home/shhfeng<span
          class="moz-txt-tag">/</span></i> |grep iso <br>
      drwxrwxrw-.  4 shhfeng shhfeng      4096 Oct  3 00:22 iso <br>
      $ setfacl -m u:qemu:x /home/shhfeng/iso <br>
      $ getfacl /home/shhfeng/iso <br>
      getfacl: Removing leading '/' from absolute path names <br>
      # file: home/shhfeng/iso <br>
      # owner: shhfeng <br>
      # group: shhfeng <br>
      user::rwx <br>
      user:qemu:--x <br>
      group::rwx <br>
      mask::rwx <br>
      other::rw- <br>
      <br>
      Now I can also start it successfully. <br>
      <br>
      <br>
      5. remove qemu user as search permission, <br>
      add set the 'kvm' group to the path, qemu is in "kvm" group <br>
      $ setfacl -x u:qemu /home/shhfeng/iso <br>
      $ groups qemu <br>
      qemu : qemu kvm <br>
      $ sudo chown shhfeng:kvm <i class="moz-txt-slash"><span
          class="moz-txt-tag">/</span>home/shhfeng/iso<span
          class="moz-txt-tag">/</span></i> <br>
      <br>
      Now I can also start it successfully. <br>
      <br>
      <br>
      only case 2, the kimchi deny to start vm for permission. <br>
      <br>
      <br>
      <br>
      I write a python script checkperm.py <br>
      It is the same result of libvirt run qemu. you can have a try. <br>
      If others do like like checkperm.py, <br>
      I think we can probe the permission by let libvirt to start the
      guest with the iso. <br>
      <br>
      $ sudo python checkperm.py
      /home/shhfeng/iso/RHEL6.4-20130123.0-Server-x86_64-DVD1.iso <br>
      <br>
      # checkperm.py <br>
        1 from multiprocessing import Process, Queue <br>
        2 import sys <br>
        3 import grp <br>
        4 import os <br>
        5 import pwd <br>
        6 <br>
        7 <br>
        8 def probe_file_permission_as_user(file, user): <br>
        9     def probe_permission(q, file, user): <br>
       10         uid = pwd.getpwnam(user).pw_uid <br>
       11         gid = pwd.getpwnam(user).pw_gid <br>
       12         gids = [g.gr_gid for g in grp.getgrall() if user in
      g.gr_mem] <br>
       13         os.setgid(gid) <br>
       14         os.setgroups(gids) <br>
       15         os.setuid(uid) <br>
       16         try: <br>
       17             with open(file): <br>
       18                 q.put(True) <br>
       19         except Exception: <br>
       20             q.put(False) <br>
       21 <br>
       22     queue = Queue() <br>
       23     p = Process(target=probe_permission, args=(queue, file,
      user)) <br>
       24     p.start() <br>
       25     p.join() <br>
       26     return queue.get() <br>
       27 <br>
       28 if __name__ == '__main__': <br>
       29     print probe_file_permission_as_user(sys.argv[1], 'qemu')
      <blockquote cite="mid:1394223617.4878.215.camel@christy-toshiba"
        type="cite">
        <blockquote type="cite">
          <pre wrap="">"""
The simplest option is the latter one, of just enabling the 
'execute/search' bit. For any directory to be used for storing disk 
images, this can be achieved by running the following command on the 
directory itself, and any parent directories

chmod o+x /path/to/directory

In particular note that if using the "system" instance and attempting to 
store disk images in a user home directory, the default permissions on 
$HOME are typically too restrictive to allow access.
"""


</pre>
          <blockquote type="cite">
            <blockquote type="cite">
              <pre wrap="">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
</pre>
            </blockquote>
            <pre wrap="">I don't see how the patch listed there will apply. Maybe github
reformats it strangely when you paste it in?

Even so, as I pasted from the libvirt doc, I don't think that will be
enough to make libvirt happy.
</pre>
          </blockquote>
          <pre wrap="">save as, and git apply. but I think it will not successfully, for the 
upstream changes so quickly.
Yes,  we should make libvirt and kimchi user happy?
</pre>
        </blockquote>
        <pre wrap="">I think it's a security leak to allow someone to put a file in one
directory, and then we give o+x to every parent of it. If I put an ISO
at /home/christy/example.iso and kimchi changed recursively all parents
to o+x, it would give all users the ability to see what's in /home/. 
</pre>
        <blockquote type="cite">
          <blockquote type="cite">
            <blockquote type="cite">
              <blockquote type="cite">
                <pre wrap="">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='<a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://libvirt.org/schemas/domain/qemu/1.0">http://libvirt.org/schemas/domain/qemu/1.0</a>'"

@@ -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) &gt; 0:
               iso_distro, iso_version = self.get_iso_info(iso)
               if not iso.startswith('/'):
</pre>
              </blockquote>
            </blockquote>
            <pre wrap="">Regards,

- Christy

</pre>
          </blockquote>
          <pre wrap="">
</pre>
        </blockquote>
        <pre wrap="">Regards,

- Christy

</pre>
      </blockquote>
      <br>
      <br>
      <pre class="moz-signature" cols="72">-- 
Thanks and best regards!

Sheldon Feng(冯少合)<a moz-do-not-send="true" class="moz-txt-link-rfc2396E" href="mailto:shaohef@linux.vnet.ibm.com">&lt;shaohef@linux.vnet.ibm.com&gt;</a>
IBM Linux Technology Center</pre>
    </blockquote>
    <br>
  </body>
</html>