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

On 03/19/2014 03:59 AM, Sheldon wrote: > On 03/19/2014 01:04 AM, Aline Manera wrote: >> From: Aline Manera <alinefm(a)br.ibm.com&gt; >> >> 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: > On my fedora, the system can call yum automatically sometimes, > and the user can also call yum manully sometimes. > Do we need to avoid this race condition? The race condition happens on cherrypy level - with 2 requests at the same time

>> >> [...] >> 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(a)br.ibm.com&gt; >> --- >> 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) > > _______________________________________________ Kimchi-devel mailing list Kimchi-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel