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

rbd-mirror: prevent enabling/disabling an image's mirroring when not in image mode #8332

Merged
merged 3 commits into from Mar 29, 2016

Conversation

rjfd
Copy link
Contributor

@rjfd rjfd commented Mar 28, 2016

@@ -183,7 +183,9 @@ namespace librbd {
int mirror_peer_set_cluster(IoCtx& io_ctx, const std::string &uuid,
const std::string &cluster_name);

int mirror_image_enable_internal(ImageCtx *ictx);

Choose a reason for hiding this comment

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

Any reason to expose these outside of the file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, no good reason. I'll move them to the "anonymous namespace" block

@dillaman dillaman added this to the jewel milestone Mar 28, 2016
@dillaman
Copy link

lgtm -- just the minor nitpick

@dillaman dillaman changed the title rbd-mirror: Enabling/Disabling image mirroring must respect pool mirroring mode rbd-mirror: prevent enabling/disabling an image's mirroring when not in image mode Mar 28, 2016
@rjfd
Copy link
Contributor Author

rjfd commented Mar 29, 2016

Fixed the commits and rebased on top of master.

@rjfd
Copy link
Contributor Author

rjfd commented Mar 29, 2016

@dillaman this PR will conflict with the changes you make in your PR #8287, so I think this PR should be merged only after your PR is merged.
Please tell me when your PR is merged so I can resolve the conflicts and push the fix.
Thanks

@dillaman
Copy link

@rjfd Is it just a minor conflict or were you planning structural changes post-merge of PR #8287? If it's just minor, I can easily just merge yours and fix the conflicts in mine.

@rjfd
Copy link
Contributor Author

rjfd commented Mar 29, 2016

@dillaman it's just a minor conflict with the mirror_image_enable function. I was just trying to save you from the trouble of fixing conflicts, but I'm good either way.

@dillaman
Copy link

@rjfd The TestMirroring.* test cases are failing when I tested this locally.

[ FAILED ] TestMirroring.EnableImageMirror_In_MirrorModePool
[ FAILED ] TestMirroring.EnableImageMirror_In_MirrorModeDisabled
[ FAILED ] TestMirroring.DisableImageMirror_In_MirrorModeImage
[ FAILED ] TestMirroring.DisableImageMirror_In_MirrorModePool
[ FAILED ] TestMirroring.DisableImageMirror_In_MirrorModeDisabled
[ FAILED ] TestMirroring.EnableImageMirror_WithoutJournaling
[ FAILED ] TestMirroring.CreateImage_In_MirrorModeDisabled
[ FAILED ] TestMirroring.CreateImage_In_MirrorModeImage
[ FAILED ] TestMirroring.CreateImage_In_MirrorModePool
[ FAILED ] TestMirroring.CreateImage_In_MirrorModePool_WithoutJournaling
[ FAILED ] TestMirroring.CreateImage_In_MirrorModeImage_WithoutJournaling
[ FAILED ] TestMirroring.EnableJournaling_In_MirrorModeDisabled
[ FAILED ] TestMirroring.EnableJournaling_In_MirrorModeImage
[ FAILED ] TestMirroring.EnableJournaling_In_MirrorModePool
[ FAILED ] TestMirroring.DisableJournaling_In_MirrorModePool
[ FAILED ] TestMirroring.DisableJournaling_In_MirrorModeImage
[ FAILED ] TestMirroring.MirrorModeSet_DisabledMode_To_PoolMode
[ FAILED ] TestMirroring.MirrorModeSet_ImageMode_To_DisabledMode

…g mode

Signed-off-by: Ricardo Dias <rdias@suse.com>
… mode

Signed-off-by: Ricardo Dias <rdias@suse.com>
Fixes: ceph#15267
@rjfd
Copy link
Contributor Author

rjfd commented Mar 29, 2016

@dillaman The failures were caused by the merge of #8261. Going to resolve this...

…ew conditions to enable/disable image mirroring

Signed-off-by: Ricardo Dias <rdias@suse.com>
@rjfd
Copy link
Contributor Author

rjfd commented Mar 29, 2016

@dillaman fixed the unit tests.

@dillaman dillaman merged commit e94e779 into ceph:master Mar 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants