New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test/librbd: added update_features RPC message to test_notify #18561
Conversation
fe47641
to
ed636a3
Compare
src/test/librbd/test_notify.py
Outdated
with Image(ioctx, CLONE_IMG_NAME) as image: | ||
print("rename") | ||
RBD().rename(ioctx, CLONE_IMG_NAME, CLONE_IMG_RENAME); | ||
assert(not image.is_exclusive_lock_owner()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wandering if this assert is useful here. If the lock had been stolen it would have been stolen by the image context created by rename
internally?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the lock was stolen, that indicates that the RPC message wasn't sent to the lock owner (plus the "master" waits for its lock to get stolen to know the test is over via the "slave" issuing a write).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dillaman not sure I understand. I mean that if "rename" had stolen the lock by some reason, the assert not image.is_exclusive_lock_owner()
would have been still valid (the test would not fail at this point), because the lock is not stolen by this image context. Sure the test would fail later but I am talking only about usefulness of this assert. Am I missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah -- sorry, I see what you are saying. I was just trying to fix previous bug where image
wasn't even initialized. I'll just drop it.
Fixes: http://tracker.ceph.com/issues/21936 Signed-off-by: Jason Dillaman <dillaman@redhat.com>
ed636a3
to
b003ff3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Signed-off-by: Jason Dillaman dillaman@redhat.com