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

messages/MOSDMap: mark as enlighten OSDMap encoder #10843

Merged
merged 1 commit into from Sep 1, 2016

Conversation

liewegas
Copy link
Member

Back in 7009440 we require that anyone
encoding an OSDMap pass in a special feature bit indicating that they are
'enlightened' and understand the rules around encoding OSDMaps (basically,
only mons get to do it). We forgot to update MOSDMap, which may have to
reencode an OSDMap without some features to talk to a really old client.
We haven't noticed until now because the old set of features we had to do
this for are really old.

We are about to introduce new features (for addr2 encoding) that change
that, and this fix is needed to prevent an assert when doing a reencode.

Signed-off-by: Sage Weil sage@redhat.com

@yuriw
Copy link
Contributor

yuriw commented Aug 24, 2016

@tchaikov waiting for thumb up and needs-qa tag and then will retest

@jdurgin
Copy link
Member

jdurgin commented Aug 24, 2016

lgtm

@tchaikov
Copy link
Contributor

quote from the commit message of 7009440,

Note that we skip this check for legacy encodings, so we don't touch MOSDMap in this patch.

and in the commit message in this PR

We forgot to update MOSDMap, which may have to reencode an OSDMap without some features to talk to a really old client. We haven't noticed until now because the old set of features we had to do this for are really old.

so we are basically dropping the support of old clients which do not understand these "really old" features bits? because the feature passed in MOSDMap::encode_payload() is the intersection of the features of both parties of the connection. so if we are sending OSDMaps not understood by the client, client will fail to decode the received MOSDMaps. and this renders the re-encoding logic meaningless.

@liewegas
Copy link
Member Author

We're not changing any behavior for old clients. The problem is just that the "old clients" and "really old" features are the branch of MOSDMap that does a reencode, and they all used to be old.. but now we've added a new feature bit that requires a reencode and does do the 'enlightened encoder' assert.

Back in 7009440 we require that anyone
encoding an OSDMap pass in a special feature bit indicating that they are
'enlightened' and understand the rules around encoding OSDMaps (basically,
only mons get to do it).  We forgot to update MOSDMap, which may have to
reencode an OSDMap without some features to talk to a really old client.
We haven't noticed until now because the old set of features we had to do
this for are really old.

We are about to introduce new features (for addr2 encoding) that change
that, and this fix is needed to prevent an assert when doing a reencode.

Signed-off-by: Sage Weil <sage@redhat.com>
@yuriw
Copy link
Contributor

yuriw commented Aug 30, 2016

FYI
The following tests FAILED:
5 - cephtool-test-mon.sh (Failed)
14 - run-tox-ceph-disk (Failed)
15 - run-tox-ceph-detect-init (Failed)

This ^ was b/c incorrect reg exp match issue, so tests actually passed per @jdurgin

@liewegas
Copy link
Member Author

The same tests passed when run by jenkins. I suspect an environment issue on your machine?

@yuriw
Copy link
Contributor

yuriw commented Aug 30, 2016

tests on master passed on smithi049 tho ?!

@tchaikov
Copy link
Contributor

tchaikov commented Sep 1, 2016

lgtm.

@liewegas liewegas merged commit 903f94e into ceph:master Sep 1, 2016
@liewegas liewegas deleted the wip-osdmap-reencode branch September 1, 2016 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants