You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
313 lines
11 KiB
313 lines
11 KiB
3 years ago
|
From 2f07d12f902e371d8cb8c76007d558e3a727b56a Mon Sep 17 00:00:00 2001
|
||
|
From: Kotresh HR <khiremat@redhat.com>
|
||
|
Date: Tue, 9 Apr 2019 18:23:05 +0530
|
||
|
Subject: [PATCH 122/124] posix/ctime: Fix stat(time attributes) inconsistency
|
||
|
during readdirp
|
||
|
|
||
|
Problem:
|
||
|
Creation of tar file on gluster volume throws warning
|
||
|
'file changed as we read it'
|
||
|
|
||
|
Cause:
|
||
|
During readdirp, for few of the files whose inode is not
|
||
|
present, time attributes were served from backend. This caused
|
||
|
the ctime of few files to be different between before readdir
|
||
|
and after readdir by tar.
|
||
|
|
||
|
Solution:
|
||
|
If ctime feature is enabled and inode is not present, don't
|
||
|
serve the time attributes from backend file, serve it from xattr.
|
||
|
|
||
|
Backport of:
|
||
|
> Patch: https://review.gluster.org/22540
|
||
|
> fixes: bz#1698078
|
||
|
> Change-Id: I427ef865f97399475faf5aa6ca495f7e317603ae
|
||
|
> Signed-off-by: Kotresh HR <khiremat@redhat.com>
|
||
|
|
||
|
BUG: 1699709
|
||
|
Change-Id: I427ef865f97399475faf5aa6ca495f7e317603ae
|
||
|
Signed-off-by: Kotresh HR <khiremat@redhat.com>
|
||
|
Reviewed-on: https://code.engineering.redhat.com/gerrit/168687
|
||
|
Tested-by: RHGS Build Bot <nigelb@redhat.com>
|
||
|
Reviewed-by: Atin Mukherjee <amukherj@redhat.com>
|
||
|
---
|
||
|
tests/basic/ctime/ctime-readdir.c | 29 +++++++++++++++++
|
||
|
tests/basic/ctime/ctime-readdir.t | 50 ++++++++++++++++++++++++++++++
|
||
|
xlators/storage/posix/src/posix-helpers.c | 29 +++++++++++------
|
||
|
xlators/storage/posix/src/posix-metadata.c | 41 ++++++++++++++----------
|
||
|
4 files changed, 123 insertions(+), 26 deletions(-)
|
||
|
create mode 100644 tests/basic/ctime/ctime-readdir.c
|
||
|
create mode 100644 tests/basic/ctime/ctime-readdir.t
|
||
|
|
||
|
diff --git a/tests/basic/ctime/ctime-readdir.c b/tests/basic/ctime/ctime-readdir.c
|
||
|
new file mode 100644
|
||
|
index 0000000..8760db2
|
||
|
--- /dev/null
|
||
|
+++ b/tests/basic/ctime/ctime-readdir.c
|
||
|
@@ -0,0 +1,29 @@
|
||
|
+#include <stdio.h>
|
||
|
+#include <dirent.h>
|
||
|
+#include <string.h>
|
||
|
+#include <assert.h>
|
||
|
+
|
||
|
+int
|
||
|
+main(int argc, char **argv)
|
||
|
+{
|
||
|
+ DIR *dir = NULL;
|
||
|
+ struct dirent *entry = NULL;
|
||
|
+ int ret = 0;
|
||
|
+ char *path = NULL;
|
||
|
+
|
||
|
+ assert(argc == 2);
|
||
|
+ path = argv[1];
|
||
|
+
|
||
|
+ dir = opendir(path);
|
||
|
+ if (!dir) {
|
||
|
+ printf("opendir(%s) failed.\n", path);
|
||
|
+ return -1;
|
||
|
+ }
|
||
|
+
|
||
|
+ while ((entry = readdir(dir)) != NULL) {
|
||
|
+ }
|
||
|
+ if (dir)
|
||
|
+ closedir(dir);
|
||
|
+
|
||
|
+ return ret;
|
||
|
+}
|
||
|
diff --git a/tests/basic/ctime/ctime-readdir.t b/tests/basic/ctime/ctime-readdir.t
|
||
|
new file mode 100644
|
||
|
index 0000000..4564fc1
|
||
|
--- /dev/null
|
||
|
+++ b/tests/basic/ctime/ctime-readdir.t
|
||
|
@@ -0,0 +1,50 @@
|
||
|
+#!/bin/bash
|
||
|
+
|
||
|
+. $(dirname $0)/../../include.rc
|
||
|
+. $(dirname $0)/../../volume.rc
|
||
|
+
|
||
|
+cleanup;
|
||
|
+
|
||
|
+TEST glusterd
|
||
|
+
|
||
|
+TEST $CLI volume create $V0 replica 3 ${H0}:$B0/brick{1,2,3};
|
||
|
+TEST $CLI volume set $V0 performance.stat-prefetch on
|
||
|
+TEST $CLI volume set $V0 performance.readdir-ahead off
|
||
|
+TEST $CLI volume start $V0;
|
||
|
+
|
||
|
+TEST glusterfs --volfile-id=$V0 --volfile-server=$H0 --entry-timeout=0 $M0;
|
||
|
+
|
||
|
+TEST mkdir $M0/dir0
|
||
|
+TEST "echo hello_world > $M0/dir0/FILE"
|
||
|
+
|
||
|
+ctime1=$(stat -c %Z $M0/dir0/FILE)
|
||
|
+echo "Mount change time: $ctime1"
|
||
|
+
|
||
|
+sleep 2
|
||
|
+
|
||
|
+#Write to back end directly to modify ctime of backend file
|
||
|
+TEST "echo write_from_backend >> $B0/brick1/dir0/FILE"
|
||
|
+TEST "echo write_from_backend >> $B0/brick2/dir0/FILE"
|
||
|
+TEST "echo write_from_backend >> $B0/brick3/dir0/FILE"
|
||
|
+echo "Backend change time"
|
||
|
+echo "brick1: $(stat -c %Z $B0/brick1/dir0/FILE)"
|
||
|
+echo "brick2: $(stat -c %Z $B0/brick2/dir0/FILE)"
|
||
|
+echo "brick3: $(stat -c %Z $B0/brick3/dir0/FILE)"
|
||
|
+
|
||
|
+#Stop and start to hit the case of no inode for readdir
|
||
|
+TEST umount $M0
|
||
|
+TEST $CLI volume stop $V0
|
||
|
+TEST $CLI volume start $V0
|
||
|
+TEST glusterfs --volfile-id=$V0 --volfile-server=$H0 --entry-timeout=0 $M0;
|
||
|
+
|
||
|
+TEST build_tester $(dirname $0)/ctime-readdir.c
|
||
|
+
|
||
|
+#Do readdir
|
||
|
+TEST ./$(dirname $0)/ctime-readdir $M0/dir0
|
||
|
+
|
||
|
+EXPECT "$ctime1" stat -c %Z $M0/dir0/FILE
|
||
|
+echo "Mount change time after readdir $(stat -c %Z $M0/dir0/FILE)"
|
||
|
+
|
||
|
+cleanup_tester $(dirname $0)/ctime-readdir
|
||
|
+
|
||
|
+cleanup;
|
||
|
diff --git a/xlators/storage/posix/src/posix-helpers.c b/xlators/storage/posix/src/posix-helpers.c
|
||
|
index 193afc5..37e33a9 100644
|
||
|
--- a/xlators/storage/posix/src/posix-helpers.c
|
||
|
+++ b/xlators/storage/posix/src/posix-helpers.c
|
||
|
@@ -832,17 +832,26 @@ posix_pstat(xlator_t *this, inode_t *inode, uuid_t gfid, const char *path,
|
||
|
|
||
|
iatt_from_stat(&stbuf, &lstatbuf);
|
||
|
|
||
|
- if (inode && priv->ctime) {
|
||
|
- if (!inode_locked) {
|
||
|
- ret = posix_get_mdata_xattr(this, path, -1, inode, &stbuf);
|
||
|
+ if (priv->ctime) {
|
||
|
+ if (inode) {
|
||
|
+ if (!inode_locked) {
|
||
|
+ ret = posix_get_mdata_xattr(this, path, -1, inode, &stbuf);
|
||
|
+ } else {
|
||
|
+ ret = __posix_get_mdata_xattr(this, path, -1, inode, &stbuf);
|
||
|
+ }
|
||
|
+ if (ret) {
|
||
|
+ gf_msg(this->name, GF_LOG_WARNING, errno, P_MSG_GETMDATA_FAILED,
|
||
|
+ "posix get mdata failed on gfid: %s",
|
||
|
+ uuid_utoa(inode->gfid));
|
||
|
+ goto out;
|
||
|
+ }
|
||
|
} else {
|
||
|
- ret = __posix_get_mdata_xattr(this, path, -1, inode, &stbuf);
|
||
|
- }
|
||
|
- if (ret) {
|
||
|
- gf_msg(this->name, GF_LOG_WARNING, errno, P_MSG_GETMDATA_FAILED,
|
||
|
- "posix get mdata failed on gfid: %s",
|
||
|
- uuid_utoa(inode->gfid));
|
||
|
- goto out;
|
||
|
+ ret = __posix_get_mdata_xattr(this, path, -1, NULL, &stbuf);
|
||
|
+ if (ret) {
|
||
|
+ gf_msg(this->name, GF_LOG_WARNING, errno, P_MSG_GETMDATA_FAILED,
|
||
|
+ "posix get mdata failed on path: %s", path);
|
||
|
+ goto out;
|
||
|
+ }
|
||
|
}
|
||
|
}
|
||
|
|
||
|
diff --git a/xlators/storage/posix/src/posix-metadata.c b/xlators/storage/posix/src/posix-metadata.c
|
||
|
index 0ea9099..7ff5225 100644
|
||
|
--- a/xlators/storage/posix/src/posix-metadata.c
|
||
|
+++ b/xlators/storage/posix/src/posix-metadata.c
|
||
|
@@ -79,6 +79,7 @@ posix_fetch_mdata_xattr(xlator_t *this, const char *real_path_arg, int _fd,
|
||
|
fd_based_fop = _gf_true;
|
||
|
}
|
||
|
if (!(fd_based_fop || real_path_arg)) {
|
||
|
+ GF_VALIDATE_OR_GOTO(this->name, inode, out);
|
||
|
MAKE_HANDLE_PATH(real_path, this, inode->gfid, NULL);
|
||
|
if (!real_path) {
|
||
|
uuid_utoa_r(inode->gfid, gfid_str);
|
||
|
@@ -114,14 +115,14 @@ posix_fetch_mdata_xattr(xlator_t *this, const char *real_path_arg, int _fd,
|
||
|
key,
|
||
|
real_path ? real_path
|
||
|
: (real_path_arg ? real_path_arg : "null"),
|
||
|
- uuid_utoa(inode->gfid));
|
||
|
+ inode ? uuid_utoa(inode->gfid) : "null");
|
||
|
} else {
|
||
|
gf_msg(this->name, GF_LOG_DEBUG, *op_errno, P_MSG_XATTR_FAILED,
|
||
|
"getxattr failed"
|
||
|
" on %s gfid: %s key: %s ",
|
||
|
real_path ? real_path
|
||
|
: (real_path_arg ? real_path_arg : "null"),
|
||
|
- uuid_utoa(inode->gfid), key);
|
||
|
+ inode ? uuid_utoa(inode->gfid) : "null", key);
|
||
|
}
|
||
|
op_ret = -1;
|
||
|
goto out;
|
||
|
@@ -148,7 +149,7 @@ posix_fetch_mdata_xattr(xlator_t *this, const char *real_path_arg, int _fd,
|
||
|
"getxattr failed on "
|
||
|
" on %s gfid: %s key: %s ",
|
||
|
real_path ? real_path : (real_path_arg ? real_path_arg : "null"),
|
||
|
- uuid_utoa(inode->gfid), key);
|
||
|
+ inode ? uuid_utoa(inode->gfid) : "null", key);
|
||
|
goto out;
|
||
|
}
|
||
|
|
||
|
@@ -233,9 +234,14 @@ __posix_get_mdata_xattr(xlator_t *this, const char *real_path, int _fd,
|
||
|
int ret = -1;
|
||
|
int op_errno = 0;
|
||
|
|
||
|
- GF_VALIDATE_OR_GOTO(this->name, inode, out);
|
||
|
+ /* Handle readdirp: inode might be null, time attributes should be served
|
||
|
+ * from xattr not from backend's file attributes */
|
||
|
+ if (inode) {
|
||
|
+ ret = __inode_ctx_get1(inode, this, (uint64_t *)&mdata);
|
||
|
+ } else {
|
||
|
+ ret = -1;
|
||
|
+ }
|
||
|
|
||
|
- ret = __inode_ctx_get1(inode, this, (uint64_t *)&mdata);
|
||
|
if (ret == -1 || !mdata) {
|
||
|
mdata = GF_CALLOC(1, sizeof(posix_mdata_t), gf_posix_mt_mdata_attr);
|
||
|
if (!mdata) {
|
||
|
@@ -251,7 +257,9 @@ __posix_get_mdata_xattr(xlator_t *this, const char *real_path, int _fd,
|
||
|
* is hit when in-memory status is lost due to brick
|
||
|
* down scenario
|
||
|
*/
|
||
|
- __inode_ctx_set1(inode, this, (uint64_t *)&mdata);
|
||
|
+ if (inode) {
|
||
|
+ __inode_ctx_set1(inode, this, (uint64_t *)&mdata);
|
||
|
+ }
|
||
|
} else {
|
||
|
/* Failed to get mdata from disk, xattr missing.
|
||
|
* This happens on two cases.
|
||
|
@@ -278,7 +286,8 @@ __posix_get_mdata_xattr(xlator_t *this, const char *real_path, int _fd,
|
||
|
*/
|
||
|
gf_msg(this->name, GF_LOG_WARNING, op_errno,
|
||
|
P_MSG_FETCHMDATA_FAILED, "file: %s: gfid: %s key:%s ",
|
||
|
- real_path ? real_path : "null", uuid_utoa(inode->gfid),
|
||
|
+ real_path ? real_path : "null",
|
||
|
+ inode ? uuid_utoa(inode->gfid) : "null",
|
||
|
GF_XATTR_MDATA_KEY);
|
||
|
GF_FREE(mdata);
|
||
|
ret = 0;
|
||
|
@@ -297,6 +306,10 @@ __posix_get_mdata_xattr(xlator_t *this, const char *real_path, int _fd,
|
||
|
stbuf->ia_atime = mdata->atime.tv_sec;
|
||
|
stbuf->ia_atime_nsec = mdata->atime.tv_nsec;
|
||
|
}
|
||
|
+ /* Not set in inode context, hence free mdata */
|
||
|
+ if (!inode) {
|
||
|
+ GF_FREE(mdata);
|
||
|
+ }
|
||
|
|
||
|
out:
|
||
|
return ret;
|
||
|
@@ -416,6 +429,11 @@ posix_set_mdata_xattr(xlator_t *this, const char *real_path, int fd,
|
||
|
}
|
||
|
}
|
||
|
|
||
|
+ if ((flag->ctime == 0) && (flag->mtime == 0) && (flag->atime == 0)) {
|
||
|
+ ret = 0;
|
||
|
+ goto unlock;
|
||
|
+ }
|
||
|
+
|
||
|
/* Earlier, mdata was updated only if the existing time is less
|
||
|
* than the time to be updated. This would fail the scenarios
|
||
|
* where mtime can be set to any time using the syscall. Hence
|
||
|
@@ -486,7 +504,6 @@ out:
|
||
|
stbuf->ia_atime_nsec = mdata->atime.tv_nsec;
|
||
|
}
|
||
|
|
||
|
-
|
||
|
return ret;
|
||
|
}
|
||
|
|
||
|
@@ -604,10 +621,6 @@ posix_set_ctime(call_frame_t *frame, xlator_t *this, const char *real_path,
|
||
|
|
||
|
if (priv->ctime) {
|
||
|
(void)posix_get_mdata_flag(frame->root->flags, &flag);
|
||
|
- if ((flag.ctime == 0) && (flag.mtime == 0) && (flag.atime == 0)) {
|
||
|
- goto out;
|
||
|
- }
|
||
|
-
|
||
|
if (frame->root->ctime.tv_sec == 0) {
|
||
|
gf_msg(this->name, GF_LOG_WARNING, errno, P_MSG_SETMDATA_FAILED,
|
||
|
"posix set mdata failed, No ctime : %s gfid:%s", real_path,
|
||
|
@@ -643,9 +656,6 @@ posix_set_parent_ctime(call_frame_t *frame, xlator_t *this,
|
||
|
|
||
|
if (inode && priv->ctime) {
|
||
|
(void)posix_get_parent_mdata_flag(frame->root->flags, &flag);
|
||
|
- if ((flag.ctime == 0) && (flag.mtime == 0) && (flag.atime == 0)) {
|
||
|
- goto out;
|
||
|
- }
|
||
|
ret = posix_set_mdata_xattr(this, real_path, fd, inode,
|
||
|
&frame->root->ctime, stbuf, &flag,
|
||
|
_gf_false);
|
||
|
@@ -655,7 +665,6 @@ posix_set_parent_ctime(call_frame_t *frame, xlator_t *this,
|
||
|
uuid_utoa(inode->gfid));
|
||
|
}
|
||
|
}
|
||
|
-out:
|
||
|
return;
|
||
|
}
|
||
|
|
||
|
--
|
||
|
1.8.3.1
|
||
|
|