Bug 508233 - SAF Test lck: saLckResourceClose/5.c
SAF Test lck: saLckResourceClose/5.c
Product: Fedora
Classification: Fedora
Component: openais (Show other bugs)
All Linux
low Severity medium
: ---
: ---
Assigned To: Ryan O'Hara
Fedora Extras Quality Assurance
Depends On:
  Show dependency treegraph
Reported: 2009-06-26 05:42 EDT by Jan Friesse
Modified: 2009-06-29 15:23 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2009-06-29 15:23:55 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)
Cancel callbacks is resource handle is no longer valid. (9.17 KB, patch)
2009-06-29 13:12 EDT, Ryan O'Hara
no flags Details | Diff
New saLckResourceClose/5.c test case. (4.62 KB, text/x-csrc)
2009-06-29 13:22 EDT, Ryan O'Hara
no flags Details

  None (edit)
Description Jan Friesse 2009-06-26 05:42:42 EDT
Description of problem:
Test in SUBJ doesn't work

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

How reproducible:
Run it

Steps to Reproduce:

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-26 23:29:28 EDT
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 13:08:16 EDT
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 13:12:02 EDT
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 13:22:54 EDT
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 15:23:55 EDT
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.

Note You need to log in before you can comment on or make changes to this bug.