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

osd/OSDMap: fix HAVE_FEATURE logic in encode() #20922

Merged
merged 1 commit into from Mar 15, 2018

Conversation

idryomov
Copy link
Contributor

Currently clients that don't have SERVER_LUMINOUS end up being fed
v6 (i.e. luminous) encoding because they also don't have SERVER_MIMIC.
This was introduced in commit 553048f ("osd/OSDMap: track newly
removed and purged snaps in each epoch").

Signed-off-by: Ilya Dryomov idryomov@gmail.com

Currently clients that don't have SERVER_LUMINOUS end up being fed
v6 (i.e. luminous) encoding because they also don't have SERVER_MIMIC.
This was introduced in commit 553048f ("osd/OSDMap: track newly
removed and purged snaps in each epoch").

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
@gregsfortytwo
Copy link
Member

Yikes, this LGTM too.

Do we think the right pattern here is just to use “else if” statements as in this PR, or is there some other kind of meta-structure we can use that looks more different when correct?

@liewegas
Copy link
Member

i think else if is fine. i'm worried there are other cases like this we missed. checking my peering branch now!

@idryomov
Copy link
Contributor Author

I have checked master before submitting this PR, nothing else caught my eye.

@liewegas
Copy link
Member

yeah i don't see anything either. 👍

@idryomov
Copy link
Contributor Author

krbd test case on a 4.4 kernel, good to merge from the kernel client perspective: http://pulpito.ceph.com/dis-2018-03-15_18:48:40-krbd-wip-osdmap-encoding-fix-distro-basic-smithi/

@liewegas liewegas merged commit 1bd33de into ceph:master Mar 15, 2018
@idryomov idryomov deleted the wip-osdmap-encoding-fix branch March 15, 2018 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants