RFC - Guest memory usage
by Daniel Henrique Barboza
I was looking at the following item in the backlog:
Guests Stats: Display memory utilization (use virt-df or virt-top or ...)
If I understood it right, the idea here is to show the -inner- memory
allocation of the guest. If you have a VM with 4Gb of RAM running an
Ubuntu, we want to know how much memory the Ubuntu OS and its processes
are using.
I've done an investigation and I haven't found any tool to accomplish
this. "virt-top", "virsh dommemstat" and the libvirt API retrieves the
information of the memory usage of the guest relative to the host. In
the example mentioned before, supposing that the host has 64Gb of RAM,
all these tools would show that the VM is using 12% of the host RAM.
They do not dive in the VM and shows the actual mem usage of the Ubuntu
and its processes running there.
Haven't found anything useful in other MLs and forums. The common answer
is 'run top in a terminal inside the VM', which of course does not suit
us. My question is: any thoughts about how we can implement this
feature? Because I am starting to think that, in the end, this kind of
info is strict to the guest OS and can't be polled from the outside.
Thanks!
9 years, 1 month
[kimchi-devel][PATCHv3 0/6] Upload storage volume
by lvroyce＠linux.vnet.ibm.com
From: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
known issue:
When using api to send requests without setting up session,
cherrypy server will report error after hundred of calls,
because race condition below:
https://bitbucket.org/cherrypy/cherrypy/pull-request/50/fix-race-conditio...
But it works well with session setup.
v3>v1,
Use libvirt upload api instead of r/w file
Use lock to synchronize upload to prevent
Royce Lv (6):
Update docs and json schema of storage volume upload
Update controller to make update accept formdata params
Add lock facility for storage volume upload
Update model for storage volume update
Fix incomplete record when uploading
update test case for storage volume upload
docs/API.md | 7 ++--
src/kimchi/API.json | 19 +++++++++++
src/kimchi/control/base.py | 6 ++--
src/kimchi/i18n.py | 4 +++
src/kimchi/isoinfo.py | 5 ++-
src/kimchi/mockmodel.py | 8 ++---
src/kimchi/model/storagevolumes.py | 61 +++++++++++++++-------------------
src/kimchi/model/utils.py | 46 ++++++++++++++++++++++++++
tests/test_model.py | 67 ++++++++++++++++++++++++++++++++++++++
9 files changed, 177 insertions(+), 46 deletions(-)
--
2.1.0
9 years, 8 months
[PATCH] RFC patch to make nginx proxy optional
by Julien Goodwin
Sending this as an RFC patch, I expect people will want changes from this.
Implements Issue #570.
Julien Goodwin (1):
Initial prototype, make nginx proxy optional.
docs/Makefile.am | 1 +
docs/apache.conf.ex | 35 +++++++++++++++++++++++++++++++++++
src/kimchi.conf.in | 3 +++
src/kimchi/config.py.in | 1 +
src/kimchi/proxy.py | 6 ++++++
5 files changed, 46 insertions(+)
create mode 100644 docs/apache.conf.ex
--
2.1.4
9 years, 8 months
[RFC] New API to create a random guest console password
by Aline Manera
Today user may set/change the guest console password and also its
expiration time through Kimchi API.
When passing an empty password, a random password is automatically
generated.
curl -u <user:password> -H "Content-Type: application/json" -H "Accept:
application/json"
http://localhost:8010/vms/blah -X PUT -d'{"graphics": {"passwd": ""}}'
That way is difficult to handle when user wants to reset the guest password.
We have a similar issue when we automatically change the passwdValidTo
when it is expired - increasing it in 30 seconds.
My proposal is simple: only change "passwd" and "passwdValidTo" when
user wants to do it.
curl -u <user:password> -H "Content-Type: application/json" -H "Accept:
application/json"
http://localhost:8010/vms/blah -X PUT -d'{"graphics": {"passwd":
"123456", "passwdValidTo": "<some datetime format>"}}'
curl -u <user:password> -H "Content-Type: application/json" -H "Accept:
application/json"
http://localhost:8010/vms/blah -X PUT -d'{"graphics":
{"passwdValidTo": "<some datetime format>"}}'
And make sure the passwdValidTo is only acceptable when there is a
passwd set.
And to reset those values, we only need to send an empty string:
curl -u <user:password> -H "Content-Type: application/json" -H "Accept:
application/json"
http://localhost:8010/vms/blah -X PUT -d'{"graphics": {"password":
"", "passwdValidTo": ""}}'
And create a new API: POST /vms/blah/ticket to automatically generate a
random password valid only for 30 seconds.
curl -u <user:password> -H "Content-Type: application/json" -H "Accept:
application/json"
http://localhost:8010/vms/blah/ticket -X POST -d'{}'
What do you think about it?
Regards,
Aline Manera
9 years, 8 months
[PATCH][RFC] Kimchi and relative URLs
by Frédéric Bonnard
From: Frederic Bonnard <frediz(a)linux.vnet.ibm.com>
Hi,
many people and distros use a subdirectory in the configuration of the system's webserver
to configure different websites.
The usual way to setup a website is thus to :
1. use the distro's webserver to serve all the websites
2. have a subdirectory to copy the configuration file that makes available a website
by only restarting the webserver (and removing the website is just done by removing
the configuration file and restart). Concerning this point, the classical
configurations can be :
a) a sub web location : https://website.org/webapp
b) a virtual host : http://webapp.website.org
a) and b) correspond each to different configuration files that are just dropped into :
- Ubuntu/Debian : /etc/{apache,nginx,..}/sites-available/{webapp_subweb,webapp_virthost}.conf
- Fedora/Opensuse : /etc/{httpd,nginx,..}/conf.d/
At the moment kimchi is launched with a private instance of nginx, not the system's
installed one and it would be nice to have this improved and this is covered in another thread :
http://lists.ovirt.org/pipermail/kimchi-devel/2015-February/009642.html
Provided that previous patch, one can use the distro's webserver to run proxy kimchi
and that fullfills 1.
Then for 2. one need a configuration file for either b) (provided for apache in the
link above ; I have also one for nginx ) and a), which I provide in this patch.
But this nginx configuration file is not enough for a sub web location configuration
as kimchi relies on absolute path based on / from the http://server/.
I tried to make URLs paths relative so that kimchi doesn't have to know where it has
been placed on the webserver, for example https://server/kimchi/ which will probably
be the URL used in distros.
I'm not a web developer and I tried to modify all failing requests with a a) configuration
with relative paths in mind. So I'd like comments on the way this is done, and this
may need more extensive testing (paths I didn't test).
Though I used that code back on a virtualhost configuration and it worked as well.
Thanks for your help,
F.
Frederic Bonnard (1):
Making urls relative
docs/Makefile.am | 1 +
docs/nginx.conf.subsite.ex | 37 +++++++
src/kimchi/screenshot.py | 2 +-
ui/css/theme-default/template_add.css | 20 ++--
ui/css/theme-default/topbar.css | 2 +-
ui/js/src/kimchi.api.js | 176 +++++++++++++++++-----------------
ui/js/src/kimchi.login.js | 2 +-
ui/pages/guest.html.tmpl | 2 +-
ui/pages/help/dita-help.xsl | 4 +-
ui/pages/kimchi-ui.html.tmpl | 4 +-
ui/pages/storagepool-add.html.tmpl | 2 +-
ui/pages/tabs/storage.html.tmpl | 2 +-
ui/pages/template-add.html.tmpl | 2 +-
13 files changed, 147 insertions(+), 109 deletions(-)
create mode 100644 docs/nginx.conf.subsite.ex
--
1.9.1
9 years, 8 months
[PATCH v2] Remove Non-ASCII chars from tests when unnecessary
by Christy Perez
A domain with non-ASCII characters will fail to start. There are
several tests that use such characters, and these tests fail. None
of the failing tests are required to test the validity of non-ascii
characters, so it's best to remove them and create seperate tests
explicitly for these chracters. This way we will not be masking actual
issues by ignoring an expected failure.
The issue was first noticed with libvirt 1.1.3.
The character issue is discussed in this bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1062943
Signed-off-by: Christy Perez <christy(a)linux.vnet.ibm.com>
---
tests/test_model.py | 112 +++++++++++++++++++++++++++-----------
tests/test_model_network.py | 6 +-
tests/test_model_storagepool.py | 27 +++++++++
tests/test_model_storagevolume.py | 1 -
4 files changed, 113 insertions(+), 33 deletions(-)
diff --git a/tests/test_model.py b/tests/test_model.py
index f80f1c9..ada7498 100644
--- a/tests/test_model.py
+++ b/tests/test_model.py
@@ -97,6 +97,44 @@ def test_vm_info(self):
self.assertTrue(info['persistent'])
@unittest.skipUnless(utils.running_as_root(), 'Must be run as root')
+ def test_non_ascii_vm_name(self):
+ inst = model.Model(objstore_loc=self.tmp_store)
+
+ with RollbackContext() as rollback:
+ vol_params = {'name': u'test-vol', 'capacity': 1024}
+ task = inst.storagevolumes_create(u'default', vol_params)
+ rollback.prependDefer(inst.storagevolume_delete, u'default',
+ vol_params['name'])
+ inst.task_wait(task['id'])
+ task = inst.task_lookup(task['id'])
+ self.assertEquals('finished', task['status'])
+ vol = inst.storagevolume_lookup(u'default', vol_params['name'])
+
+ # Create template
+ params = {'name': 'test', 'disks': [{'base': vol['path'],
+ 'size': 1}],
+ 'cdrom': self.kimchi_iso}
+ inst.templates_create(params)
+ rollback.prependDefer(inst.template_delete, 'test')
+
+ # Create VM with non-ascii char name
+ params = {'name': u'��e��-�����', 'template': '/templates/test'}
+ inst.vms_create(params)
+ rollback.prependDefer(inst.vm_delete, u'��e��-�����')
+
+ vms = inst.vms_get_list()
+ self.assertTrue(u'��e��-�����' in vms)
+
+ # Start VM
+ # Note: This fails with libvirt > 1.1.3
+ inst.vm_start(u'��e��-�����')
+ info = inst.vm_lookup(u'��e��-�����')
+ self.assertEquals('running', info['state'])
+
+ vms = inst.vms_get_list()
+ self.assertFalse(u'��e��-�����' in vms)
+
+ @unittest.skipUnless(utils.running_as_root(), 'Must be run as root')
def test_vm_lifecycle(self):
inst = model.Model(objstore_loc=self.tmp_store)
@@ -620,6 +658,18 @@ def test_template_storage_customise(self):
params = {'storagepool': '/storagepools/default'}
inst.template_update('test', params)
+ def test_non_ascii_template_create(self):
+ inst = model.Model('test:///default',
+ objstore_loc=self.tmp_store)
+
+ with RollbackContext() as rollback:
+ params = {'name': u'��e��-te��plate', 'memory': 1024, 'cpus': 1,
+ 'cdrom': self.kimchi_iso}
+ inst.templates_create(params)
+ rollback.prependDefer(inst.template_delete, u'��e��-te��plate')
+ info = inst.template_lookup(u'��e��-te��plate')
+ self.assertEquals(info['name'], u'��e��-te��plate')
+
@unittest.skipUnless(utils.running_as_root(), 'Must be run as root')
def test_template_create(self):
inst = model.Model('test:///default',
@@ -737,7 +787,7 @@ def test_template_update(self):
inst.network_activate(net_name)
rollback.prependDefer(inst.network_deactivate, net_name)
- net_name = u'k������h��-��et'
+ net_name = u'kimchi-net'
net_args = {'name': net_name,
'connection': 'nat',
'subnet': '127.0.20.0/24'}
@@ -764,7 +814,7 @@ def test_template_update(self):
self.assertEquals("default", info["networks"][0])
params = {'name': 'new-test', 'memory': 1024, 'cpus': 1,
- 'networks': ['default', 'test-network', u'k������h��-��et']}
+ 'networks': ['default', 'test-network', u'kimchi-net']}
inst.template_update('new-test', params)
info = inst.template_lookup('new-test')
for key in params.keys():
@@ -792,7 +842,7 @@ def test_template_update(self):
self.assertEquals('some-vm', inst.vms_create(params))
rollback.prependDefer(inst.vm_delete, 'some-vm')
- iface_args = {'type': 'network', 'network': u'k������h��-��et'}
+ iface_args = {'type': 'network', 'network': u'kimchi-net'}
mac = inst.vmifaces_create('some-vm', iface_args)
self.assertEquals(17, len(mac))
@@ -865,52 +915,52 @@ def test_vm_edit(self):
self.assertRaises(OperationFailed, inst.vm_update,
'kimchi-vm1', {'name': 'kimchi-vm2'})
- params = {'name': u'��e��-�����', 'cpus': 4, 'memory': 2048}
+ params = {'name': u'new-vm', 'cpus': 4, 'memory': 2048}
inst.vm_update('kimchi-vm1', params)
rollback.prependDefer(utils.rollback_wrapper, inst.vm_delete,
- u'��e��-�����')
- self.assertEquals(info['uuid'], inst.vm_lookup(u'��e��-�����')['uuid'])
- info = inst.vm_lookup(u'��e��-�����')
+ u'new-vm')
+ self.assertEquals(info['uuid'], inst.vm_lookup(u'new-vm')['uuid'])
+ info = inst.vm_lookup(u'new-vm')
for key in params.keys():
self.assertEquals(params[key], info[key])
# change only VM users - groups are not changed (default is empty)
users = inst.users_get_list()[:3]
- inst.vm_update(u'��e��-�����', {'users': users})
- self.assertEquals(users, inst.vm_lookup(u'��e��-�����')['users'])
- self.assertEquals([], inst.vm_lookup(u'��e��-�����')['groups'])
+ inst.vm_update(u'new-vm', {'users': users})
+ self.assertEquals(users, inst.vm_lookup(u'new-vm')['users'])
+ self.assertEquals([], inst.vm_lookup(u'new-vm')['groups'])
# change only VM groups - users are not changed (default is empty)
groups = inst.groups_get_list()[:2]
- inst.vm_update(u'��e��-�����', {'groups': groups})
- self.assertEquals(users, inst.vm_lookup(u'��e��-�����')['users'])
- self.assertEquals(groups, inst.vm_lookup(u'��e��-�����')['groups'])
+ inst.vm_update(u'new-vm', {'groups': groups})
+ self.assertEquals(users, inst.vm_lookup(u'new-vm')['users'])
+ self.assertEquals(groups, inst.vm_lookup(u'new-vm')['groups'])
# change VM users and groups by adding a new element to each one
users.append(pwd.getpwuid(os.getuid()).pw_name)
groups.append(grp.getgrgid(os.getgid()).gr_name)
- inst.vm_update(u'��e��-�����', {'users': users, 'groups': groups})
- self.assertEquals(users, inst.vm_lookup(u'��e��-�����')['users'])
- self.assertEquals(groups, inst.vm_lookup(u'��e��-�����')['groups'])
+ inst.vm_update(u'new-vm', {'users': users, 'groups': groups})
+ self.assertEquals(users, inst.vm_lookup(u'new-vm')['users'])
+ self.assertEquals(groups, inst.vm_lookup(u'new-vm')['groups'])
# change VM users (wrong value) and groups
# when an error occurs, everything fails and nothing is changed
- self.assertRaises(InvalidParameter, inst.vm_update, u'��e��-�����',
+ self.assertRaises(InvalidParameter, inst.vm_update, u'new-vm',
{'users': ['userdoesnotexist'], 'groups': []})
- self.assertEquals(users, inst.vm_lookup(u'��e��-�����')['users'])
- self.assertEquals(groups, inst.vm_lookup(u'��e��-�����')['groups'])
+ self.assertEquals(users, inst.vm_lookup(u'new-vm')['users'])
+ self.assertEquals(groups, inst.vm_lookup(u'new-vm')['groups'])
# change VM users and groups (wrong value)
# when an error occurs, everything fails and nothing is changed
- self.assertRaises(InvalidParameter, inst.vm_update, u'��e��-�����',
+ self.assertRaises(InvalidParameter, inst.vm_update, u'new-vm',
{'users': [], 'groups': ['groupdoesnotexist']})
- self.assertEquals(users, inst.vm_lookup(u'��e��-�����')['users'])
- self.assertEquals(groups, inst.vm_lookup(u'��e��-�����')['groups'])
+ self.assertEquals(users, inst.vm_lookup(u'new-vm')['users'])
+ self.assertEquals(groups, inst.vm_lookup(u'new-vm')['groups'])
# change VM users and groups by removing all elements
- inst.vm_update(u'��e��-�����', {'users': [], 'groups': []})
- self.assertEquals([], inst.vm_lookup(u'��e��-�����')['users'])
- self.assertEquals([], inst.vm_lookup(u'��e��-�����')['groups'])
+ inst.vm_update(u'new-vm', {'users': [], 'groups': []})
+ self.assertEquals([], inst.vm_lookup(u'new-vm')['users'])
+ self.assertEquals([], inst.vm_lookup(u'new-vm')['groups'])
def test_multithreaded_connection(self):
def worker():
@@ -1090,19 +1140,19 @@ def test_delete_running_vm(self):
inst.templates_create(params)
rollback.prependDefer(inst.template_delete, 'test')
- params = {'name': u'k������h��-�����', 'template': u'/templates/test'}
+ params = {'name': 'kimchi-vm', 'template': u'/templates/test'}
inst.vms_create(params)
rollback.prependDefer(utils.rollback_wrapper, inst.vm_delete,
- u'k������h��-�����')
+ 'kimchi-vm')
- inst.vm_start(u'k������h��-�����')
+ inst.vm_start('kimchi-vm')
rollback.prependDefer(utils.rollback_wrapper, inst.vm_poweroff,
- u'k������h��-�����')
+ 'kimchi-vm')
- inst.vm_delete(u'k������h��-�����')
+ inst.vm_delete('kimchi-vm')
vms = inst.vms_get_list()
- self.assertFalse(u'k������h��-�����' in vms)
+ self.assertFalse('kimchi-vm' in vms)
@unittest.skipUnless(utils.running_as_root(), 'Must be run as root')
def test_vm_list_sorted(self):
diff --git a/tests/test_model_network.py b/tests/test_model_network.py
index 5dbe54d..c61f37c 100644
--- a/tests/test_model_network.py
+++ b/tests/test_model_network.py
@@ -99,6 +99,10 @@ class NetworkTests(unittest.TestCase):
def setUp(self):
self.request = partial(request, host, ssl_port)
+ def test_non_ascii_net_names(self):
+ net = {'name': u'k������h��-��et', 'connection': 'isolated'}
+ _do_network_test(self, model, net)
+
def test_get_networks(self):
networks = json.loads(self.request('/networks').read())
self.assertIn('default', [net['name'] for net in networks])
@@ -127,7 +131,7 @@ def test_get_networks(self):
def test_network_lifecycle(self):
# Verify all the supported network type
- networks = [{'name': u'k������h��-��et', 'connection': 'isolated'},
+ networks = [{'name': u'kimchi-net', 'connection': 'isolated'},
{'name': u'nat-network', 'connection': 'nat'},
{'name': u'subnet-network', 'connection': 'nat',
'subnet': '127.0.100.0/24'}]
diff --git a/tests/test_model_storagepool.py b/tests/test_model_storagepool.py
index eabf875..de4469d 100644
--- a/tests/test_model_storagepool.py
+++ b/tests/test_model_storagepool.py
@@ -60,6 +60,33 @@ class StoragepoolTests(unittest.TestCase):
def setUp(self):
self.request = partial(request, host, ssl_port)
+ def test_non_ascii_storagepool_name(self):
+ storagepools = json.loads(self.request('/storagepools').read())
+ self.assertIn('default', [pool['name'] for pool in storagepools])
+
+ with RollbackContext() as rollback:
+ name = u'k������h��-storagepool'
+ req = json.dumps({'name': name, 'type': 'dir',
+ 'path': '/var/lib/libvirt/images/test_pool'})
+ resp = self.request('/storagepools', req, 'POST')
+ rollback.prependDefer(model.storagepool_delete, name)
+
+ self.assertEquals(201, resp.status)
+
+ # Verify pool information
+ resp = self.request('/storagepools/%s' % name.encode("utf-8"))
+ p = json.loads(resp.read())
+ keys = [u'name', u'state', u'capacity', u'allocated',
+ u'available', u'path', u'source', u'type',
+ u'nr_volumes', u'autostart', u'persistent']
+ self.assertEquals(sorted(keys), sorted(p.keys()))
+ self.assertEquals(name, p['name'])
+ resp = self.request('/storagepools/%s/activate'
+ % name.encode('utf-8'), req, 'POST')
+ rollback.prependDefer(model.storagepool_deactivate, name)
+ p = json.loads(resp.read())
+ self.assertEquals('active', p['state'])
+
def test_get_storagepools(self):
storagepools = json.loads(self.request('/storagepools').read())
self.assertIn('default', [pool['name'] for pool in storagepools])
diff --git a/tests/test_model_storagevolume.py b/tests/test_model_storagevolume.py
index a3c3ce3..39531e9 100644
--- a/tests/test_model_storagevolume.py
+++ b/tests/test_model_storagevolume.py
@@ -1,4 +1,3 @@
-# -*- coding: utf-8 -*-
#
# Project Kimchi
#
--
2.1.0
9 years, 9 months
[PATCH] Change python #! line to be more portable
by Alan Jenkins
Hey guys,
This is a patch to change the #! line used in Kimchi from the ambiguous
/usr/bin/python to a more specific and more portable /usr/bin/env python2.
This enables users of distributions that have switched to the default python version
to python3 to run Kimchi without having to go and manually change the line and at the
same time allows people who have python installed to non default locations to also use
Kimchi by setting the python2 environment variable.
Thanks,
Alan Jenkins (demon012)
Alan Jenkins (1):
Change from using /usr/bin/python to /usr/bin/env python2 to improve
portability.
contrib/check_i18n.py | 2 +-
src/kimchi/vnc.py | 2 +-
src/kimchid.in | 2 +-
ui/pages/help/gen-index.py | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
--
2.2.2
9 years, 9 months
[PATCH v2 0/5] Create VMs Asynchronously
by Christy Perez
If a guest has a large disk, and uses a filesystem that requires
preallocation, it can take several minutes to create a VM. During that
time, kimchi is tied up by the VM creation.
This patch changes the VMs Collection to be an AsyncCollection.
Another change required for this was to create a more granular way to
query tasks. Currently it is not possible (using the API) to query tasks
for the same collection or resource type that may have different operations.
For example, VM cloning is also an asynchronous operation. For the guests tab,
the UI was querying all running tasks and displaying them with the Cloning
label. This picked up VMs that were being created as well. For more information
about how the tasks can be queried, see the updated API doc and the UI change.
Christy Perez (5):
Granular Task Queries: Backend
Granular task queries test updates
More Granular Task Queries: UI
Create guests asynchronously: Backend
Async vm creation test updates
docs/API.md | 14 +++++++
src/kimchi/asynctask.py | 6 ++-
src/kimchi/control/vms.py | 4 +-
src/kimchi/mockmodel.py | 7 ++--
src/kimchi/model/debugreports.py | 4 +-
src/kimchi/model/host.py | 4 +-
src/kimchi/model/storagepools.py | 2 +-
src/kimchi/model/storagevolumes.py | 7 ++--
src/kimchi/model/tasks.py | 1 -
src/kimchi/model/vms.py | 33 ++++++++++++---
src/kimchi/model/vmsnapshots.py | 3 +-
src/kimchi/utils.py | 4 +-
tests/test_authorization.py | 23 ++++++-----
tests/test_mockmodel.py | 15 +++++--
tests/test_model.py | 62 ++++++++++++++++++----------
tests/test_rest.py | 82 ++++++++++++++++++++++++++++----------
ui/js/src/kimchi.guest_main.js | 2 +-
17 files changed, 192 insertions(+), 81 deletions(-)
--
2.1.0
9 years, 9 months
[PATCH] issue #545: Handle simultaneous authentication methods when updating VM permission
by Crístian Viana
If a VM has one authentication method in its metadata (e.g. 'pam') and
a user sets some permissions with a different authentication method
(e.g. 'ldap'), then the group of the first authentication method is
lost and only the last type is kept. In other words, it is not possible
to have more than one authenticatin type in a VM's metadata.
Add the ability to handle different authentication methods
simultaneously when updating VM permission. Whatever is already set will
always be kept, even if the user chooses a different method.
Fix issue #545 (VM permission data is lost when changing authentication
method).
Signed-off-by: Crístian Viana <vianac(a)linux.vnet.ibm.com>
---
src/kimchi/model/vms.py | 54 ++++++++++++++++++++++++++++++++++++-------------
1 file changed, 40 insertions(+), 14 deletions(-)
diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py
index 379c850..018df9e 100644
--- a/src/kimchi/model/vms.py
+++ b/src/kimchi/model/vms.py
@@ -552,18 +552,48 @@ class VMModel(object):
# remove the new object store entry should an error occur later
rollback.prependDefer(_rollback_objstore)
- def _build_access_elem(self, users, groups):
+ def _build_access_elem(self, dom, users, groups):
auth = config.get("authentication", "method")
- auth_elem = E.auth(type=auth)
- for user in users:
- auth_elem.append(E.user(user))
+ access_xml = get_metadata_node(dom, "access",
+ self.caps.metadata_support)
- for group in groups:
- auth_elem.append(E.group(group))
+ auth_elem = None
- access = E.access()
- access.append(auth_elem)
- return access
+ if not access_xml:
+ # there is no metadata element 'access'
+ access_elem = E.access()
+ else:
+ access_elem = ET.fromstring(access_xml)
+
+ same_auth = access_elem.xpath('./auth[@type="%s"]' % auth)
+ if len(same_auth) > 0:
+ # there is already a sub-element 'auth' with the same type;
+ # update it.
+ auth_elem = same_auth[0]
+
+ if users is not None:
+ for u in auth_elem.findall('user'):
+ auth_elem.remove(u)
+
+ if groups is not None:
+ for g in auth_elem.findall('group'):
+ auth_elem.remove(g)
+
+ if auth_elem is None:
+ # there is no sub-element 'auth' with the same type
+ # (or no 'auth' at all); create it.
+ auth_elem = E.auth(type=auth)
+ access_elem.append(auth_elem)
+
+ if users is not None:
+ for u in users:
+ auth_elem.append(E.user(u))
+
+ if groups is not None:
+ for g in groups:
+ auth_elem.append(E.group(g))
+
+ return access_elem
def _vm_update_access_metadata(self, dom, params):
users = groups = None
@@ -583,11 +613,7 @@ class VMModel(object):
if users is None and groups is None:
return
- old_users, old_groups = self._get_access_info(dom)
- users = old_users if users is None else users
- groups = old_groups if groups is None else groups
-
- node = self._build_access_elem(users, groups)
+ node = self._build_access_elem(dom, users, groups)
set_metadata_node(dom, node, self.caps.metadata_support)
def _get_access_info(self, dom):
--
2.1.0
9 years, 9 months
