Skip to content
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

librbd: Truncate of non-existent object results in object map flagged… #7772

Merged
merged 2 commits into from Mar 9, 2016

Conversation

xinxinsh
Copy link

… as exists

Fixes: #14789

Signed-off-by: xinxin shu xinxin.shu@intel.com

@dillaman
Copy link

I think this fixes the object map, but it would still send a truncate op to the OSD for a non-existent object. It would be nice to avoid that extra op if the object map says it isn't required.

@dillaman dillaman self-assigned this Feb 24, 2016
… as exists

Fixes: ceph#14789

Signed-off-by: xinxin shu <xinxin.shu@intel.com>
@dillaman
Copy link

dillaman commented Mar 3, 2016

@xinxinsh
Copy link
Author

xinxinsh commented Mar 7, 2016

@dillaman , updated

@@ -434,12 +434,12 @@ namespace librbd {
}

bool AbstractAioObjectWrite::send_post() {
RWLock::RLocker owner_locker(m_ictx->owner_lock);
RWLock::RLocker snap_locker(m_ictx->snap_lock);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe snap_lock is used to protect m_ictx->object_map during image refreshes, so it can't just be moved. You might need to do something like this to avoid the lock recursion: https://github.com/ceph/ceph/blob/master/src/librbd/AioObjectRequest.cc#L219

@xinxinsh
Copy link
Author

xinxinsh commented Mar 8, 2016

@dillaman , ok, thanks for your hint, one question, if snap_lock is used to protect m_ictx->object_map, what does m_ictx->object_map_lock protect

@dillaman
Copy link

dillaman commented Mar 8, 2016

@xinxinsh That's used to protect the contents of the object map from being updated concurrently.

Signed-off-by: xinxin shu <xinxin.shu@intel.com>
@xinxinsh
Copy link
Author

xinxinsh commented Mar 8, 2016

@dillaman , updated

@jdurgin
Copy link
Member

jdurgin commented Mar 9, 2016

lgtm now, running through testing

jdurgin added a commit that referenced this pull request Mar 9, 2016
librbd: truncate does not need to mark the object as existing in the object map

Reviewed-by: Josh Durgin <jdurgin@redhat.com>
@jdurgin jdurgin merged commit 3538f11 into ceph:master Mar 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants