From 5d930252407cc11604c4b9de2984784c62c43a4c Mon Sep 17 00:00:00 2001 From: Arshad Hussain Date: Tue, 4 Apr 2023 03:18:15 +0530 Subject: [PATCH] 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 or Remove file(s) by FID(s) usage: rmfid ... 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 Change-Id: Idda9313c97e48e9f7bf6486894b6ae3c74d71981 Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/50388 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Andreas Dilger Reviewed-by: Thomas Bertschinger Reviewed-by: Oleg Drokin --- lustre/doc/Makefile.am | 1 + lustre/doc/llapi_rmfid.3 | 12 +++++++ lustre/doc/llapi_rmfid_at.3 | 1 + lustre/doc/llapi_root_path_open.3 | 34 +++++++++++++++++++ lustre/doc/lustreapi.7 | 3 ++ lustre/include/lustre/lustreapi.h | 2 ++ lustre/utils/lfs.c | 55 +++++++++++++++++++++++------- lustre/utils/liblustreapi_util.c | 71 ++++++++++++++++++++++++++++++--------- 8 files changed, 151 insertions(+), 28 deletions(-) create mode 100644 lustre/doc/llapi_rmfid_at.3 create mode 100644 lustre/doc/llapi_root_path_open.3 diff --git a/lustre/doc/Makefile.am b/lustre/doc/Makefile.am index 55cb1b9..fd35718 100644 --- 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 --- 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 .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 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 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 + +.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 --- 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 --- 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 --- 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 --- 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; } -- 1.8.3.1