LU-16427 lfs: rmfid does not print anything on error

This patch:

01. Improve rmfid

    Adds llapi_root_path_open() This function accepts the device
    or path and returns the open fd for them. This was done so
    that it is called only _once_ and not at every lookup.

    Adds llapi_rmfid_at() This function makes the final IOCTL to
    rmfid. Since llapi_root_path_open() we already had the valid
    fd and populated 'fid_structure'. We could isolate this and
    not call the former llapi_rmfid()

02. 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
    $

03. Fix rmfid memory leak

    After Patch:
    ~~~~~~~~~~~~
    $ valgrind --leak-check=full lfs  rmfid lustre 0x200000402:0x1:0x0
    ==33793== HEAP SUMMARY:
    ==33793==     in use at exit: 0 bytes in 0 blocks
    ==33793==   total heap usage: 4 allocs, 4 frees, 1,567 bytes allocated
    ==33793==
    ==33793== All heap blocks were freed -- no leaks are possible

    Before Patch:
    ~~~~~~~~~~~~
    $ valgrind --leak-check=full lfs rmfid lustre 0x200000402:0x1:0x0
    ==30812== LEAK SUMMARY:
    ==30812==    definitely lost: 48 bytes in 1 blocks
    ==30812==    indirectly lost: 0 bytes in 0 blocks
    ==30812==      possibly lost: 0 bytes in 0 blocks
    ==30812==    still reachable: 0 bytes in 0 blocks
    ==30812==         suppressed: 0 bytes in 0 blocks

04. Update/Add Man pages

    Update llapi_rmfid.3 and lustreapi.7 man pages
    Add new llapi_rmfid_at.3 and llapi_root_path_open.3 man pages

Test-Parameters: trivial testlist=sanity
Signed-off-by: Arshad Hussain <arshad.hussain@aeoncomputing.com>
Change-Id: Idda9313c97e48e9f7bf6486894b6ae3c74d71981
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/50388
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Thomas Bertschinger <bertschinger@lanl.gov>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/doc/Makefile.am b/lustre/doc/Makefile.am
index 55cb1b9..fd35718 100644 (file)
--- a/lustre/doc/Makefile.am
+++ b/lustre/doc/Makefile.am
@@ -198,6 +198,7 @@ LIBMAN =                                    \
        llapi_pcc_state_get_fd.3                \
        llapi_quotactl.3                        \
        llapi_rmfid.3                           \
+       llapi_rmfid_at.3                        \
        llapi_search_mdt.3                      \
        llapi_search_ost.3                      \
        llapi_search_tgt.3                      \

diff --git a/lustre/doc/llapi_rmfid.3 b/lustre/doc/llapi_rmfid.3
index d49c9f4..d7c8db0 100644 (file)
--- a/lustre/doc/llapi_rmfid.3
+++ b/lustre/doc/llapi_rmfid.3
@@ -6,6 +6,7 @@ llapi_rmfid \- Remove files by their FIDs in Lustre.
 .B #include <lustre/lustreapi.h>
 .PP
 .BI "int llapi_rmfid(const char *" path ", struct fid_array *" fa ");
+.BI "int llapi_rmfid_at(int " fd ", struct fid_array *" fa ");
 
 .sp
 .fi
@@ -22,9 +23,19 @@ only for root or regular users on filesystems mounted with
 mount option to delete files that they own and are in a directory in which
 they have write permission.
 
+.BR llapi_rmfid_at()
+Is similar to
+.I llapi_rmfid.
+It tries to remove Lustre files by FIDs stored in
+.I fa->fa_fids
+where path or device is pointed by an already verifed
+.I fd.
+
 .SH RETURN VALUES
 .LP
 .B llapi_rmfid()
+and
+.B llapi_rmfid_at()
 return 0 on success or a negative errno value on failure. Result for each file
 is stored in the corresponding
 .I fa->fa_fid[N].f_ver
@@ -48,3 +59,4 @@ Invalid FID is passed
 Not enough memory to process the request
 .SH "SEE ALSO"
 .BR lustreapi (7)
+.BR llapi_rmfid_at (3)

diff --git a/lustre/doc/llapi_rmfid_at.3 b/lustre/doc/llapi_rmfid_at.3
new file mode 100644 (file)
index 0000000..4edcda9
--- /dev/null
+++ b/lustre/doc/llapi_rmfid_at.3
@@ -0,0 +1 @@
+.so man3/llapi_rmfid.3

diff --git a/lustre/doc/llapi_root_path_open.3 b/lustre/doc/llapi_root_path_open.3
new file mode 100644 (file)
index 0000000..1ccc973
--- /dev/null
+++ b/lustre/doc/llapi_root_path_open.3
@@ -0,0 +1,34 @@
+.TH llapi_root_path_open 3 "2023 Apr 21" "Lustre User API"
+.SH NAME
+llapi_root_path_open \- Return open fd for a given device/path provided
+.SH SYNOPSIS
+.nf
+.B #include <lustre/lustreapi.h>
+
+.sp
+.BI "int llapi_root_path_open(char *"device ", int *" fd " );
+
+.sp
+.fi
+.SH DESCRIPTION
+.LP
+.B llapi_root_path_open(\|)
+is called with
+.I device
+which points to the FSNAME or complet path. On success
+.I fd
+is populated with valid descriptor.
+
+.SH RETURN VALUES
+.LP
+.B llapi_root_path_open(\|)
+return:
+.TP
+>=0
+on success.
+.TP
+<0
+on failure.
+.fi
+.SH "SEE ALSO"
+.BR lustreapi (7)

diff --git a/lustre/doc/lustreapi.7 b/lustre/doc/lustreapi.7
index 5cbae56..d855537 100644 (file)
--- a/lustre/doc/lustreapi.7
+++ b/lustre/doc/lustreapi.7
@@ -75,6 +75,9 @@ quotas, file layouts, etc).  See the referenced man pages for details.
 .BR llapi_path2fid (3),
 .BR llapi_path2parent (3),
 .BR llapi_quotactl (3),
+.BR llapi_rmfid (3),
+.BR llapi_rmfid_at (3),
+.BR llapi_root_path_open (3),
 .BR llapi_search_tgt (3),
 .BR llapi_search_rootpath (3),
 .BR llapi_search_rootpath_by_dev (3),

diff --git a/lustre/include/lustre/lustreapi.h b/lustre/include/lustre/lustreapi.h
index 54481c5..e257eb4 100644 (file)
--- a/lustre/include/lustre/lustreapi.h
+++ b/lustre/include/lustre/lustreapi.h
@@ -515,6 +515,8 @@ int llapi_path2parent(const char *path, unsigned int linkno,
 int llapi_fd2parent(int fd, unsigned int linkno, struct lu_fid *parent_fid,
                    char *name, size_t name_size);
 int llapi_rmfid(const char *path, struct fid_array *fa);
+int llapi_rmfid_at(int fd, struct fid_array *fa);
+int llapi_root_path_open(char *device, int *outfd);
 int llapi_chomp_string(char *buf);
 
 struct file_handle;

diff --git a/lustre/utils/lfs.c b/lustre/utils/lfs.c
index 4521b77..2888070 100644 (file)
--- a/lustre/utils/lfs.c
+++ b/lustre/utils/lfs.c
@@ -9866,11 +9866,11 @@ static int lfs_path2fid(int argc, char **argv)
 #define MAX_ERRNO      4095
 #define IS_ERR_VALUE(x) ((unsigned long)(x) >= (unsigned long)-MAX_ERRNO)
 
-static int lfs_rmfid_and_show_errors(const char *device, struct fid_array *fa)
+static int lfs_rmfid_and_show_errors(int rootfd, struct fid_array *fa)
 {
        int rc, rc2, k;
 
-       rc = llapi_rmfid(device, fa);
+       rc = llapi_rmfid_at(rootfd, fa);
        if (rc < 0) {
                fprintf(stderr, "%s rmfid: cannot remove FIDs: %s\n",
                        progname, strerror(-rc));
@@ -9896,50 +9896,81 @@ static int lfs_rmfid_and_show_errors(const char *device, struct fid_array *fa)
 static int lfs_rmfid(int argc, char **argv)
 {
        char *fidstr, *device;
-       int rc = 0, rc2, nr;
+       int rc = 0, rc2, rc3 = 0, nr;
        struct fid_array *fa;
+       int rootfd;
+
+       /* Interactive mode: Adjust optind */
+       if (!optind)
+               optind++;
+
+       device = argv[optind++];
 
        if (optind > argc - 1) {
                fprintf(stderr, "%s rmfid: missing dirname\n", progname);
                return CMD_HELP;
        }
 
-       device = argv[optind++];
-
        nr = argc - optind;
+
+       rc = llapi_root_path_open(device, &rootfd);
+       if (rc < 0) {
+               fprintf(stderr,
+                       "%s rmfid: error opening device/fsname '%s': %s\n",
+                       progname, device, strerror(-rc));
+               return -rc;
+       }
+
        fa = malloc(offsetof(struct fid_array, fa_fids[nr + 1]));
-       if (!fa)
+       if (!fa) {
+               fprintf(stderr, "%s rmfid: error allocating %zd bytes: %s\n",
+                       progname, offsetof(struct fid_array, fa_fids[nr + 1]),
+                       strerror(errno));
                return -ENOMEM;
+       }
 
        fa->fa_nr = 0;
        rc = 0;
        while (optind < argc) {
+               char *origfidstr;
                int found;
 
-               fidstr = argv[optind++];
+               origfidstr = fidstr = argv[optind++];
                while (*fidstr == '[')
                        fidstr++;
                found = sscanf(fidstr, SFID, RFID(&fa->fa_fids[fa->fa_nr]));
                if (found != 3) {
-                       fprintf(stderr, "unrecognized FID: %s\n",
-                               argv[optind - 1]);
-                       exit(1);
+                       fprintf(stderr, "lfs rmfid: '%s': Wrong FID format\n",
+                               origfidstr);
+                       if (!rc3)
+                               rc3 = -EINVAL; /* Invalid argument */
+                       continue;
                }
                fa->fa_nr++;
                if (fa->fa_nr == OBD_MAX_FIDS_IN_ARRAY) {
                        /* start another batch */
-                       rc2 = lfs_rmfid_and_show_errors(device, fa);
+                       rc2 = lfs_rmfid_and_show_errors(rootfd, fa);
                        if (rc2 && !rc)
                                rc = rc2;
+                       if (rc3)
+                               rc = rc3;
                        fa->fa_nr = 0;
                }
        }
        if (fa->fa_nr) {
-               rc2 = lfs_rmfid_and_show_errors(device, fa);
+               rc2 = lfs_rmfid_and_show_errors(rootfd, fa);
                if (rc2 && !rc)
                        rc = rc2;
+               if (rc3)
+                       rc = rc3;
+       }
+
+       if (fa) {
+               free(fa);
+               fa = NULL;
        }
 
+       close(rootfd);
        return rc;
 }
 

diff --git a/lustre/utils/liblustreapi_util.c b/lustre/utils/liblustreapi_util.c
index dc24fce..9818851 100644 (file)
--- a/lustre/utils/liblustreapi_util.c
+++ b/lustre/utils/liblustreapi_util.c
@@ -287,27 +287,66 @@ int llapi_search_ost(const char *fsname, const char *poolname,
        return llapi_search_tgt(fsname, poolname, ostname, false);
 }
 
+/**
+ * Return the open fd for a given device/path provided
+ *
+ * \param device[in]           buffer holding device or path string
+ * \param rootfd[out]          file descriptor after successful opening of
+ *                              of above path or device
+ *
+ * \retval                     0 on success
+ * \retval                     -ve on failure
+ */
+int llapi_root_path_open(char *device, int *rootfd)
+{
+       int tmp_fd, rc;
+
+       if (*device == '/')
+               rc = get_root_path(WANT_FD, NULL, &tmp_fd,
+                                  (char *)device, -1, NULL, NULL);
+       else
+               rc = get_root_path(WANT_FD, (char *)device, &tmp_fd,
+                                  NULL, -1, NULL, NULL);
+
+       if (!rc)
+               *rootfd = dup(tmp_fd);
+
+       return rc;
+}
+
+/**
+ * Call IOCTL to remove file by fid. The fd must be valid and fa
+ * (fid_array) struct must allready be populated.
+ *
+ * \param fd[in]               valid descriptor of device/path
+ * \param fa[in]               fid_array struct holding fids
+ *
+ * \retval                     0 on success
+ * \retval                     -ve/errno on failure
+ */
+int llapi_rmfid_at(int fd, struct fid_array *fa)
+{
+       return ioctl(fd, LL_IOC_RMFID, fa) ? -errno : 0;
+}
+
 int llapi_rmfid(const char *path, struct fid_array *fa)
 {
-       char rootpath[PATH_MAX];
-       int fd, rc;
+       int rootfd, rc;
 
-retry_open:
-       fd = open(path, O_RDONLY | O_NONBLOCK | O_NOFOLLOW);
-       if (fd < 0) {
-               if (errno == ENOENT && path != rootpath) {
-                       rc = llapi_search_rootpath(rootpath, path);
-                       if (!rc) {
-                               path = rootpath;
-                               goto retry_open;
-                       }
-               } else {
-                       return -errno;
-               }
+       rc = llapi_root_path_open((char *)path, &rootfd);
+       if (rc < 0) {
+               fprintf(stderr,
+                       "lfs rmfid: error opening device/fsname '%s': %s\n",
+                       path, strerror(-rc));
+               return -rc;
        }
 
-       rc = ioctl(fd, LL_IOC_RMFID, fa);
-       close(fd);
+       rc = llapi_rmfid_at(rootfd, fa);
+       if (rc < 0) {
+               fprintf(stderr, "lfs rmfid: cannot remove FIDs: %s\n",
+                       strerror(-rc));
+               return rc;
+       }
 
        return rc ? -errno : 0;
 }