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

Sheldon shaohef at linux.vnet.ibm.com
Wed Mar 19 06:59:32 UTC 2014


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?
>
> [...]
>    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)


-- 
Thanks and best regards!

Sheldon Feng(冯少合)<shaohef at linux.vnet.ibm.com>
IBM Linux Technology Center




More information about the Kimchi-devel mailing list