search

cea-hpc / robinhood

Robinhood Policy Engine : a versatile tool to monitor filesystem contents and schedule actions on filesystem entries.
http://robinhood.sf.net
Other
182 stars 62 forks source link

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

Closed arshad512 closed 1 week ago

arshad512 commented 1 year ago

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

tl-cea commented 3 months ago

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 commented 3 months ago

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 commented 2 months ago

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

tl-cea commented 1 week ago

Thank you for the review. Patch landed.