[LU-18108] mdt_rec_reint template is not consistent

• Type: Bug
• Resolution: Fixed
• Priority: Minor
• Fix Version/s: Lustre 2.16.0
• Affects Version/s: Lustre 2.16.0
• Labels: easy medium
• Severity: 3
• Assignee: Andreas Dilger
• Reporter: Andreas Dilger

Description

The struct mdt_rec_reint should be the template for all struct mdt_rec_* instances. However, the introduction of the "__u16 rr_mirror_id" field added to the end of only mdt_rec_resync and mdt_rec_reint breaks this pattern, and risks future data corruption in mixed-endian environments if the __u32 padding field at the end of other mdt_rec_* fields is used as a __u32 instead, because there is only a single swabbing function for all of these records.

There are two possible solutions to this issue:

• change the last __u32 padding field of all other struct mdt_rec_* to a pai of __u16 padding fields. This is technically less complex to implement. However, this prevents their use as a __u32 in the future.
• move the mirror_id usage to a different __u32 field in struct mdt_rec_reint along with a new required OBD_CONNECT_MIRROR_ID_FIX interoperability check. The struct mdt_rec_reint has lots of other spare fields and using a __u32 to store a 16-bit value is fine. This is more complex to implement and will require some time for transition (the field was originally added in commit v2_11_56_0-70-g14171e787dd0, so would need interoperability for some time. However, this field is only used in one place, so the compat support is isolated and would liberate this field for use by the nanosecond timestamp code in LU-1158, or some other future usage (which is important since it is te last common reserved field in all structs).

Issue Links

• is related to: LU-1158 nanosecond timestamp support for Lustre

Comments

[Peter Jones]

Merged for 2.16

[Gerrit Updater]

"Oleg Drokin <[e-mail]>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/55988/
Subject: LU-18108 idl: deprecate rr_mirror_id usage
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: e56829645671de4d7fc628e075cdade840e89f80

[Gerrit Updater]

"Andreas Dilger <[e-mail]>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/55988
Subject: LU-18108 idl: deprecate rr_mirror_id usage
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 1c22666fc70e2a7d5c6eb3f320ec85ed640401c4

[Andreas Dilger]

It would still be useful to implement this, because the current code risks request corruption if the __u32 field overlapping rr_mirror_id is ever used, and at some point we will want to use another __u32 field in mdt_rec_reint and we will wish that this had been implemented sooner.

[Feng Lei]

This issue is not urgent today because nanosecond timestamp code replaces rr_a/m/ctime with rr_a/m/ctime_ns and no additional space is needed in mdt_rec_reint.