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

lustre: Correctly identify the IOCTL in case of failure. #139

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

arshad512
Copy link

In case of failure returned by IOCTL, under lustre_mds_stat_by_fid() and lustre_mds_stat() the default case was printing the new IOCTL string regardless. This would be incorrect in case OLD IOCTL is called. This patch fixes the above issue, buy rechecking
IOCTL in current use, assigning the correct string then calling DisplayLog() to print the string.

Change-Id: I3d493b75f599aae7b7ba2815451a1ae8f534a282

In case of failure returned by IOCTL, under
lustre_mds_stat_by_fid() and lustre_mds_stat() the
default case was printing the new IOCTL string regardless.
This would be incorrect in case OLD IOCTL is called.
This patch fixes the above issue, buy rechecking
IOCTL in current use, assigning the correct string
then calling DisplayLog() to print the string.

Change-Id: I3d493b75f599aae7b7ba2815451a1ae8f534a282
Signed-off-by: Arshad Hussain <[email protected]>
@tl-cea
Copy link
Member

tl-cea commented Aug 30, 2024

Thank your for the contribution.

Given that IOC_MDC_GETFILEINFO refers to different ioctls depending on the Lustre version, and IOC_MDC_GETFILEINFO_OLD may or may not exist, or may also change in the future,
what about just tracing the ioctl name as "IOC_MDC_GETFILEINFO_V1" which precisely refer to what is called and won't change? Would it be satisfying for you?

@arshad512
Copy link
Author

Thanks for the review!

Given that IOC_MDC_GETFILEINFO refers to different ioctls depending on the Lustre version, and

Ack

IOC_MDC_GETFILEINFO_OLD may or may not exist, or may also change in the future,

Ack. I think this is already being removed in Lustre master.

what about just tracing the ioctl name as "IOC_MDC_GETFILEINFO_V1" which precisely refer to what is called

Ack

and won't change?

I would not know. At least not in very near future :-)

Would it be satisfying for you?

Ack. I agree with you.

@tl-cea
Copy link
Member

tl-cea commented Sep 26, 2024

Modified version pushed here:
https://review.gerrithub.io/c/cea-hpc/robinhood/+/1201865

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants