- Kimchi-devel - oVirt List Archives

[PATCH] bug fix: add an icon distros list.
by shaohef＠linux.vnet.ibm.com 10 Mar '14

10 Mar '14

From: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com> Now I create template with rhel iso, but when I create a VM from this template, the template info displays differently with other templates. That's because icon-rhel is missing for it is RED HAT trademark. But this make us uncomfortable. If the icon is missing we should display the default icon Now there are only 7 icons, include the default icon icon-vm. $ ls ui/images/*.png ui/images/icon-centos.png ui/images/icon-gentoo.png ui/images/icon-debian.png ui/images/icon-opensuse.png ui/images/icon-fedora.png ui/images/icon-ubuntu.png ui/images/icon-vm.png Signed-off-by: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com> --- src/kimchi/osinfo.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/kimchi/osinfo.py b/src/kimchi/osinfo.py index 106cc0e..2eac63e 100644 --- a/src/kimchi/osinfo.py +++ b/src/kimchi/osinfo.py @@ -55,6 +55,8 @@ modern_version_bases = {'x86': {'debian': '6.0', 'ubuntu': '7.10', 'rhel': '6.0', 'fedora': '16', 'gentoo': '0'}, 'power': {'rhel': '7.0', 'fedora': '19'}} +icon_available_distros = ['debian', 'centos', 'fedora', + 'gentoo', 'opensuse', 'ubuntu'] isolinks = { 'debian': { @@ -100,15 +102,18 @@ def lookup(distro, version): arch = _get_arch() if distro in modern_version_bases[arch]: - params['icon'] = 'images/icon-%s.png' % distro if LooseVersion(version) >= LooseVersion( modern_version_bases[arch][distro]): params.update(template_specs[arch]['modern']) else: params.update(template_specs[arch]['old']) else: - params['icon'] = 'images/icon-vm.png' params['os_distro'] = params['os_version'] = "unknown" params.update(template_specs[arch]['old']) + if distro in icon_available_distros: + params['icon'] = 'images/icon-%s.png' % distro + else + params['icon'] = 'images/icon-vm.png' + return params -- 1.8.4.2

3 2

[PATCHv7 0/7] Support storage volume ref_cnt
by lvroyce＠linux.vnet.ibm.com 10 Mar '14

10 Mar '14

From: Royce Lv <lvroyce(a)linux.vnet.ibm.com> tested: 1. sudo make check 2. from UI: list storage volumes and unrefered volume ref_cnt is 0, ref_cnt for refered ones are 1. Available volumes will be tracked by volume reference count. Add this field to storage volume, so that storage volume/pool action validation can rely on it. Royce Lv (7): Export list vms functionality Fix vm disk path when it does not have source element Add volume ref_cnt: update api.md Add volume ref_cnt: Update controller and json schema Add volume ref_cnt: Add model and mockmodel implementation Add volume ref_cnt: Update test Multiple pep8 fixes docs/API.md | 3 +++ src/kimchi/API.json | 25 +++++++++++++++++++++++++ src/kimchi/control/storagevolumes.py | 1 + src/kimchi/i18n.py | 4 ++++ src/kimchi/mockmodel.py | 10 +++++++--- src/kimchi/model/debugreports.py | 2 +- src/kimchi/model/storagevolumes.py | 30 ++++++++++++++++++++++++++++++ src/kimchi/model/templates.py | 2 +- src/kimchi/model/vms.py | 6 +++++- src/kimchi/model/vmstorages.py | 23 +++++++++++++---------- tests/test_model.py | 4 ++++ tests/test_rest.py | 2 ++ 12 files changed, 96 insertions(+), 16 deletions(-) -- 1.8.1.2

3 15

[RFC][PATCH V2 0/3] Issue #322
by shaohef＠linux.vnet.ibm.com 10 Mar '14

10 Mar '14

From: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com> V1 -> V2 use debug to log the open exception trace stack. It is complex to check the permission for qemu user. There are too many access controlling mechanisms in Linux, such as ACL, traditional ugo+-rwx, SELinux and AppArmor. It's not possible to enumerate and check every access mechanisms if it allows QEMU to access a file. So now I'm simply trying to access the file with qemu user and see if it's OK 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 | 13 ++++++++- src/kimchi/utils.py | 26 ++++++++++++++++++ 5 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 src/kimchi/kvmusertests.py -- 1.8.4.2

2 5

[PATCH] Do not show a success msg when creating a template
by Crístian Viana 10 Mar '14

10 Mar '14

Tested on Fedora 20. Crístian Viana (1): Do not show a success msg when creating a template ui/js/src/kimchi.template_add_main.js | 1 - 1 file changed, 1 deletion(-) -- 1.8.5.3

3 5

[PATCH] [UI] Window Widget Enhancement - Allow onClose Listener
by Hongliang Wang 10 Mar '14

10 Mar '14

Sometimes we want to trigger some action when window is being closed. For example, we register some listeners in one window, and we want to unregister them after this window is closed, in this case we can take advantage of this feature. Signed-off-by: Hongliang Wang <hlwang(a)linux.vnet.ibm.com> --- ui/js/src/kimchi.window.js | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/ui/js/src/kimchi.window.js b/ui/js/src/kimchi.window.js index b32ab60..c2af63e 100644 --- a/ui/js/src/kimchi.window.js +++ b/ui/js/src/kimchi.window.js @@ -17,6 +17,7 @@ */ kimchi.window = (function() { var _windows = []; + var _listeners = {}; var open = function(settings) { var settings = jQuery.type(settings) === 'object' ? settings : { url: settings @@ -29,6 +30,7 @@ kimchi.window = (function() { } _windows.push(windowID); + _listeners[windowID] = settings['close']; var windowNode = $('<div></div>', { id: windowID, 'class': settings['class'] ? settings['class'] + ' bgmask' : 'bgmask' @@ -37,9 +39,6 @@ kimchi.window = (function() { $(windowNode).css(settings['style'] || ''); $(windowNode).appendTo('body').on('click', '.window .close', function() { - if (settings.close) { - settings.close(); - } kimchi.window.close(); }); @@ -52,7 +51,14 @@ kimchi.window = (function() { }; var close = function() { - $('#' + _windows.pop()).fadeOut(100, function() { + var windowID = _windows.pop(); + if(_listeners[windowID]) { + _listeners[windowID](); + _listeners[windowID] = null; + } + delete _listeners[windowID]; + + $('#' + windowID).fadeOut(100, function() { $(this).remove(); }); }; -- 1.8.1.4

2 2

[PATCH] Bug fix: Kimchi could not start guest with iscsi lun assigned as disk
by Rodrigo Trujillo 10 Mar '14

10 Mar '14

There is an error in Kimchi when you try to start a guest created based on a template that is using an iscsi storage pool. A lun should be used as disk, but the iscsi case was not being expected and kimchi assigned an .img file to the guest, raising error in the starting. This patch fixed this problem. Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo(a)linux.vnet.ibm.com> --- src/kimchi/vmtemplate.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py index f380170..9825159 100644 --- a/src/kimchi/vmtemplate.py +++ b/src/kimchi/vmtemplate.py @@ -24,6 +24,7 @@ import urlparse from kimchi import osinfo +from kimchi.config import READONLY_POOL_TYPE from kimchi.exception import InvalidParameter, IsoFormatError from kimchi.isoinfo import IsoImage from kimchi.utils import check_url_path, pool_name_from_uri @@ -271,7 +272,7 @@ class VMTemplate(object): # Current implementation just allows to create disk in one single # storage pool, so we cannot mix the types (scsi volumes vs img file) - if self._get_storage_type() == 'scsi': + if self._get_storage_type() in READONLY_POOL_TYPE: params['disks'] = self._get_scsi_disks_xml(kwargs.get('volumes')) else: params['disks'] = self._get_disks_xml(vm_uuid) -- 1.8.5.3

2 2

[PATCH] Rebuild .po files when running "make"
by Crístian Viana 10 Mar '14

10 Mar '14

Tested on Fedora 20. Crístian Viana (1): Rebuild .po files when running "make" po/Makefile.in.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 1.8.5.3

3 5

[PATCH] bug fix: setup disks to use cache=none to support live migration.
by Paulo Vital 10 Mar '14

10 Mar '14

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

7 8

[RFC PATCH 0/3] Issue #322
by shaohef＠linux.vnet.ibm.com 10 Mar '14

10 Mar '14

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

2 5

Re: [Kimchi-devel] [PATCH] Don't allow templates to be created with ISOs that won't be usable.
by Sheldon 10 Mar '14

10 Mar '14

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

2 1