diff --git a/lib/locking.py b/lib/locking.py index 938c372df40890a7a212e3853495d67992bc552c..300462108c1fc4f76df887cfadd356848afcc0a8 100644 --- a/lib/locking.py +++ b/lib/locking.py @@ -512,8 +512,8 @@ class LockSet: you are already holding exclusively the locks. Returns: - A list of lock which we failed to delete. The list is always empty if we - were holding all the locks exclusively. + A list of lock which we removed. The list is always equal to the names + list if we were holding all the locks exclusively. """ if not blocking and not self._is_owned(): @@ -530,7 +530,7 @@ class LockSet: assert not self._is_owned() or self._list_owned().issuperset(names), ( "remove() on acquired lockset while not owning all elements") - delete_failed=[] + removed = [] for lname in names: # Calling delete() acquires the lock exclusively if we don't already own @@ -540,8 +540,8 @@ class LockSet: # everything we want to delete, or we hold none. try: self.__lockdict[lname].delete() + removed.append(lname) except (KeyError, errors.LockError): - delete_failed.append(lname) # This cannot happen if we were already holding it, verify: assert not self._is_owned(), "remove failed while holding lockset" else: @@ -557,7 +557,7 @@ class LockSet: if self._is_owned(): self._del_owned(lname) - return delete_failed + return removed # Locking levels, must be acquired in increasing order. diff --git a/test/ganeti.locking_unittest.py b/test/ganeti.locking_unittest.py index 1a661522d9ed5d765ed0b8987ad287f5f6f69272..74524ed0733bb2d05b4521d044a1d1e50771bfe0 100755 --- a/test/ganeti.locking_unittest.py +++ b/test/ganeti.locking_unittest.py @@ -291,7 +291,7 @@ class TestLockSet(unittest.TestCase): self.assert_('six' in self.ls._names()) self.assert_('seven' in self.ls._names()) self.assertEquals(self.ls._list_owned(), set(['five', 'six', 'seven'])) - self.ls.remove(['five', 'six']) + self.assertEquals(self.ls.remove(['five', 'six']), ['five', 'six']) self.assert_('five' not in self.ls._names()) self.assert_('six' not in self.ls._names()) self.assertEquals(self.ls._list_owned(), set(['seven'])) @@ -305,18 +305,19 @@ class TestLockSet(unittest.TestCase): self.ls.remove(['two']) self.assert_('two' not in self.ls._names()) self.ls.acquire('three') - self.ls.remove(['three']) + self.assertEquals(self.ls.remove(['three']), ['three']) self.assert_('three' not in self.ls._names()) - self.assertEquals(self.ls.remove('three'), ['three']) - self.assertEquals(self.ls.remove(['one', 'three', 'six']), ['three', 'six']) + self.assertEquals(self.ls.remove('three'), []) + self.assertEquals(self.ls.remove(['one', 'three', 'six']), ['one']) self.assert_('one' not in self.ls._names()) def testRemoveNonBlocking(self): self.assertRaises(NotImplementedError, self.ls.remove, 'one', blocking=0) self.ls.acquire('one') - self.assertEquals(self.ls.remove('one', blocking=0), []) + self.assertEquals(self.ls.remove('one', blocking=0), ['one']) self.ls.acquire(['two', 'three']) - self.assertEquals(self.ls.remove(['two', 'three'], blocking=0), []) + self.assertEquals(self.ls.remove(['two', 'three'], blocking=0), + ['two', 'three']) def testNoDoubleAdd(self): self.assertRaises(errors.LockError, self.ls.add, 'two') @@ -406,9 +407,9 @@ class TestLockSet(unittest.TestCase): Thread(target=self._doRemoveSet, args=(['four', 'six'], )).start() self.assertRaises(Queue.Empty, self.done.get, True, 0.2) self.ls.remove('four') - self.assertEqual(self.done.get(True, 1), ['four']) + self.assertEqual(self.done.get(True, 1), ['six']) Thread(target=self._doRemoveSet, args=(['two'])).start() - self.assertEqual(self.done.get(True, 1), []) + self.assertEqual(self.done.get(True, 1), ['two']) self.ls.release()