Uploaded image for project: 'Lustre'
  1. Lustre
  2. LU-16427

'lfs rmfid' does not print anything on error

Details

    • Bug
    • Resolution: Fixed
    • Minor
    • Lustre 2.16.0
    • None
    • 3
    • 9223372036854775807

    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).

      Attachments

        Issue Links

          Activity

            [LU-16427] 'lfs rmfid' does not print anything on error
            pjones Peter Jones added a comment -

            Landed for 2.16

            pjones Peter Jones added a comment - Landed for 2.16

            "Oleg Drokin <green@whamcloud.com>" 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

            gerrit Gerrit Updater added a comment - "Oleg Drokin <green@whamcloud.com>" 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

            @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.

            arshad512 Arshad Hussain added a comment - @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.

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

            bertschinger Thomas Bertschinger added a comment - @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 <arshad.hussain@aeoncomputing.com>" 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

            gerrit Gerrit Updater added a comment - "Arshad Hussain <arshad.hussain@aeoncomputing.com>" 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
            arshad512 Arshad Hussain added a comment - - edited

            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}

            arshad512 Arshad Hussain added a comment - - edited 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}

            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?

            bertschinger Thomas Bertschinger added a comment - 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 -EBAD F), 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?

            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?

             

            bertschinger Thomas Bertschinger added a comment - 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?  

            People

              arshad512 Arshad Hussain
              adilger Andreas Dilger
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: