[PATCH 0/3] Fix permission of cdrom files

From: Aline Manera <alinefm@br.ibm.com> It is a continuation of Sheldon's work. What I added: - Run setfacl to add read permission to user who own the file - Fix permission of cdrom file before starting the virtual machine Aline Manera (2): Remove kvmusertests.py Make sure ISO files have read permission while starting VM ShaoHe Feng (1): Add a method to fix read permission on ISO files src/kimchi/kvmusertests.py | 66 -------------------------------------------- src/kimchi/model/vms.py | 10 ++++++- src/kimchi/utils.py | 9 ++++-- 3 files changed, 16 insertions(+), 69 deletions(-) delete mode 100644 src/kimchi/kvmusertests.py -- 1.7.10.4

From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> When starting a virtual machine, libvirt changes the user:group of the disk files to libvirt-qemu or qemu (depends on Linux distribution) That way libvirt can work properly with those files. But if the file does not have read permission, libvirt won't be able to access them. This patch adds a method to add read permission on files to avoiding problems when starting a virtual machine. Also remove useless imports from utils.py Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> Signed-off-by: Aline Manera <alinefm@linux.vnet.ibm.com> Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/utils.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py index 3d8f961..8795957 100644 --- a/src/kimchi/utils.py +++ b/src/kimchi/utils.py @@ -33,9 +33,8 @@ from kimchi.config import paths, PluginPaths from kimchi.exception import TimeoutExpired -from kimchi import config from kimchi.asynctask import AsyncTask -from kimchi.exception import InvalidParameter, TimeoutExpired +from kimchi.exception import InvalidParameter kimchi_log = cherrypy.log.error_log @@ -220,3 +219,9 @@ def listPathModules(path): if ext in ('.py', '.pyc', '.pyo'): modules.add(base) return sorted(modules) + + +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 -- 1.7.10.4

Reviewed-by: Daniel Barboza <danielhb@linux.vnet.ibm.com> On 02/03/2014 01:01 PM, Aline Manera wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
When starting a virtual machine, libvirt changes the user:group of the disk files to libvirt-qemu or qemu (depends on Linux distribution) That way libvirt can work properly with those files.
But if the file does not have read permission, libvirt won't be able to access them.
This patch adds a method to add read permission on files to avoiding problems when starting a virtual machine.
Also remove useless imports from utils.py
Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> Signed-off-by: Aline Manera <alinefm@linux.vnet.ibm.com> Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/utils.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py index 3d8f961..8795957 100644 --- a/src/kimchi/utils.py +++ b/src/kimchi/utils.py @@ -33,9 +33,8 @@ from kimchi.config import paths, PluginPaths from kimchi.exception import TimeoutExpired
-from kimchi import config from kimchi.asynctask import AsyncTask -from kimchi.exception import InvalidParameter, TimeoutExpired +from kimchi.exception import InvalidParameter
kimchi_log = cherrypy.log.error_log @@ -220,3 +219,9 @@ def listPathModules(path): if ext in ('.py', '.pyc', '.pyo'): modules.add(base) return sorted(modules) + + +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

From: Aline Manera <alinefm@br.ibm.com> As libvirt changes the user:group of disk files we don't need to probe the username of qemu process We just need to add read permission for those files as they already will be owned by libvirt. Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- src/kimchi/kvmusertests.py | 66 -------------------------------------------- 1 file changed, 66 deletions(-) delete mode 100644 src/kimchi/kvmusertests.py diff --git a/src/kimchi/kvmusertests.py b/src/kimchi/kvmusertests.py deleted file mode 100644 index 3d69eb4..0000000 --- a/src/kimchi/kvmusertests.py +++ /dev/null @@ -1,66 +0,0 @@ -# -# Project Kimchi -# -# Copyright IBM, Corp. 2013 -# -# Authors: -# ShaoHe Feng <shaohef@linux.vnet.ibm.com> -# -# 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 libvirt -import psutil -import uuid - - -from kimchi.rollbackcontext import RollbackContext - - -class UserTests(object): - SIMPLE_VM_XML = """ - <domain type='kvm'> - <name>%s</name> - <uuid>%s</uuid> - <memory unit='KiB'>10240</memory> - <os> - <type arch='x86_64' machine='pc'>hvm</type> - <boot dev='hd'/> - </os> - </domain>""" - - def __init__(self): - self.vm_uuid = uuid.uuid3(uuid.NAMESPACE_DNS, 'vm-test.kimchi.org') - self.vm_name = "kimchi_test_%s" % self.vm_uuid - - def probe_user(self): - xml = self.SIMPLE_VM_XML % (self.vm_name, self.vm_uuid) - user = None - with RollbackContext() as rollback: - conn = libvirt.open('qemu:///system') - rollback.prependDefer(conn.close) - dom = conn.defineXML(xml) - rollback.prependDefer(dom.undefine) - dom.create() - rollback.prependDefer(dom.destroy) - with open('/var/run/libvirt/qemu/%s.pid' % self.vm_name) as f: - pidStr = f.read() - p = psutil.Process(int(pidStr)) - user = p.username - return user - - -if __name__ == '__main__': - ut = UserTests() - print ut.probe_user() -- 1.7.10.4

Reviewed-by: Daniel Barboza <danielhb@linux.vnet.ibm.com> On 02/03/2014 01:01 PM, Aline Manera wrote:
From: Aline Manera <alinefm@br.ibm.com>
As libvirt changes the user:group of disk files we don't need to probe the username of qemu process We just need to add read permission for those files as they already will be owned by libvirt.
Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- src/kimchi/kvmusertests.py | 66 -------------------------------------------- 1 file changed, 66 deletions(-) delete mode 100644 src/kimchi/kvmusertests.py
diff --git a/src/kimchi/kvmusertests.py b/src/kimchi/kvmusertests.py deleted file mode 100644 index 3d69eb4..0000000 --- a/src/kimchi/kvmusertests.py +++ /dev/null @@ -1,66 +0,0 @@ -# -# Project Kimchi -# -# Copyright IBM, Corp. 2013 -# -# Authors: -# ShaoHe Feng <shaohef@linux.vnet.ibm.com> -# -# 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 libvirt -import psutil -import uuid - - -from kimchi.rollbackcontext import RollbackContext - - -class UserTests(object): - SIMPLE_VM_XML = """ - <domain type='kvm'> - <name>%s</name> - <uuid>%s</uuid> - <memory unit='KiB'>10240</memory> - <os> - <type arch='x86_64' machine='pc'>hvm</type> - <boot dev='hd'/> - </os> - </domain>""" - - def __init__(self): - self.vm_uuid = uuid.uuid3(uuid.NAMESPACE_DNS, 'vm-test.kimchi.org') - self.vm_name = "kimchi_test_%s" % self.vm_uuid - - def probe_user(self): - xml = self.SIMPLE_VM_XML % (self.vm_name, self.vm_uuid) - user = None - with RollbackContext() as rollback: - conn = libvirt.open('qemu:///system') - rollback.prependDefer(conn.close) - dom = conn.defineXML(xml) - rollback.prependDefer(dom.undefine) - dom.create() - rollback.prependDefer(dom.destroy) - with open('/var/run/libvirt/qemu/%s.pid' % self.vm_name) as f: - pidStr = f.read() - p = psutil.Process(int(pidStr)) - user = p.username - return user - - -if __name__ == '__main__': - ut = UserTests() - print ut.probe_user()

From: Aline Manera <alinefm@br.ibm.com> libvirt needs to have read permission on cdrom files in order to start the virtual machine without problems. Otherwise, the user will get the following error: $ sudo virsh start aaaa error: Failed to start domain aaaa error: internal error process exited while connecting to monitor: kvm: -drive file=/root/Fedora-Live-Desktop-x86_64-19-1.iso: could not open disk image /root/Fedora-Live-Desktop-x86_64-19-1.iso: Permission denied So make sure the ISO file has read permission before starting the virtual machine. Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- src/kimchi/model/vms.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index e9f7753..d4384a1 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -36,7 +36,7 @@ from kimchi.model.config import CapabilitiesModel from kimchi.model.templates import TemplateModel from kimchi.model.utils import get_vm_name from kimchi.screenshot import VMScreenshot -from kimchi.utils import template_name_from_uri +from kimchi.utils import run_setfacl_set_attr, template_name_from_uri DOM_STATE_MAP = {0: 'nostate', @@ -346,6 +346,14 @@ class VMModel(object): vnc.remove_proxy_token(name) def start(self, name): + # make sure the ISO file has read permission + dom = self.get_vm(name, self.conn) + xml = dom.XMLDesc(0) + xpath = "/domain/devices/disk[@device='cdrom']/source/@file" + isofiles = xmlutils.xpath_get_text(xml, xpath) + for iso in isofiles: + run_setfacl_set_attr(iso) + dom = self.get_vm(name, self.conn) dom.create() -- 1.7.10.4

Reviewed-by: Daniel Barboza <danielhb@linux.vnet.ibm.com> On 02/03/2014 01:01 PM, Aline Manera wrote:
From: Aline Manera <alinefm@br.ibm.com>
libvirt needs to have read permission on cdrom files in order to start the virtual machine without problems.
Otherwise, the user will get the following error:
$ sudo virsh start aaaa error: Failed to start domain aaaa error: internal error process exited while connecting to monitor: kvm: -drive file=/root/Fedora-Live-Desktop-x86_64-19-1.iso: could not open disk image /root/Fedora-Live-Desktop-x86_64-19-1.iso: Permission denied
So make sure the ISO file has read permission before starting the virtual machine.
Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- src/kimchi/model/vms.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index e9f7753..d4384a1 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -36,7 +36,7 @@ from kimchi.model.config import CapabilitiesModel from kimchi.model.templates import TemplateModel from kimchi.model.utils import get_vm_name from kimchi.screenshot import VMScreenshot -from kimchi.utils import template_name_from_uri +from kimchi.utils import run_setfacl_set_attr, template_name_from_uri
DOM_STATE_MAP = {0: 'nostate', @@ -346,6 +346,14 @@ class VMModel(object): vnc.remove_proxy_token(name)
def start(self, name): + # make sure the ISO file has read permission + dom = self.get_vm(name, self.conn) + xml = dom.XMLDesc(0) + xpath = "/domain/devices/disk[@device='cdrom']/source/@file" + isofiles = xmlutils.xpath_get_text(xml, xpath) + for iso in isofiles: + run_setfacl_set_attr(iso) + dom = self.get_vm(name, self.conn) dom.create()
participants (2)
-
Aline Manera
-
Daniel H Barboza