Red Hat Bugzilla – Bug 506529
SAF Test lck: SaLckResourceUnlockCallbackT/7
Last modified: 2009-06-27 10:26:26 EDT
Created attachment 348290 [details] 7.c Description of problem: SSIA Version-Release number of selected component (if applicable): How reproducible: Run test Steps to Reproduce: 1. 2. 3. Actual results: Does not conform the expected behaviors! saLckResourceUnlock, Return value: SA_AIS_ERR_NOT_EXIST, should be SA_AIS_OK callback return should be SA_AIS_ERR_NOT_EXIST Expected results: Test will run ok Additional info: Code is not much clear, but it looks like: saLckResourceUnlockAsync is called select wait for event saLckResourceUnlock is called - should unlock saLckDispatch - should run real saLckResourceUnlockAsync
Created attachment 348399 [details] Another test with similar problem Form my point of view, included file has some problem as original.
In regards to the bug with SaLckResourceUnlockCallbackT/7.c ... This is a tricky problem. The issue is that we remove the lock in the exec/service level immediately in both saLckResourceUnlock and saLckResourceUnlockAsync. Any subsequent calls to Unlock/UnlockAsync result is SA_AIS_ERR_NOT_EXIST being passed back to the library from the exec. Also note that we remove the lockId form the database in the library as soon as either call returns from the exec without error. This might have to change. This test seems to suggest that we should not actually remove the lock until the callback is processed (in saLckDispatch). Honestly, that seems a bit strange to me, it is what we would have to do in order to pass this test. It is also important to note that the synchronous call, saLckResourceUnlock, takes a timeout value. Because of the way openais is implemented, we cna safely ignore this. The saLckResourceUnlock call never blocks, so we have no need for this timeout. Because of this details, saLckResourceUnlock and saLckResourceUnlockAsync behave exactly the same -- except that the async call generates a callback. At least that is how the code works now. So, we could make these two calls behave differently by deferring removal of the lock (for the async call) until the callback is processed. This seems pointless, but as I said it will allow us to pass this test. As far that the code for this test case is concerned, I believe that they are testing the following: 1) Create a lock. 2) Unlock the lock (async). 3) Unlock the lock (sync). 4) Process the callback from #2. For this test to work, the async unlock (step #2) will not actually remove the lock. Then (step #3) the synchronous unlock occurs and does actually remove the lock. Finally (step #4), the callback is processed but the lock is gone, thus we get SA_AIS_ERR_NOT_EXIST.
(In reply to comment #1) > Created an attachment (id=348399) [details] > Another test with similar problem > > Form my point of view, included file has some problem as original. Filled as separate BZ 508236
I'm attaching the patch I sent upstream that fixes this test case. Here is an explanation of said patch: The old code would remove a lock as soon as either Unlock or UnlockAsync received a result from the exec without error. The two calls were exactly the same except that the async call generated a callback. This patch changes saLckResourceUnlockAsync such that the lock is not removed from the database in the library until the callback it made. This patch also changes both saLckResourceUnlock and saLckResourceUnlockAsync such that if a lock is not found in the executive layer, we do not return an error. it is valid for the lock_id to exist in the library database but not the executive, as this is a symptom of having an async unlock do the unlock but not remove the lock from the database in the library because its callback has not yet been processed. Finally, saLckDispatch changes the way it deals with resource unlock callbacks. If the lock_id specified in the callback exists, we remove it. It it does not exist, it means that the lock was removed while the callback was waiting to be processed. In this case we return SA_AIS_ERR_NOT_EXIST, per the spec.
Created attachment 349619 [details] Change the way we do unlock operations. Here is the patch I sent upstream. It fixes the problems with SaLckResourceUnlockCallbackT/7.c test case.
Fixed upstream. OpenAIS trunk, revision 2005.