[Kimchi-devel] [PATCH 10/10] bug fix: Lock yum/apt operations
Aline Manera
alinefm at linux.vnet.ibm.com
Wed Mar 19 23:56:57 UTC 2014
On 03/19/2014 10:03 AM, Aline Manera wrote:
> 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
Sorry, let me explain better.
The race condition is avoided in the cherrypy level
The problem was identified when Kimchi makes the /host/packagesupdate
and /host/repositories
requests at the same time.
You can verify this race condition in 2 terminals:
- in one of them run "yum check-update"
- in other one run "yum repolist all"
The first one will run properly:
[root at localhost ~]# yum check-update
Loaded plugins: langpacks, refresh-packagekit
But the second one will wait to lock be released.
[root at localhost ~]# yum repolist all
Loaded plugins: langpacks, refresh-packagekit
Existing lock /var/run/yum.pid: another copy is running as pid 1703.
Another app is currently holding the yum lock; waiting for it to exit...
The other application is: yum
Memory : 36 M RSS (355 MB VSZ)
Started: Wed Mar 19 20:53:30 2014 - 00:02 ago
State : Uninterruptible, pid: 1703
Another app is currently holding the yum lock; waiting for it to exit...
The other application is: yum
Memory : 45 M RSS (364 MB VSZ)
Started: Wed Mar 19 20:53:30 2014 - 00:04 ago
State : Running, pid: 1703
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
>
>>>
>>> [...]
>>> 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)
>>
>>
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>
More information about the Kimchi-devel
mailing list