[PATCH] Bug fix: Allow user creates multiple templates
While trying to create multiple templates I got the following error on console: [08/Jun/2015:12:57:23] HTTP Traceback (most recent call last): File "/usr/lib/python2.7/dist-packages/cherrypy/_cprequest.py", line 670, in respond response.body = self.handler() File "/usr/lib/python2.7/dist-packages/cherrypy/lib/encoding.py", line 217, in __call__ self.body = self.oldhandler(*args, **kwargs) File "/usr/lib/python2.7/dist-packages/cherrypy/_cpdispatch.py", line 61, in __call__ return self.callable(*self.args, **self.kwargs) File "/home/alinefm/kimchi/src/kimchi/control/base.py", line 330, in index return self.create(parse_request(), *args) File "/home/alinefm/kimchi/src/kimchi/control/base.py", line 262, in create name = create(*args) File "/home/alinefm/kimchi/src/kimchi/model/templates.py", line 48, in create user = UserTests().probe_user() File "/home/alinefm/kimchi/src/kimchi/kvmusertests.py", line 55, in probe_user flags=libvirt.VIR_DOMAIN_START_AUTODESTROY) File "/usr/lib/python2.7/dist-packages/libvirt.py", line 3424, in createXML if ret is None:raise libvirtError('virDomainCreateXML() failed', conn=self) libvirtError: operation failed: domain 'KVMUSERTEST_VM' already exists with uuid 2be4b2e8-f57a-4f87-8c24-5ac59d4bb4af The error happens because of a race condition while trying to get the kvm user (to validate the ISO file permissions). To avoid this problem, a lock was added to ensure the code is run once at a time. Signed-off-by: Aline Manera <alinefm@linux.vnet.ibm.com> --- src/kimchi/kvmusertests.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/kimchi/kvmusertests.py b/src/kimchi/kvmusertests.py index 37a80d7..315354e 100644 --- a/src/kimchi/kvmusertests.py +++ b/src/kimchi/kvmusertests.py @@ -18,6 +18,7 @@ import platform import psutil +import threading import libvirt @@ -36,18 +37,21 @@ class UserTests(object): <boot dev='hd'/> </os> </domain>""" + lock = threading.Lock() user = None @classmethod def probe_user(cls): - if cls.user: - return cls.user + with cls.lock: + if cls.user: + return cls.user arch = 'ppc64' if platform.machine() == 'ppc64le' \ else platform.machine() xml = cls.SIMPLE_VM_XML % {'name': KVMUSERTEST_VM_NAME, 'arch': arch} + cls.lock.acquire() with RollbackContext() as rollback: conn = libvirt.open(None) rollback.prependDefer(conn.close) @@ -67,6 +71,7 @@ class UserTests(object): else: cls.user = p.username + cls.lock.release() return cls.user -- 2.1.0
On 08-06-2015 15:27, Aline Manera wrote:
+ cls.lock.acquire() with RollbackContext() as rollback: conn = libvirt.open(None) rollback.prependDefer(conn.close) @@ -67,6 +71,7 @@ class UserTests(object): else: cls.user = p.username
+ cls.lock.release() return cls.user
The function "cls.lock.release()" may not be called if an exception is raised from inside the rollback block, and the lock will be acquired forever. You should enclose the "with rollback" block with a "with cls.lock" block as well, just like you did a few lines above. In that case, the "with cls.lock" block will release the lock regardless of how the block ends.
On 09/06/2015 10:54, Crístian Deives wrote:
On 08-06-2015 15:27, Aline Manera wrote:
+ cls.lock.acquire() with RollbackContext() as rollback: conn = libvirt.open(None) rollback.prependDefer(conn.close) @@ -67,6 +71,7 @@ class UserTests(object): else: cls.user = p.username + cls.lock.release() return cls.user
The function "cls.lock.release()" may not be called if an exception is raised from inside the rollback block, and the lock will be acquired forever. You should enclose the "with rollback" block with a "with cls.lock" block as well, just like you did a few lines above. In that case, the "with cls.lock" block will release the lock regardless of how the block ends.
OK.
participants (2)
-
Aline Manera -
Crístian Deives