LU-7057 utils: use stronger flags when opening volatile files

Existing files with volatile-ish names may be accidentally opened by
lfs_migrate(). This is not the intent so disallow this behavior by
adding O_EXCL and O_NOFOLLOW to the flags to open(). Use the original
file's MDT index and a random suffix when constructing the volatile
file name (as is done in llapi_create_volatile_idx()) and reduce the
mode of the file created.

To handle pre-existing files use a do while lookup around the volatile
opens in llapi_create_volatile_idx() and lfs_migrate().

Signed-off-by: John L. Hammond <john.hammond@intel.com>
Change-Id: I399d30104d841df13fe1051aaa8264514911714d
Reviewed-on: http://review.whamcloud.com/16166
Tested-by: Jenkins
Reviewed-by: Henri Doreau <henri.doreau@cea.fr>
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
Tested-by: Andreas Dilger <andreas.dilger@intel.com>

diff --git a/lustre/utils/lfs.c b/lustre/utils/lfs.c
index c58113d..c221e14 100644 (file)
--- a/lustre/utils/lfs.c
+++ b/lustre/utils/lfs.c
@@ -623,9 +623,13 @@ static int lfs_migrate(char *name, __u64 migration_flags,
 {
        int                      fd = -1;
        int                      fdv = -1;
-       char                     volatile_file[PATH_MAX +
-                                               LUSTRE_VOLATILE_HDR_LEN + 4];
        char                     parent[PATH_MAX];
+       int                      mdt_index;
+       int                      random_value;
+       char                     volatile_file[sizeof(parent) +
+                                              LUSTRE_VOLATILE_HDR_LEN +
+                                              2 * sizeof(mdt_index) +
+                                              2 * sizeof(random_value) + 4];
        char                    *ptr;
        int                      rc;
        struct lov_user_md      *lum = NULL;
@@ -699,18 +703,29 @@ static int lfs_migrate(char *name, __u64 migration_flags,
                        *ptr = '\0';
        }
 
-       rc = snprintf(volatile_file, sizeof(volatile_file), "%s/%s::", parent,
-                     LUSTRE_VOLATILE_HDR);
-       if (rc >= sizeof(volatile_file)) {
-               rc = -E2BIG;
+       rc = llapi_file_fget_mdtidx(fd, &mdt_index);
+       if (rc < 0) {
+               fprintf(stderr, "%s: %s: cannot get MDT index: %s\n",
+                       progname, name, strerror(-rc));
                goto error;
        }
 
-       /* create, open a volatile file, use caching (ie no directio) */
-       /* exclusive create is not needed because volatile files cannot
-        * conflict on name by construction */
-       fdv = llapi_file_open_param(volatile_file, O_CREAT | O_WRONLY, 0644,
-                                   param);
+       do {
+               random_value = random();
+               rc = snprintf(volatile_file, sizeof(volatile_file),
+                             "%s/%s:%.4X:%.4X", parent, LUSTRE_VOLATILE_HDR,
+                             mdt_index, random_value);
+               if (rc >= sizeof(volatile_file)) {
+                       rc = -E2BIG;
+                       goto error;
+               }
+
+               /* create, open a volatile file, use caching (ie no directio) */
+               fdv = llapi_file_open_param(volatile_file,
+                               O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW,
+                                           S_IRUSR | S_IWUSR, param);
+       } while (fdv == -EEXIST);
+
        if (fdv < 0) {
                rc = fdv;
                fprintf(stderr, "%s: %s: cannot create volatile file in"

diff --git a/lustre/utils/liblustreapi.c b/lustre/utils/liblustreapi.c
index 5cab621..ff8e58d 100644 (file)
--- a/lustre/utils/liblustreapi.c
+++ b/lustre/utils/liblustreapi.c
@@ -4628,26 +4628,30 @@ int llapi_create_volatile_idx(char *directory, int idx, int open_flags)
 {
        char    file_path[PATH_MAX];
        char    filename[PATH_MAX];
+       int     saved_errno = errno;
        int     fd;
        int     rnumber;
        int     rc;
 
-       rnumber = random();
-       if (idx == -1)
-               snprintf(filename, sizeof(filename),
-                        LUSTRE_VOLATILE_HDR"::%.4X", rnumber);
-       else
-               snprintf(filename, sizeof(filename),
-                        LUSTRE_VOLATILE_HDR":%.4X:%.4X", idx, rnumber);
+       do {
+               rnumber = random();
+               if (idx == -1)
+                       snprintf(filename, sizeof(filename),
+                                LUSTRE_VOLATILE_HDR"::%.4X", rnumber);
+               else
+                       snprintf(filename, sizeof(filename),
+                                LUSTRE_VOLATILE_HDR":%.4X:%.4X", idx, rnumber);
+
+               rc = snprintf(file_path, sizeof(file_path),
+                             "%s/%s", directory, filename);
+               if (rc >= sizeof(file_path))
+                       return -E2BIG;
 
-       rc = snprintf(file_path, sizeof(file_path),
-                     "%s/%s", directory, filename);
-       if (rc >= sizeof(file_path))
-               return -E2BIG;
+               fd = open(file_path,
+                         O_RDWR | O_CREAT | O_EXCL | O_NOFOLLOW | open_flags,
+                         S_IRUSR | S_IWUSR);
+       } while (fd < 0 && errno == EEXIST);
 
-       fd = open(file_path,
-                 O_RDWR | O_CREAT | O_EXCL | O_NOFOLLOW | open_flags,
-                 S_IRUSR | S_IWUSR);
        if (fd < 0) {
                llapi_error(LLAPI_MSG_ERROR, errno,
                            "Cannot create volatile file '%s' in '%s'",
@@ -4655,10 +4659,17 @@ int llapi_create_volatile_idx(char *directory, int idx, int open_flags)
                            directory);
                return -errno;
        }
-       /* unlink file in case this wasn't a Lustre filesystem, and the
-        * magic volatile filename wasn't handled as intended.  The effect
-        * is the same. */
-       unlink(file_path);
+
+       /* Unlink file in case this wasn't a Lustre filesystem and the
+        * magic volatile filename wasn't handled as intended. The
+        * effect is the same. If volatile open was supported then we
+        * expect unlink() to return -ENOENT. */
+       (void)unlink(file_path);
+
+       /* Since we are returning successfully we restore errno (and
+        * mask out possible EEXIST from open() and ENOENT from
+        * unlink(). */
+       errno = saved_errno;
 
        return fd;
 }