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.

Sign up for GitHub

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

Jump to bottom

librbd: fix rollback if failed to disable mirroring for image #11260

Merged
merged 1 commit into from Oct 10, 2016
+33 −30
Merged

librbd: fix rollback if failed to disable mirroring for image #11260

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
63 changes: 33 additions & 30 deletions src/librbd/internal.cc
View file
Open in desktop
Expand Up @@ -2467,10 +2467,9 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
return r;
} else {
bool rollback = false;
BOOST_SCOPE_EXIT_ALL(ictx, rollback) {
BOOST_SCOPE_EXIT_ALL(ictx, &mirror_image_internal, &rollback) {
if (rollback) {
CephContext *cct = ictx->cct;
cls::rbd::MirrorImage mirror_image_internal;
mirror_image_internal.state = cls::rbd::MIRROR_IMAGE_STATE_ENABLED;
int r = cls_client::mirror_image_set(&ictx->md_ctx, ictx->id, mirror_image_internal);
if (r < 0) {
Expand All @@ -2480,46 +2479,50 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
}
};

RWLock::RLocker l(ictx->snap_lock);
map<librados::snap_t, SnapInfo> snap_info = ictx->snap_info;
for (auto &info : snap_info) {
librbd::parent_spec parent_spec(ictx->md_ctx.get_id(), ictx->id, info.first);
map< pair<int64_t, string>, set<string> > image_info;
{
RWLock::RLocker l(ictx->snap_lock);
map<librados::snap_t, SnapInfo> snap_info = ictx->snap_info;
for (auto &info : snap_info) {
librbd::parent_spec parent_spec(ictx->md_ctx.get_id(), ictx->id, info.first);
map< pair<int64_t, string>, set<string> > image_info;

r = list_children_info(ictx, parent_spec, image_info);
if (r < 0) {
rollback = true;
return r;
}
if (image_info.empty())
continue;

Rados rados(ictx->md_ctx);
for (auto &info: image_info) {
IoCtx ioctx;
r = rados.ioctx_create2(info.first.first, ioctx);
r = list_children_info(ictx, parent_spec, image_info);
if (r < 0) {
rollback = true;
lderr(cct) << "Error accessing child image pool " << info.first.second << dendl;
return r;
}
for (auto &id_it : info.second) {
cls::rbd::MirrorImage mirror_image_internal;
r = cls_client::mirror_image_get(&ioctx, id_it, &mirror_image_internal);
if (r != -ENOENT) {
if (image_info.empty())
continue;

Rados rados(ictx->md_ctx);
for (auto &info: image_info) {
IoCtx ioctx;
r = rados.ioctx_create2(info.first.first, ioctx);
if (r < 0) {
rollback = true;
lderr(cct) << "mirroring is enabled on one or more children " << dendl;
return -EBUSY;
lderr(cct) << "Error accessing child image pool " << info.first.second << dendl;
return r;
}
for (auto &id_it : info.second) {
cls::rbd::MirrorImage mirror_image_internal;
r = cls_client::mirror_image_get(&ioctx, id_it, &mirror_image_internal);
if (r != -ENOENT) {
rollback = true;
lderr(cct) << "mirroring is enabled on one or more children " << dendl;
return -EBUSY;
}
}
}
}
}
}

r = mirror_image_disable_internal(ictx, force);
if (r < 0) {
return r;
r = mirror_image_disable_internal(ictx, force);
Copy link

Choose a reason for hiding this comment

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

I believe this was outside the block due to snap_lock being held.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry, did not notice the lock :(
updated and moved the code needs the snap_lock into a block.
thanks.

if (r < 0) {
rollback = true;
return r;
}
}

return 0;
}

Expand Down