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 : fix enable objectmap feature issue #6339

Merged
merged 1 commit into from Nov 5, 2015

Conversation

Projects
None yet
3 participants
@xinxinsh
Copy link
Member

xinxinsh commented Oct 21, 2015

Fixes: #13558

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

@xinxinsh

This comment has been minimized.

Copy link
Member Author

xinxinsh commented Nov 2, 2015

@dillaman , pls help review

@@ -1564,6 +1582,11 @@ int invoke_async_request(ImageCtx *ictx, const std::string& request_type,
lderr(cct) << "cannot enable object map" << dendl;
return -EINVAL;
}
if ((ictx->features & RBD_FEATURE_OBJECT_MAP) == 0) {

This comment has been minimized.

@dillaman

dillaman Nov 2, 2015

Contributor

I would create the object map after the feature bits have been updated on disk (just in case the feature bit update fails). I would also create an object map for each existing snapshot to avoid similar "failed to load object map" errors when opening a snapshot.

@dillaman dillaman self-assigned this Nov 2, 2015

@xinxinsh xinxinsh force-pushed the xinxinsh:wip-13558 branch from 11bd973 to 3d6a603 Nov 3, 2015

@@ -1575,13 +1605,6 @@ int invoke_async_request(ImageCtx *ictx, const std::string& request_type,
enable_flags |= RBD_FLAG_FAST_DIFF_INVALID;
features_mask |= (RBD_FEATURE_OBJECT_MAP | RBD_FEATURE_EXCLUSIVE_LOCK);
}

if (enable_flags != 0) {
r = update_all_flags(ictx, enable_flags, enable_flags);

This comment has been minimized.

@dillaman

dillaman Nov 4, 2015

Contributor

I wouldn't move this down -- we want to flag the object map as invalid before anyone gets a chance to use it.

@@ -1597,11 +1620,6 @@ int invoke_async_request(ImageCtx *ictx, const std::string& request_type,
}

disable_flags = RBD_FLAG_OBJECT_MAP_INVALID;
r = remove_object_map(ictx);

This comment has been minimized.

@dillaman

dillaman Nov 4, 2015

Contributor

I would keep this here as well. If we crash between removing the object map and clearing the feature bit, the image will still function w/ object map implicitly disabled. If we move this down, if we crash between clearing the bit and remove the object map, we leave a object behind.

xinxin shu
librbd : fix enable objectmap feature issue
Fixes: #13558

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

@xinxinsh xinxinsh force-pushed the xinxinsh:wip-13558 branch from 3d6a603 to b0536eb Nov 5, 2015

@xinxinsh

This comment has been minimized.

Copy link
Member Author

xinxinsh commented Nov 5, 2015

@dillaman , updated, help review

@dillaman

This comment has been minimized.

Copy link
Contributor

dillaman commented Nov 5, 2015

lgtm

dillaman added a commit that referenced this pull request Nov 5, 2015

Merge pull request #6339 from xinxinsh/wip-13558
librbd : fix enable objectmap feature issue

Reviewed-by: Jason Dillaman <dillaman@redhat.com>

@dillaman dillaman merged commit 72a9fb4 into ceph:master Nov 5, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.