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: permit removal of image being bootstrapped by rbd-mirror #12549

Merged
merged 1 commit into from Jan 6, 2017

Conversation

Projects
None yet
2 participants
@trociny
Contributor

trociny commented Dec 17, 2016

Fixes: http://tracker.ceph.com/issues/16555
Signed-off-by: Mykola Golub mgolub@mirantis.com

@@ -163,6 +163,46 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
return 0;
}
void filter_out_mirror_watchers(ImageCtx *ictx,
std::list<obj_watch_t> &watchers) {

This comment has been minimized.

@dillaman

dillaman Dec 20, 2016

Contributor

Nit: use a pointer instead of a reference for an in-out parameter.

m_ictx->md_ctx.notify_ack(RBD_MIRRORING, notify_id, cookie, bl);
});
m_closed = true;
m_ictx->state->close(ctx);

This comment has been minimized.

@dillaman

dillaman Dec 20, 2016

Contributor

Probably overkill since rbd-mirror will never be able to close the image before acking the notification.

Mykola Golub
@trociny

This comment has been minimized.

Contributor

trociny commented Dec 20, 2016

@dillaman Updated.

@dillaman

lgtm

@dillaman

This comment has been minimized.

Contributor

dillaman commented Jan 3, 2017

@trociny I see some random error messages popping up in the rbd-mirror daemon logs, which could be awkward if running "systemctl status rbd-mirror@XYZ" and you see lots of false errors listed. I think if this PR is combined w/ my PR for listening to the add/remove mirroring events, it might vastly reduce the chance for such false errors since the bootstrap will be canceled nearly immediately when the "DISABLING" event is received. Perhaps remove should wait for all rbd-mirror watchers to disappear before proceeding (and time-out after X seconds if they don't)? Maybe I am thinking too much into it and we can deal with that later.

@trociny

This comment has been minimized.

Contributor

trociny commented Jan 3, 2017

@dillaman So, do you propose before checking for watchers (or if the check returned positive) and the mirroring is enabled to send DISABLING event and (re) check the watchers (expecting that if it was due to bootstrap then it would be cancelled)? I can do this way, rebasing it on your PR #12364 (and adding necessary bootsrap cancel bits if needed).

On the other hand, we already send DISABLING event when disabling mirror before actually removing the image. So after the rbd-mirror is updated to cancel the bootstrap when receiving DISABLING event, I suppose the errors should gone with my current version too?

I don't have a strong opinion what is better though.

@dillaman

This comment has been minimized.

Contributor

dillaman commented Jan 3, 2017

@trociny I think the chance of the race condition will be reduced (where bootstrap can complain about the image disappearing) ... and that it probably enough for the time being.

@dillaman

This comment has been minimized.

Contributor

dillaman commented Jan 5, 2017

retest this please

@dillaman dillaman merged commit a159f3d into ceph:master Jan 6, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment