Project

General

Profile

Bug #13234

dumpling incrementals do not work properly on hammer and newer

Added by Samuel Just about 7 years ago. Updated about 7 years ago.

Status:
Resolved
Priority:
Urgent
Assignee:
Category:
-
Target version:
-
% Done:

0%

Source:
Community (user)
Tags:
Backport:
hammer
Regression:
No
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:
Pull request ID:
Crash signature (v1):
Crash signature (v2):

Related issues

Copied to Ceph - Backport #13307: dumpling incrementals do not work properly on hammer and newer Resolved

Associated revisions

Revision 04679c54 (diff)
Added by Samuel Just about 7 years ago

OSDMap: fill in known encode_features where possible

Otherwise, if we get an incremental from hammer (struct_v = 6) we will
encode the full map as if it were before CEPH_FEATURE_PGID64, which
was actually pre-argonaut. Similarly, if struct_v >= 7, we know it
was encoded with CEPH_FEATURE_OSDMAP_ENC.

Fixes: #13234
Backport: hammer
Signed-off-by: Samuel Just <>

Revision 3c1f7cbc (diff)
Added by Samuel Just about 7 years ago

OSDMap: fill in known encode_features where possible

Otherwise, if we get an incremental from hammer (struct_v = 6) we will
encode the full map as if it were before CEPH_FEATURE_PGID64, which
was actually pre-argonaut. Similarly, if struct_v >= 7, we know it
was encoded with CEPH_FEATURE_OSDMAP_ENC.

Fixes: #13234
Backport: hammer
Signed-off-by: Samuel Just <>
(cherry picked from commit 04679c5451e353c966f6ed00b33fa97be8072a79)

History

#1 Updated by Samuel Just about 7 years ago

Dumpling OSDMap::Incremental encodings do not include encode_features. Hammer OSD::handle_osd_map when it attempts to reencode the full map after it gets an incremental map will feed that field (which is 0 for an incremental created in dumpling) to the fullmap encode function. This causes the fullmap to be encoded using encode_client_old. That's actually wrong since that will encode at version 5 even if the full should have been encoded at 6. Furthermore, decode_classic seems to be buggy for version 5 osdmaps leading to the crash (the encodes and decodes do not seem to match up in the v < 6 branch of decode_classic).

#2 Updated by Sage Weil about 7 years ago

  • Status changed from New to 7

#4 Updated by Sage Weil about 7 years ago

  • Status changed from 7 to Pending Backport
  • Source changed from other to Community (user)
  • Backport set to hammer

#5 Updated by Samuel Just about 7 years ago

The hammer version of the test is the ceph-qa-suite branch ceph/wip-13234-hammer. The hammer version of the fix is the ceph branch ceph/wip-13234-hammer.

#6 Updated by Ken Dreyer about 7 years ago

PR that went into infernalis/master: https://github.com/ceph/ceph/pull/6122

#7 Updated by Nathan Cutler about 7 years ago

Sam, thanks for doing the hammer backport1 for us. Do you intend to merge it directly into "hammer" from "wip-13234-hammer", or shall we make a PR out of it and subject it to our normal integration testing round prior to merge?

[1] https://github.com/ceph/ceph/commit/73ddba1ac23e8da58b5ac684cae9899a0cbce096

#8 Updated by Loïc Dachary about 7 years ago

  • Status changed from Pending Backport to Resolved

Also available in: Atom PDF