Bug 508233

Summary: SAF Test lck: saLckResourceClose/5.c
Product: [Fedora] Fedora Reporter: Jan Friesse <jfriesse>
Component: openaisAssignee: Ryan O'Hara <rohara>
Status: CLOSED UPSTREAM QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: rawhideCC: agk, fdinitto, sdake
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-06-29 19:23:55 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
Cancel callbacks is resource handle is no longer valid.
none
New saLckResourceClose/5.c test case. none

Description Jan Friesse 2009-06-26 09:42:42 UTC
Description of problem:
Test in SUBJ doesn't work

Version-Release number of selected component (if applicable):
Trunk

How reproducible:
Run it

Steps to Reproduce:
1.
2.
3.

Actual results:
select error!

Expected results:
Test runs ok

Additional info:
It looks like saLckResourceClose doesn't cancel pending request lock

Comment 1 Ryan O'Hara 2009-06-27 03:29:28 UTC
It does cancel the lock request. However the  saLckResourceClose operations does not cancel pending callbacks. The reason this test fails is due to how we handle callbacks in OpenAIS.

As soon as the exec handle for saLckResourceOpenAsync, we do 2 things:

1. Send a response to the library.
2. Send a callback request to the library.

The callback can be processed by saLckDispatch at any time in the future. In this case, the callback request has been sent to the dispatcher and will be processed the next time that saLckDispatch is called. I see two options:

1. Ignore this test. For saLckResourceClose, the specification clearly states:

"This call cancels all pending callbacks that refer directly or indirectly to the handle lockResourceHandle. Note that because the callback invocation is asynchronous, it is still possible that some callback calls are processed after this call returns successfully."

So there it seems reasonable to process the callbacks, despite the fact that the resource has been closed. Note that if saLckResourceLockAsync was called, that lock was granted and then stripped as part of the Close operations, the lock grant callback will report status == SA_AIS_ERR_NOT_EXIST, which is valid. This is exactly what happens in this test case.

2. Check that the resource handle is still valid before we actually call the callback function. This seems like is might be the best choice, but it is important to note that it will not fix this test case because the select call will still see that there is data to be processed on the file descriptor. In other words, the callback request will still be processed, but we would chose to not actually call the callback if the resource handle is no longer valid.

Comment 2 Ryan O'Hara 2009-06-29 17:08:16 UTC
I am attaching a patch that checks that a resource handle is valid before making a callback. This check happens in saLckDispatch for all callbacks. Other code changes needed for this to work is passing the resource handle as part of the callback request, and for all API calls that generate a callback.

Comment 3 Ryan O'Hara 2009-06-29 17:12:02 UTC
Created attachment 349823 [details]
Cancel callbacks is resource handle is no longer valid.

Note that callbacks are still sent to dispatch. When saLckDispatch is called, we check that the resource handle is valid. If it is not, we simply skip the callback. The saftest test case in question here will still fail because it assumes that we can determine is a callback will be made by doing a select on the file descriptor. That is a error in the saftest code.

Comment 4 Ryan O'Hara 2009-06-29 17:22:54 UTC
Created attachment 349825 [details]
New saLckResourceClose/5.c test case.

Here is a new version of saLckResourceClose/5.c that does not use select to determine is a callback has been made.

Comment 5 Ryan O'Hara 2009-06-29 19:23:55 UTC
Closing this as fixed upstream. OpenAIS lock service now has code to cancel/skip callbacks for resources that have been closed with pending callbacks. Fix for saftest code it attached above.