[LU-4023] wrong type used for ldlm flag

• Type: Bug
• Resolution: Fixed
• Priority: Blocker
• Fix Version/s: Lustre 2.5.0, Lustre 2.6.0, Lustre 2.4.2
• Affects Version/s: Lustre 2.4.0, Lustre 2.5.0
• Environment: clang 3.4
• Severity: 3
• Assignee: Dmitry Eremin (Inactive)
• Reporter: Alexey Lyashkov

Description

While porting lustre to macos i found a some strange behavior - likely wrong type used for variable.

osc_request.c:2503:41: error: implicit conversion from 'unsigned long long' to 'int' changes value from 2199023255552 to 0 [-Werror,-Wconstant-conversion]
        int match_lvb = (agl != 0 ? 0 : LDLM_FL_LVB_READY);
            ~~~~~~~~~                   ^~~~~~~~~~~~~~~~~
/Users/shadow/work/lustre/work/WC-review/mac-os/lustre/include/lustre_dlm_flags.h:244:41: note: expanded from macro 'LDLM_FL_LVB_READY'
#define LDLM_FL_LVB_READY               0x0000020000000000ULL // bit  41

so if we assign 64bit data to 32bit variable - we will be have zero always and that logic newer work.

Comments

[Alexey Lyashkov]

stop talking about blockers. we need proper patch at first place and don't need rewrite a single fix for different branches it's add many problems with supporting that code.

current patch don't fix all similar issues in future and fix just one instance. if we have lost one point in code review where 32bit -> 64bit conversion is lost - you really think we don't have similar bugs in different places? because you review a code? but as you see review isn't guaranteed to kill all bugs.

changes define to enum and use correct type for a ldlm flags avoid bugs at all, because introduce are compile time checking and allocating a property type for all variables and that is provide better code and more optimization for compiler if need.

[Dmitry Eremin (Inactive)]

It's blocker and required immediate action for 2.5 release. So, the current patch fix the issue and then we can elaborate other solution.

[Alexey Lyashkov]

I isn't really understand - it's new feature to ignore a requests one of reviewers and push patch anyway?

i put comment in gerrit before new patch test start

Alexey Lyashkov		Oct 3 2:15 PM
Patch Set 4:
may we converted #define to enum to avoid such errors in feature ?

but none response - and patch landed to master without all reviewers confirmed - that patch is OK.

PS. sorry for english - autocompletion was rewrite a future to feature.

[Dmitry Eremin (Inactive)]

Patch was landed to master. New patch is for b2_4 http://review.whamcloud.com/#/c/7849/

[Andreas Dilger]

This code has been broken since Vitaly landed the patch b58a810 in 2.3.54. It isn't clear why we didn't notice this failure, but it seems that the "obvious" fix is introducing some kind of breakage and we need a closer look at what is going wrong.

[Dmitry Eremin (Inactive)]

patch is http://review.whamcloud.com/#/c/7799/