[PATCH] bug fix: setup disks to use cache=none to support live migration.
by Paulo Vital
When executing a virsh live migration of a VM created by Kimchi, the
migration stops due to miss configuration "cache=none" on disk's
performance parameters.
Signed-off-by: Paulo Vital <pvital(a)linux.vnet.ibm.com>
---
src/kimchi/vmtemplate.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py
index e2d78a4..89b5e72 100644
--- a/src/kimchi/vmtemplate.py
+++ b/src/kimchi/vmtemplate.py
@@ -152,7 +152,7 @@ class VMTemplate(object):
params = {'src': src, 'dev': dev, 'bus': self.info['disk_bus']}
ret += """
<disk type='file' device='disk'>
- <driver name='qemu' type='qcow2'/>
+ <driver name='qemu' type='qcow2' cache='none'/>
<source file='%(src)s' />
<target dev='%(dev)s' bus='%(bus)s' />
</disk>
@@ -182,7 +182,7 @@ class VMTemplate(object):
# Passthrough configuration
disk_xml = """
<disk type='volume' device='lun'>
- <driver name='qemu' type='raw'/>
+ <driver name='qemu' type='raw' cache='none'/>
<source dev='%(src)s'/>
<target dev='%(dev)s' bus='scsi'/>
</disk>"""
--
1.8.3.1
10 years, 9 months
[RFC PATCH 0/3] Issue #322
by shaohef＠linux.vnet.ibm.com
From: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
It is complex to check the permission for qemu user.
x bit for seach, group, user, selinux and so on.
So now I try to open the iso with qemu user.
It is simple and avoid handling corner case.
Also I can try to start a simple guest with the given iso.
ShaoHe Feng (3):
add a method to probe the permission as qemu user
qemu user tests: probe the username of qemu process started by libvirt
Don't allow templates to be created with ISOs that won't be usable.
Makefile.am | 1 +
src/kimchi/i18n.py | 5 ++++
src/kimchi/kvmusertests.py | 62 +++++++++++++++++++++++++++++++++++++++++++
src/kimchi/model/templates.py | 11 +++++++-
src/kimchi/utils.py | 26 ++++++++++++++++++
5 files changed, 104 insertions(+), 1 deletion(-)
create mode 100644 src/kimchi/kvmusertests.py
--
1.8.4.2
10 years, 9 months
Re: [Kimchi-devel] [PATCH] Don't allow templates to be created with ISOs that won't be usable.
by Sheldon
On 03/08/2014 04:20 AM, Christy Perez wrote:
> 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:
>> On 03/04/2014 06:38 AM, Christy Perez wrote:
>>> On Wed, 2014-02-26 at 20:26 +0800, Sheldon wrote:
>>>> 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(a)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?
>>> 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: http://libvirt.org/drvqemu.html
>> so that's means the disk images without S_IXOTH, qemu still can have
>> search permissions right?
> 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
>
>> o+x is sufficient but not necessary condition?
> o+x is necessary.
>>>> 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?
>>> 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.
>> That's strangely. but the docs http://libvirt.org/drvqemu.html
>> says:
>> it simple to set o+x, means this is the only way.
> Yes, I think so.
it is strange from what I test, it seems o+x is not necessary but
sufficient.
I really want to know why o+x is necessary and why it is the only way
to check the permission.
I have try 5 test case with kimchi.
1. There's a iso on my laptop, with o+x permission
$ ls /home/shhfeng/iso/RHEL6.4-20130123.0-Server-x86_64-DVD1.iso
/home/shhfeng/iso/RHEL6.4-20130123.0-Server-x86_64-DVD1.iso
$ ls /home/shhfeng/ -l |grep iso
drwxrwxrwx. 4 shhfeng shhfeng 4096 Oct 3 00:22 iso
I can create a vm and start is successfully.
2. Now I remove x permission
$ sudo chmod o-x /home/shhfeng/iso
$ ls /home/shhfeng/ -l |grep iso
drwxrwxrw-. 4 shhfeng shhfeng 4096 Oct 3 00:22 iso
I failed to start it for permission.
3. I change the own of this iso, still without x permission.
$ sudo chown qemu:qemu /home/shhfeng/iso
$ ls /home/shhfeng/ -l |grep iso
drwxrwxrw-. 4 qemu qemu 4096 Oct 3 00:22 iso
Now I can start it successfully.
4. just set the qemu user as search permission.
$ sudo chown shhfeng:shhfeng /home/shhfeng/iso/
$ ls -l /home/shhfeng/ |grep iso
drwxrwxrw-. 4 shhfeng shhfeng 4096 Oct 3 00:22 iso
$ setfacl -m u:qemu:x /home/shhfeng/iso
$ getfacl /home/shhfeng/iso
getfacl: Removing leading '/' from absolute path names
# file: home/shhfeng/iso
# owner: shhfeng
# group: shhfeng
user::rwx
user:qemu:--x
group::rwx
mask::rwx
other::rw-
Now I can also start it successfully.
5. remove qemu user as search permission,
add set the 'kvm' group to the path, qemu is in "kvm" group
$ setfacl -x u:qemu /home/shhfeng/iso
$ groups qemu
qemu : qemu kvm
$ sudo chown shhfeng:kvm /home/shhfeng/iso/
Now I can also start it successfully.
only case 2, the kimchi deny to start vm for permission.
I write a python script checkperm.py
It is the same result of libvirt run qemu. you can have a try.
If others do like like checkperm.py,
I think we can probe the permission by let libvirt to start the guest
with the iso.
$ sudo python checkperm.py
/home/shhfeng/iso/RHEL6.4-20130123.0-Server-x86_64-DVD1.iso
# checkperm.py
1 from multiprocessing import Process, Queue
2 import sys
3 import grp
4 import os
5 import pwd
6
7
8 def probe_file_permission_as_user(file, user):
9 def probe_permission(q, file, user):
10 uid = pwd.getpwnam(user).pw_uid
11 gid = pwd.getpwnam(user).pw_gid
12 gids = [g.gr_gid for g in grp.getgrall() if user in g.gr_mem]
13 os.setgid(gid)
14 os.setgroups(gids)
15 os.setuid(uid)
16 try:
17 with open(file):
18 q.put(True)
19 except Exception:
20 q.put(False)
21
22 queue = Queue()
23 p = Process(target=probe_permission, args=(queue, file, user))
24 p.start()
25 p.join()
26 return queue.get()
27
28 if __name__ == '__main__':
29 print probe_file_permission_as_user(sys.argv[1], 'qemu')
>> """
>> 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.
>> """
>>
>>
>>>> 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
>>> 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.
>> 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?
> 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/.
>>>>> 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('/'):
>>> Regards,
>>>
>>> - Christy
>>>
>>
> Regards,
>
> - Christy
>
--
Thanks and best regards!
Sheldon Feng(冯少合)<shaohef(a)linux.vnet.ibm.com>
IBM Linux Technology Center
10 years, 9 months
[PATCHv2 0/2] Make vms model singleton
by lvroyce＠linux.vnet.ibm.com
From: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Everytime when when VMsModel.get_list() need to be called
one more background task to collect vm status is started.
Fix this by making vms model singleton.
Royce Lv (2):
Make vms model to be a singleton
Hack test_model to support vm model singleton
src/kimchi/model/vms.py | 3 +++
tests/test_model.py | 2 ++
2 files changed, 5 insertions(+)
--
1.8.1.2
10 years, 9 months
[PATCH] [UI] Window - Correct Footer Height
by Hongliang Wang
The footer height is 56 pixel, though window content only leaves
48 pixel for it by:
margin: 48px 0;
Fix it in this patch.
Signed-off-by: Hongliang Wang <hlwang(a)linux.vnet.ibm.com>
---
ui/css/theme-default/window.css | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ui/css/theme-default/window.css b/ui/css/theme-default/window.css
index 09a9840..7e057fd 100644
--- a/ui/css/theme-default/window.css
+++ b/ui/css/theme-default/window.css
@@ -77,7 +77,7 @@
top: 0;
bottom: 0;
overflow: auto;
- margin: 48px 0;
+ margin: 48px 0 56px;
}
.window .close {
--
1.8.1.4
10 years, 9 months
[PATCHv1 0/2] fix create vm on logical pool
by lvroyce＠linux.vnet.ibm.com
From: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
VG path is create under 'dev' and logical volume's format should
be raw, to avoid qcow2 write exceed problem.
Royce Lv (2):
Fix storage volume format on logical pool for vm
logical pool: Fix logical pool target path
src/kimchi/model/libvirtstoragepool.py | 2 +-
src/kimchi/vmtemplate.py | 8 +++++---
tests/test_storagepool.py | 2 +-
3 files changed, 7 insertions(+), 5 deletions(-)
--
1.8.1.2
10 years, 9 months
[PATCH] Use a pool of threads to valid all remote ISOs in parallel
by Aline Manera
From: Aline Manera <alinefm(a)br.ibm.com>
That way we can reduce processing time in 8s.
Without threads the requests takes 15s to complete:
GET http://localhost:8000/config/distros 200 OK 10.64s
And with the pool of threads it takes 2s:
GET http://localhost:8000/config/distros 200 OK 2.01s
Signed-off-by: Aline Manera <alinefm(a)br.ibm.com>
---
src/kimchi/model/config.py | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/src/kimchi/model/config.py b/src/kimchi/model/config.py
index d44ef90..3580ad3 100644
--- a/src/kimchi/model/config.py
+++ b/src/kimchi/model/config.py
@@ -17,6 +17,8 @@
# License along with this library; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+from multiprocessing.pool import ThreadPool
+
import cherrypy
from kimchi.basemodel import Singleton
@@ -91,12 +93,16 @@ class DistrosModel(object):
self.distros = distroloader.get()
def get_list(self):
- res = []
- # only return distro with valid URL
- for distro, data in self.distros.iteritems():
- url = data['path']
- if check_url_path(url):
- res.append(distro)
+ def validate_distro(distro):
+ if check_url_path(distro['path']):
+ return distro['name']
+
+ n_processes = len(self.distros.keys())
+ pool = ThreadPool(processes=n_processes)
+ map_res = pool.map_async(validate_distro, self.distros.values())
+ pool.close()
+ pool.join()
+ res = list(set(map_res.get()) - set([None]))
return sorted(res)
--
1.7.10.4
10 years, 9 months
[PATCH] bug fix: "sudo: sorry, you must have a tty to run sudo".
by Paulo Vital
The authorization code executes some sudo commands in backend to
check if the user used to login in Kimchi has permission to execute
privileged commands.
By default, sudo needs a TTY to be executed and when Kimchi is executed
as a service by systemd (systemctl start kimchid.service), it runs
disconnected from any TTY, and sudo returns an error and the message:
"sudo: sorry, you must have a tty to run sudo".
This patch sets in Kimchi service file for systemd the /dev/tty11 as
default TTY to execute the service.
Signed-off-by: Paulo Vital <pvital(a)linux.vnet.ibm.com>
---
contrib/kimchid.service.fedora | 3 +++
1 file changed, 3 insertions(+)
diff --git a/contrib/kimchid.service.fedora b/contrib/kimchid.service.fedora
index 7abe49b..5f274fb 100644
--- a/contrib/kimchid.service.fedora
+++ b/contrib/kimchid.service.fedora
@@ -8,6 +8,9 @@ Type=simple
ExecStart=/usr/bin/kimchid
ExecStop=/bin/kill -TERM $MAINPID
EnvironmentFile=/etc/kimchi/kimchi.conf
+StandardInput=tty
+StandardOutput=tty
+TTYPath=/dev/tty11
[Install]
WantedBy=multi-user.target
--
1.8.3.1
10 years, 9 months
[PATCH 0/3] Fix issue #301
by Aline Manera
From: Aline Manera <alinefm(a)br.ibm.com>
Aline Manera (3):
issue #301: Only list remote ISOs with valid URL
issue #301: Add a loading message while listing default remote ISOs
Update distros JSON files to always point to a valid URL
po/en_US.po | 8 +++++++-
po/kimchi.pot | 8 +++++++-
po/pt_BR.po | 8 +++++++-
po/zh_CN.po | 8 +++++++-
src/distros.d/debian.json | 4 ++--
src/distros.d/gentoo.json | 6 +++---
src/kimchi/model/config.py | 10 ++++++++--
ui/js/src/kimchi.template_add_main.js | 4 ++++
ui/pages/template-add.html.tmpl | 12 +++++++++++-
9 files changed, 56 insertions(+), 12 deletions(-)
--
1.7.10.4
10 years, 9 months
