[Kimchi-devel] [PATCH 10/10] bug fix: Lock yum/apt operations
Aline Manera
alinefm at linux.vnet.ibm.com
Wed Mar 19 13:03:13 UTC 2014
On 03/19/2014 03:59 AM, Sheldon wrote:
> On 03/19/2014 01:04 AM, Aline Manera wrote:
>> From: Aline Manera <alinefm at 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:
> 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 at 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)
>
>
More information about the Kimchi-devel
mailing list