summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorXavi Hernandez <xhernandez@redhat.com>2021-03-09 00:24:07 +0100
committerXavi Hernandez <xhernandez@gmail.com>2021-04-09 18:30:14 +0200
commitc3172d4883adf0bf277fd3a05825845978c7d7f2 (patch)
tree7b29410810fbf5d62ccee4d9db6d4ebda1733b7d
parent74f93a8662858a3b41e0c153cc0976ea76ff1eae (diff)
downloadglusterfs-c3172d4883adf0bf277fd3a05825845978c7d7f2.tar.gz
glusterfs-c3172d4883adf0bf277fd3a05825845978c7d7f2.tar.xz
glusterfs-c3172d4883adf0bf277fd3a05825845978c7d7f2.zip
afr: fix directory entry count
AFR may hide some existing entries from a directory when reading it because they are generated internally for private management. However the returned number of entries from readdir() function is not updated accordingly. So it may return a number higher than the real entries present in the gf_dirent list. This may cause unexpected behavior of clients, including gfapi which incorrectly assumes that there was an entry when the list was actually empty. This patch also makes the check in gfapi more robust to avoid similar issues that could appear in the future. Fixes: #2232 Change-Id: I81ba3699248a53ebb0ee4e6e6231a4301436f763 Signed-off-by: Xavi Hernandez <xhernandez@redhat.com>
-rw-r--r--api/src/glfs-fops.c3
-rw-r--r--tests/bugs/replicate/issue-2232.c85
-rw-r--r--tests/bugs/replicate/issue-2232.t34
-rw-r--r--xlators/cluster/afr/src/afr-dir-read.c11
4 files changed, 129 insertions, 4 deletions
diff --git a/api/src/glfs-fops.c b/api/src/glfs-fops.c
index 7eeabd691b..41f9339370 100644
--- a/api/src/glfs-fops.c
+++ b/api/src/glfs-fops.c
@@ -3753,8 +3753,9 @@ glfd_entry_refresh(struct glfs_fd *glfd, int plus)
errno = 0;
}
- if (ret > 0)
+ if ((ret > 0) && !list_empty(&glfd->entries)) {
glfd->next = list_entry(glfd->entries.next, gf_dirent_t, list);
+ }
gf_dirent_free(&old);
out:
diff --git a/tests/bugs/replicate/issue-2232.c b/tests/bugs/replicate/issue-2232.c
new file mode 100644
index 0000000000..df547c20d8
--- /dev/null
+++ b/tests/bugs/replicate/issue-2232.c
@@ -0,0 +1,85 @@
+
+#include <stdio.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <string.h>
+#include <glusterfs/api/glfs.h>
+
+int main(int argc, char **argv)
+{
+ char log[128];
+ struct dirent entry;
+ struct dirent *ent;
+ glfs_xreaddirp_stat_t *xstat;
+ int ret, flags;
+
+ if (argc != 3) {
+ fprintf(stderr, "Syntax: %s <hostname> <volume>\n", argv[0]);
+ exit(1);
+ }
+ char *hostname = argv[1];
+ char *volname = argv[2];
+
+ glfs_t *fs = glfs_new(volname);
+ if (!fs) {
+ fprintf(stderr, "glfs_new() failed\n");
+ exit(1);
+ }
+
+ ret = glfs_set_volfile_server(fs, "tcp", hostname, 24007);
+ if (ret < 0) {
+ fprintf(stderr, "glfs_set_volfile_server() failed\n");
+ return ret;
+ }
+
+ sprintf(log, "/tmp/logs-%d.log", getpid());
+ ret = glfs_set_logging(fs, log, 9);
+ if (ret < 0) {
+ fprintf(stderr, "glfs_set_logging() failed\n");
+ return ret;
+ }
+
+ ret = glfs_init(fs);
+ if (ret < 0) {
+ fprintf(stderr, "glfs_init() failed\n");
+ return ret;
+ }
+
+ glfs_fd_t *fd = glfs_opendir(fs, "/");
+ if (fd == NULL) {
+ fprintf(stderr, "glfs_opendir() failed\n");
+ return 1;
+ }
+
+ flags = GFAPI_XREADDIRP_STAT | GFAPI_XREADDIRP_HANDLE;
+ xstat = NULL;
+ while ((ret = glfs_xreaddirplus_r(fd, flags, &xstat, &entry, &ent)) > 0) {
+ if (xstat != NULL) {
+ glfs_free(xstat);
+ }
+ if ((strcmp(ent->d_name, ".") == 0) ||
+ (strcmp(ent->d_name, "..") == 0)) {
+ xstat = NULL;
+ continue;
+ }
+ if ((xstat == NULL) || ((ret & GFAPI_XREADDIRP_HANDLE) == 0)) {
+ fprintf(stderr, "glfs_xreaddirplus_r() failed: %s\n",
+ strerror(errno));
+ return 1;
+ }
+
+ xstat = NULL;
+ }
+
+ if (ret < 0) {
+ fprintf(stderr, "glfs_xreaddirplus_r() failed\n");
+ return ret;
+ }
+
+ glfs_close(fd);
+
+ glfs_fini(fs);
+
+ return ret;
+}
diff --git a/tests/bugs/replicate/issue-2232.t b/tests/bugs/replicate/issue-2232.t
new file mode 100644
index 0000000000..66a41e000c
--- /dev/null
+++ b/tests/bugs/replicate/issue-2232.t
@@ -0,0 +1,34 @@
+#!/bin/bash
+
+. $(dirname "${0}")/../../include.rc
+. $(dirname "${0}")/../../volume.rc
+
+cleanup;
+TEST gcc $(dirname "${0}")/issue-2232.c -o $(dirname "${0}")/issue-2232 -lgfapi
+TEST glusterd
+TEST pidof glusterd
+
+TEST $CLI volume create ${V0} replica 3 ${H0}:${B0}/${V0}{0..2}
+
+# Create a fake .glusterfs-anonymous-inode-... entry
+ANONINO=".glusterfs-anonymous-inode-aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"
+TEST mkdir ${B0}/${V0}{0..2}/${ANONINO}
+gfid="$(uuidgen)"
+hex="0x$(echo "${gfid}" | tr -d '-')"
+TEST assign_gfid "${hex}" "${B0}/${V0}0/${ANONINO}"
+TEST assign_gfid "${hex}" "${B0}/${V0}1/${ANONINO}"
+TEST assign_gfid "${hex}" "${B0}/${V0}2/${ANONINO}"
+TEST mkdir -p "${B0}/${V0}0/.glusterfs/${gfid:0:2}/${gfid:2:2}"
+TEST mkdir -p "${B0}/${V0}1/.glusterfs/${gfid:0:2}/${gfid:2:2}"
+TEST mkdir -p "${B0}/${V0}2/.glusterfs/${gfid:0:2}/${gfid:2:2}"
+TEST ln -s "../../00/00/00000000-0000-0000-0000-000000000001/${ANONINO}" "${B0}/${V0}0/.glusterfs/${gfid:0:2}/${gfid:2:2}/${gfid}"
+TEST ln -s "../../00/00/00000000-0000-0000-0000-000000000001/${ANONINO}" "${B0}/${V0}1/.glusterfs/${gfid:0:2}/${gfid:2:2}/${gfid}"
+TEST ln -s "../../00/00/00000000-0000-0000-0000-000000000001/${ANONINO}" "${B0}/${V0}2/.glusterfs/${gfid:0:2}/${gfid:2:2}/${gfid}"
+
+TEST $CLI volume start ${V0}
+
+TEST $(dirname "${0}")/issue-2232 ${H0} ${V0}
+
+TEST rm -f $(dirname $0)/issue-2232
+
+cleanup
diff --git a/xlators/cluster/afr/src/afr-dir-read.c b/xlators/cluster/afr/src/afr-dir-read.c
index f8bf8340da..a6a3470569 100644
--- a/xlators/cluster/afr/src/afr-dir-read.c
+++ b/xlators/cluster/afr/src/afr-dir-read.c
@@ -163,7 +163,7 @@ afr_validate_read_subvol(inode_t *inode, xlator_t *this, int par_read_subvol)
return 0;
}
-static void
+static int32_t
afr_readdir_transform_entries(call_frame_t *frame, gf_dirent_t *subvol_entries,
int subvol, gf_dirent_t *entries, fd_t *fd)
{
@@ -174,6 +174,7 @@ afr_readdir_transform_entries(call_frame_t *frame, gf_dirent_t *subvol_entries,
afr_private_t *priv = NULL;
gf_boolean_t need_heal = _gf_false;
gf_boolean_t validate_subvol = _gf_false;
+ int32_t count = 0;
this = THIS;
priv = this->private;
@@ -190,6 +191,7 @@ afr_readdir_transform_entries(call_frame_t *frame, gf_dirent_t *subvol_entries,
list_del_init(&entry->list);
list_add_tail(&entry->list, &entries->list);
+ count++;
if (!validate_subvol)
continue;
@@ -203,6 +205,8 @@ afr_readdir_transform_entries(call_frame_t *frame, gf_dirent_t *subvol_entries,
}
}
}
+
+ return count;
}
int32_t
@@ -228,8 +232,9 @@ afr_readdir_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
}
if (op_ret >= 0)
- afr_readdir_transform_entries(frame, subvol_entries, (long)cookie,
- &entries, local->fd);
+ op_ret = afr_readdir_transform_entries(frame, subvol_entries,
+ (long)cookie, &entries,
+ local->fd);
AFR_STACK_UNWIND(readdir, frame, op_ret, op_errno, &entries, xdata);