From ac31454ecec5fdd6a6bc66c713d62ce305140ef8 Mon Sep 17 00:00:00 2001 From: Edgar Gabriel Date: Wed, 13 Dec 2023 18:30:26 +0000 Subject: [PATCH 1/2] fs/lustre: recognize soft-links on file open the llapi_file_get_stripe() function does not accept as an input file a soft-link. Check therefore whether the filename provided in File_open is a soft-link, retrieve the real file name in case it is, make sure it is also on a Lustre file system, and use the real filename in the llapi_file_get_stripe() function call instead. Fixes issue #12141 Signed-off-by: Edgar Gabriel (cherry picked from commit c6a6c256e3687459a3c58bf31bff413a421b58a5) --- ompi/mca/fs/base/base.h | 52 +++++++++++++++++++++++ ompi/mca/fs/base/fs_base_get_parent_dir.c | 41 +++--------------- ompi/mca/fs/lustre/fs_lustre_file_open.c | 18 ++++++-- 3 files changed, 73 insertions(+), 38 deletions(-) diff --git a/ompi/mca/fs/base/base.h b/ompi/mca/fs/base/base.h index de57221cf83..80b9ca2b4ac 100644 --- a/ompi/mca/fs/base/base.h +++ b/ompi/mca/fs/base/base.h @@ -37,6 +37,22 @@ #include "ompi/mca/fs/fs.h" +#ifdef HAVE_SYS_STATFS_H +#include /* or */ +#endif +#ifdef HAVE_SYS_PARAM_H +#include +#endif +#ifdef HAVE_SYS_MOUNT_H +#include +#endif +#ifdef HAVE_SYS_STAT_H +#include +#endif +#ifdef HAVE_UNISTD_H +#include +#endif + BEGIN_C_DECLS OMPI_DECLSPEC int mca_fs_base_file_select(struct ompio_file_t *file, @@ -62,6 +78,42 @@ OMPI_DECLSPEC int mca_fs_base_file_get_size (ompio_file_t *fh, OMPI_MPI_OFFSET_T OMPI_DECLSPEC int mca_fs_base_file_set_size (ompio_file_t *fh, OMPI_MPI_OFFSET_TYPE size); OMPI_DECLSPEC int mca_fs_base_file_close (ompio_file_t *fh); + +static inline bool mca_fs_base_is_link (const char *filename) +{ + int err; + bool ret = true; + struct stat statbuf; + + err = lstat(filename, &statbuf); + + if (err || (!S_ISLNK(statbuf.st_mode))) { + ret = false; + } + + return ret; +} + +static inline void mca_fs_base_get_real_filename (const char *filename, char **rfilename) +{ + int namelen; + char linkbuf[PATH_MAX+1]; + + namelen = readlink(filename, linkbuf, PATH_MAX); + if (namelen == -1) { + /* something strange has happened between the time that + * we determined that this was a link and the time that + * we attempted to read it; punt and use the old name. + */ + *rfilename = strdup(filename); + } + else { + /* successfully read the link */ + linkbuf[namelen] = '\0'; /* readlink doesn't null terminate */ + *rfilename = strdup(linkbuf); + } +} + /* * Globals */ diff --git a/ompi/mca/fs/base/fs_base_get_parent_dir.c b/ompi/mca/fs/base/fs_base_get_parent_dir.c index 78c23e7a7f9..f12f5d5d6c0 100644 --- a/ompi/mca/fs/base/fs_base_get_parent_dir.c +++ b/ompi/mca/fs/base/fs_base_get_parent_dir.c @@ -31,31 +31,17 @@ #include "ompi/mca/fs/base/base.h" #include "ompi/mca/common/ompio/common_ompio.h" -#ifdef HAVE_SYS_STATFS_H -#include /* or */ -#endif -#ifdef HAVE_SYS_PARAM_H -#include -#endif -#ifdef HAVE_SYS_MOUNT_H -#include -#endif -#ifdef HAVE_SYS_STAT_H -#include -#endif -#ifdef HAVE_UNISTD_H -#include -#endif void mca_fs_base_get_parent_dir ( char *filename, char **dirnamep) { - int err; char *dir = NULL, *slash; - struct stat statbuf; - err = lstat(filename, &statbuf); + if (strlen(filename) < 1) { + asprintf(dirnamep, ".%s", OPAL_PATH_SEP); + return; + } - if (err || (!S_ISLNK(statbuf.st_mode))) { + if (!mca_fs_base_is_link(filename)) { /* no such file, or file is not a link; these are the "normal" * cases where we can just return the parent directory. */ @@ -67,22 +53,7 @@ void mca_fs_base_get_parent_dir ( char *filename, char **dirnamep) * but this code doesn't care if the target is really there * or not. */ - int namelen; - char linkbuf[PATH_MAX+1]; - - namelen = readlink(filename, linkbuf, PATH_MAX); - if (namelen == -1) { - /* something strange has happened between the time that - * we determined that this was a link and the time that - * we attempted to read it; punt and use the old name. - */ - dir = strdup(filename); - } - else { - /* successfully read the link */ - linkbuf[namelen] = '\0'; /* readlink doesn't null terminate */ - dir = strdup(linkbuf); - } + mca_fs_base_get_real_filename(filename, &dir); } slash = strrchr(dir, '/'); diff --git a/ompi/mca/fs/lustre/fs_lustre_file_open.c b/ompi/mca/fs/lustre/fs_lustre_file_open.c index cdc7199f900..58c06056bf2 100644 --- a/ompi/mca/fs/lustre/fs_lustre_file_open.c +++ b/ompi/mca/fs/lustre/fs_lustre_file_open.c @@ -70,7 +70,7 @@ mca_fs_lustre_file_open (struct ompi_communicator_t *comm, int fs_lustre_stripe_size = -1; int fs_lustre_stripe_width = -1; char char_stripe[MPI_MAX_INFO_KEY]; - + char *rfilename = (char *)filename; struct lov_user_md *lump=NULL; perm = mca_fs_base_get_file_perm(fh); @@ -108,6 +108,16 @@ mca_fs_lustre_file_open (struct ompi_communicator_t *comm, fs_lustre_stripe_width = mca_fs_lustre_stripe_width; } + /* Check for soft links and replace filename by the actual + file used in case it is a soft link */ + if (mca_fs_base_is_link(filename)) { + mca_fs_base_get_real_filename(filename, &rfilename); + /* make sure the real file is also on a Lustre file system */ + if (LUSTRE != mca_fs_base_get_fstype(rfilename)) { + opal_output(1, "cannot use a soft-link between a LUSTRE and non-LUSTRE file system\n"); + return OPAL_ERROR; + } + } /* Reset errno */ errno = 0; @@ -115,6 +125,8 @@ mca_fs_lustre_file_open (struct ompi_communicator_t *comm, if ( (fs_lustre_stripe_size>0 || fs_lustre_stripe_width>0) && ( amode&O_CREAT) && ( (amode&O_RDWR)|| amode&O_WRONLY) ) { + /* this cannot be a soft-link since we are creating the file. + Not using rfilename here */ llapi_file_create(filename, fs_lustre_stripe_size, -1, /* MSC need to change that */ @@ -132,7 +144,7 @@ mca_fs_lustre_file_open (struct ompi_communicator_t *comm, } } - comm->c_coll->coll_bcast ( &ret, 1, MPI_INT, 0, comm, comm->c_coll->coll_bcast_module); + comm->c_coll->coll_bcast ( &ret, 1, MPI_INT, 0, comm, comm->c_coll->coll_bcast_module); if ( OMPI_SUCCESS != ret ) { fh->fd = -1; return ret; @@ -150,7 +162,7 @@ mca_fs_lustre_file_open (struct ompi_communicator_t *comm, fprintf(stderr,"Cannot allocate memory for extracting stripe size\n"); return OMPI_ERROR; } - rc = llapi_file_get_stripe(filename, lump); + rc = llapi_file_get_stripe(rfilename, lump); if (rc != 0) { opal_output(1, "get_stripe failed: %d (%s)\n", errno, strerror(errno)); free(lump); From 9d54ef2d14067aba5ede39dfac28eaffeb2f6092 Mon Sep 17 00:00:00 2001 From: Howard Pritchard Date: Thu, 21 Dec 2023 08:00:40 -0800 Subject: [PATCH 2/2] move include of sys/mount back to where it was Turns out the sys/mount.h can be tricky to use because various other system include files sometimes redefine some of the symbols in this include file, leading to compile failures. Careful ordering of include files may solve the problem, but here it simpler just to move the include of sys/mount.h back to its original location. related to #12181 Signed-off-by: Howard Pritchard (cherry picked from commit d2af1d7d52361d8b53cfc1177451966106d0ab8e) --- ompi/mca/fs/base/base.h | 3 --- ompi/mca/fs/base/fs_base_get_parent_dir.c | 8 ++++++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/ompi/mca/fs/base/base.h b/ompi/mca/fs/base/base.h index 80b9ca2b4ac..d19c61cd7e6 100644 --- a/ompi/mca/fs/base/base.h +++ b/ompi/mca/fs/base/base.h @@ -43,9 +43,6 @@ #ifdef HAVE_SYS_PARAM_H #include #endif -#ifdef HAVE_SYS_MOUNT_H -#include -#endif #ifdef HAVE_SYS_STAT_H #include #endif diff --git a/ompi/mca/fs/base/fs_base_get_parent_dir.c b/ompi/mca/fs/base/fs_base_get_parent_dir.c index f12f5d5d6c0..76ba7a1f1ae 100644 --- a/ompi/mca/fs/base/fs_base_get_parent_dir.c +++ b/ompi/mca/fs/base/fs_base_get_parent_dir.c @@ -31,6 +31,14 @@ #include "ompi/mca/fs/base/base.h" #include "ompi/mca/common/ompio/common_ompio.h" +/* + * Be careful moving this include. + * It's easy to hit problems similar to that reported in + * https://github.com/systemd/systemd/issues/8507 + */ +#ifdef HAVE_SYS_MOUNT_H +#include +#endif void mca_fs_base_get_parent_dir ( char *filename, char **dirnamep) {