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?

            People

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

              Dates

                Created:
                Updated:
                Resolved: