[Kimchi-devel] [PATCH 10/10] bug fix: Lock yum/apt operations

18 Mar 2014

From: Aline Manera <alinefm@br.ibm.com>

Just one yum/apt process can run per time so we need to lock those
operations to avoid race condition problems.

yum has a built in lock but it does not work well in kimchi as it only
prevents the user to run two different YUM applications. As the kimchi threads
are part of the same process, YUM does not really acquire the lock and
then we get the error:

[...]
  File "/usr/lib/python2.7/site-packages/urlgrabber/grabber.py", line 1258, in __init__
    self._do_open()
  File "/usr/lib/python2.7/site-packages/urlgrabber/grabber.py", line 1588, in _do_open
    self._set_opts()
  File "/usr/lib/python2.7/site-packages/urlgrabber/grabber.py", line 1375, in _set_opts
    self.curl_obj.setopt(pycurl.NOPROGRESS, False)
error: cannot invoke setopt() - perform() is currently running

So create a kimchi lock to solve this problem.
This lock can be used in any other place when needed.

Signed-off-by: Aline Manera <alinefm@br.ibm.com>
---
 src/kimchi/config.py.in    |    3 +++
 src/kimchi/repositories.py |   39 ++++++++++++++++++++++++++++++++++++---
 src/kimchi/swupdate.py     |    7 +++++++
 3 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/src/kimchi/config.py.in b/src/kimchi/config.py.in
index 426fbd1..cf8497a 100644
--- a/src/kimchi/config.py.in
+++ b/src/kimchi/config.py.in
@@ -21,6 +21,7 @@
 import libvirt
 import os
 import platform
+import threading
 
 
 from ConfigParser import SafeConfigParser
@@ -31,6 +32,8 @@ from kimchi.xmlutils import xpath_get_text
 
 DEFAULT_LOG_LEVEL = "debug"
 
+kimchiLock = threading.Lock()
+
 # Storage pool constant for read-only pool types
 READONLY_POOL_TYPE = ['iscsi', 'scsi', 'mpath']
 
diff --git a/src/kimchi/repositories.py b/src/kimchi/repositories.py
index 70f4f5b..39dde12 100644
--- a/src/kimchi/repositories.py
+++ b/src/kimchi/repositories.py
@@ -24,6 +24,7 @@ import urlparse
 from ConfigParser import ConfigParser
 
 from kimchi.basemodel import Singleton
+from kimchi.config import kimchiLock
 from kimchi.exception import InvalidOperation
 from kimchi.exception import OperationFailed, NotFoundError, MissingParameter
 
@@ -118,14 +119,19 @@ class YumRepo(object):
         """
         Return a list of repositories IDs
         """
-        return self._yb().repos.repos.keys()
+        kimchiLock.acquire()
+        repos = self._yb().repos.repos.keys()
+        kimchiLock.release()
+        return repos
 
     def getRepo(self, repo_id):
         """
         Return a dictionary in the repositories.Repositories() of the given
         repository ID format with the information of a YumRepository object.
         """
+        kimchiLock.acquire()
         repos = self._yb().repos
+        kimchiLock.release()
         if repo_id not in repos.repos.keys():
             raise NotFoundError("KCHREPOS0012E", {'repo_id': repo_id})
 
@@ -162,7 +168,9 @@ class YumRepo(object):
         if repo_id is None:
             repo_id = "kimchi_repo_%s" % str(int(time.time() * 1000))
 
+        kimchiLock.acquire()
         repos = self._yb().repos
+        kimchiLock.release()
         if repo_id in repos.repos.keys():
             raise InvalidOperation("KCHREPOS0022E", {'repo_id': repo_id})
 
@@ -193,7 +201,9 @@ class YumRepo(object):
         return repo_id
 
     def toggleRepo(self, repo_id, enable):
+        kimchiLock.acquire()
         repos = self._yb().repos
+        kimchiLock.release()
         if repo_id not in repos.repos.keys():
             raise NotFoundError("KCHREPOS0012E", {'repo_id': repo_id})
 
@@ -204,6 +214,7 @@ class YumRepo(object):
         if not enable and not entry.enabled:
             raise InvalidOperation("KCHREPOS0016E", {'repo_id': repo_id})
 
+        kimchiLock.acquire()
         try:
             if enable:
                 entry.enable()
@@ -211,18 +222,23 @@ class YumRepo(object):
                 entry.disable()
 
             self._conf.writeRawRepoFile(entry)
-            return repo_id
         except:
             if enable:
                 raise OperationFailed("KCHREPOS0020E", {'repo_id': repo_id})
 
             raise OperationFailed("KCHREPOS0021E", {'repo_id': repo_id})
+        finally:
+            kimchiLock.release()
+
+        return repo_id
 
     def updateRepo(self, repo_id, params):
         """
         Update a given repository in repositories.Repositories() format
         """
+        kimchiLock.acquire()
         repos = self._yb().repos
+        kimchiLock.release()
         if repo_id not in repos.repos.keys():
             raise NotFoundError("KCHREPOS0012E", {'repo_id': repo_id})
 
@@ -242,14 +258,18 @@ class YumRepo(object):
         entry.name = config.get('repo_name', entry.name)
         entry.gpgcheck = config.get('gpgcheck', entry.gpgcheck)
         entry.gpgkey = config.get('gpgkey', entry.gpgkey)
+        kimchiLock.acquire()
         self._conf.writeRawRepoFile(entry)
+        kimchiLock.release()
         return repo_id
 
     def removeRepo(self, repo_id):
         """
         Remove a given repository
         """
+        kimchiLock.acquire()
         repos = self._yb().repos
+        kimchiLock.release()
         if repo_id not in repos.repos.keys():
             raise NotFoundError("KCHREPOS0012E", {'repo_id': repo_id})
 
@@ -298,8 +318,10 @@ class AptRepo(object):
         return '%s-%s-%s' % (name, repo.dist, "-".join(repo.comps))
 
     def _get_source_entry(self, repo_id):
+        kimchiLock.acquire()
         repos = self._sourceslist()
         repos.refresh()
+        kimchiLock.release()
 
         for r in repos:
             # Ignore deb-src repositories
@@ -321,8 +343,10 @@ class AptRepo(object):
         internal control, the repository ID will be built as described in
         _get_repo_id()
         """
+        kimchiLock.acquire()
         repos = self._sourceslist()
         repos.refresh()
+        kimchiLock.release()
 
         res = []
         for r in repos:
@@ -366,10 +390,12 @@ class AptRepo(object):
         dist = config['dist']
         comps = config.get('comps', [])
 
+        kimchiLock.acquire()
         repos = self._sourceslist()
         repos.refresh()
         source_entry = repos.add('deb', uri, dist, comps, file=self.filename)
         repos.save()
+        kimchiLock.release()
 
         return self._get_repo_id(source_entry)
 
@@ -392,18 +418,22 @@ class AptRepo(object):
         else:
             line = '#deb'
 
+        kimchiLock.acquire()
         try:
             repos = self._sourceslist()
             repos.refresh()
             repos.remove(r)
             repos.add(line, r.uri, r.dist, r.comps, file=self.filename)
             repos.save()
-            return repo_id
         except:
             if enable:
                 raise OperationFailed("KCHREPOS0020E", {'repo_id': repo_id})
 
             raise OperationFailed("KCHREPOS0021E", {'repo_id': repo_id})
+        finally:
+            kimchiLock.release()
+
+        return repo_id
 
     def updateRepo(self, repo_id, params):
         """
@@ -434,6 +464,7 @@ class AptRepo(object):
         if r is None:
             raise NotFoundError("KCHREPOS0012E", {'repo_id': repo_id})
 
+        kimchiLock.acquire()
         try:
             repos = self._sourceslist()
             repos.refresh()
@@ -441,3 +472,5 @@ class AptRepo(object):
             repos.save()
         except:
             raise OperationFailed("KCHREPOS0017E", {'repo_id': repo_id})
+        finally:
+            kimchiLock.release()
diff --git a/src/kimchi/swupdate.py b/src/kimchi/swupdate.py
index ff5c9d1..45f7920 100644
--- a/src/kimchi/swupdate.py
+++ b/src/kimchi/swupdate.py
@@ -21,6 +21,7 @@ import subprocess
 import time
 
 from kimchi.basemodel import Singleton
+from kimchi.config import kimchiLock
 from kimchi.exception import NotFoundError, OperationFailed
 from kimchi.utils import kimchi_log, run_command
 
@@ -152,7 +153,9 @@ class YumUpdate(object):
         package = {'package_name': <string>, 'version': <string>,
                    'arch': <string>, 'repository': <string>}
         """
+        kimchiLock.acquire()
         self._refreshUpdateList()
+        kimchiLock.release()
         pkg_list = []
         for pkg in self._pkgs:
             package = {'package_name': pkg.name,
@@ -188,7 +191,9 @@ class AptUpdate(object):
         package = {'package_name': <string>, 'version': <string>,
                    'arch': <string>, 'repository': <string>}
         """
+        kimchiLock.acquire()
         self._refreshUpdateList()
+        kimchiLock.release()
         pkg_list = []
         for pkg in self._pkgs:
             package = {'package_name': pkg.shortname,
@@ -235,7 +240,9 @@ class ZypperUpdate(object):
         package = {'package_name': <string>, 'version': <string>,
                    'arch': <string>, 'repository': <string>}
         """
+        kimchiLock.acquire()
         self._refreshUpdateList()
+        kimchiLock.release()
         pkg_list = []
         for pkg in self._pkgs:
             pkg_list.append(pkg)
-- 
1.7.10.4

    