librbd: enabling/disabling rbd feature should report missing dependency #12238
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vshankar
changed the title
vshankar
reviewed
Dec 1, 2016
| lderr(cct) << "cannot disable exclusive lock" << dendl; | ||
| lderr(cct) << "cannot disable exclusive-lock feature. feature " | ||
| "object-map or journaling must be disabled " | ||
| "before disabling exclusive-lock." << dendl; |
There was a problem hiding this comment.
minor: better to keep it as "cannot disable exclusive-lock. object-map and journaling ..." rather than using "feature" word everytime?
applies to changes elsewhere.
There was a problem hiding this comment.
Right, change available in next PR.
gaurav36
force-pushed
the
bug-rbd-feature-enable-error
branch
from
e1298d4
to
61d0790
Compare
dillaman
suggested changes
Dec 1, 2016
| @@ -189,7 +190,8 @@ Context *EnableFeaturesRequest<I>::handle_get_mirror_mode(int *result) { | |||
| } | |||
| if ((m_features & RBD_FEATURE_FAST_DIFF) != 0) { | |||
| if ((m_new_features & RBD_FEATURE_OBJECT_MAP) == 0) { | |||
| lderr(cct) << "cannot enable fast diff" << dendl; | |||
| lderr(cct) << "cannot enable fast-diff. object-map must be" | |||
| " enabled before enabling fast-diff" << dendl; | |||
There was a problem hiding this comment.
Nit: move the space back to the end of the previous line's string. Comment applies below as well.
| @@ -190,7 +192,8 @@ Context *DisableFeaturesRequest<I>::handle_acquire_exclusive_lock(int *result) { | |||
| } | |||
| if ((m_features & RBD_FEATURE_OBJECT_MAP) != 0) { | |||
| if ((m_new_features & RBD_FEATURE_FAST_DIFF) != 0) { | |||
| lderr(cct) << "cannot disable object map" << dendl; | |||
| lderr(cct) << "cannot disable object-map. fast-diff must be " | |||
| "disabled before disabling object-map" << dendl; | |||
There was a problem hiding this comment.
Nit: inconsistent use of periods -- add a period to the end of the last sentence here and below where needed.
Currently while enabling or disabling rbd feature command does not give missing dependency for eg: attempting to enable the journaling feature on an image that doesn't have the exclusive-lock feature enabled should give missing dependency error message. Fixes: http://tracker.ceph.com/issues/16985 Reported-by: Jason Dillaman <dillaman@redhat.com> Signed-off-by: Gaurav Kumar Garg <garg.gaurav52@gmail.com>
gaurav36
force-pushed
the
bug-rbd-feature-enable-error
branch
from
61d0790
to
bd023cf
Compare
dillaman
approved these changes
Dec 2, 2016
|
retest this please |
|
jenkins test this please (https://jenkins.ceph.com/job/ceph-pull-requests/15202/consoleText unittest_ceph_crypto) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently while enabling or disabling rbd feature command does not
give missing dependency for eg: attempting to enable the journaling
feature on an image that doesn't have the exclusive-lock feature
enabled should give missing dependency error message.
Fixes: http://tracker.ceph.com/issues/16985
Reported-by: Jason Dillaman dillaman@redhat.com
Signed-off-by: Gaurav Kumar Garg garg.gaurav52@gmail.com