[LU-16427] 'lfs rmfid' does not print anything on error

• Status: Resolved
• Project: Lustre
• Fix Version/s: Lustre 2.16.0
• Type: Bug
• Priority: Minor
• Reporter: Andreas Dilger
• Assignee: Arshad Hussain
• Resolution: Fixed
• Labels: easy
• Severity: 3

Description

The "lfs rmfid" does not print anything useful or exit with a non-zero error code when passed bad arguments:

# lfs rmfid --help
# lfs rmfid foo
# echo $?
0

It would make sense to handle "--help" to print the usage message, and should print an error and the usage message if a bad filesystem name/mountpoint argument is given or if a bad FID is passed (though it should continue to process all remaining FIDs on the command-line and exit with an error at the end).

Issue Links

• is related to: LU-16618 rmfid miscellaneous fixes
• is related to: LU-14469 lmv_rmfid() does 128K kmalloc()

Comments

[Thomas Bertschinger]

I have been looking into this and have found that `rmfid` also appears completely nonfunctional when used within the "lfs shell". Here's what I mean:

$ lfs
lfs > rmfid /mnt/mylustre [0x200000407:0xcc:0x0]
unrecognized FID: /mnt/mylustre 
lfs > rmfid [0x200000407:0xcc:0x0]
lfs rmfid: cannot remove FIDs: Bad file descriptor

I'm guessing that lfs is not commonly used in this way, but is this shell considered supported and should be working? Should this be fixed as part of this bug?

 

[Thomas Bertschinger]

One challenge with printing out good error messages here is that I don't think the current API of llapi_rmfid() provides a good way to distinguish an error in opening the filesystem directory vs. an error with the ioctl().

A nonexistent filesystem path should result in llapi_rmfid() returning -ENOENT, I think (although currently it returns -EBADF), and while the ioctl() won't return -ENOENT from what I can tell, I don't know that it will never return -ENOENT.

One solution would be to create a function llapi_rmfid_at() that takes an FD open to the lustre mountpoint as the 1st argument so that the caller can validate the mountpoint, similar to lfs_fid2path() and llapi_fid2path_at(). (Or just modify llapi_rmfid(), although this would be a breaking API change.) This change would allow for clearer error messages from lfs rmfid, IMO. Do you have any thoughts on this idea?

[Arshad Hussain]

Andreas/Thomas,

I am already working on rmfid issues. (See below) you want me to pick up this LU?

Thanks

https://review.whamcloud.com/c/fs/lustre-release/+/50367

 

LU-16618 lfs: rmfid miscellaneous fixes

This patch fixes:

01. Fix rmfid silently accepting fid without fsname
or lustre root mount point. Make it correctly
fail if required arguments is not provided.

After Patch:
~~~~~~~~~~~~
$ lfs rmfid 0x200000402:0x1:0x0
lfs rmfid: missing <fsname|rootpath> or <fid>
Remove file(s) by FID(s)
usage: rmfid <fsname|rootpath> <fid> ...

Before Patch:
~~~~~~~~~~~~
$ lfs rmfid 0x200000402:0x1:0x0
${noformat}

[Gerrit Updater]

"Arshad Hussain <[e-mail]>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/50388
Subject: LU-16427 lfs: rmfid does not print anything on error
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 83add47417cfca6600336ae7d890125a513cee94

[Thomas Bertschinger]

@Arshad sorry, I did not realize you were already working on this one! I'll be happy to review your patch if desired.

[Arshad Hussain]

@Thomas, no worries and thanks for your review. Even I realized much later that this ticke was open much earlier by Andreas. It was missed by me. I started working on rmfid when I accidentally stumbled upon its error.

[Gerrit Updater]

"Oleg Drokin <[e-mail]>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/50388/
Subject: LU-16427 lfs: rmfid does not print anything on error
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 5d930252407cc11604c4b9de2984784c62c43a4c

[Peter Jones]

Landed for 2.16