From f72e2b00a13709717a6c7e0ff95d4c7a8f58b020 Mon Sep 17 00:00:00 2001 From: Yaniv Kaul Date: Sun, 1 Dec 2019 13:57:28 +0200 Subject: posix-metadata.c: try to getxattr in one call. Another location where instead of 2 sys calls we strive to get the xattr in a single call, by guesstimating the required size And avoid (or try to) not to first read the xattr len, then another call to actually fetch. Instead, use a sane size (256 bytes - worth checking if it makes sense or by default use a larger size), and see if we can fetch it. If we fail, we'll read the size and re-fetch. Such changes are needed elsewhere too (see https://github.com/gluster/glusterfs/issues/720 ) Change-Id: I466cea9d8b12fc45f6b37d202b1294ca28cd1fdd updates: bz#1193929 Signed-off-by: Yaniv Kaul --- xlators/storage/posix/src/posix-metadata.c | 109 ++++++++++++++++------------- 1 file changed, 61 insertions(+), 48 deletions(-) (limited to 'xlators/storage/posix/src/posix-metadata.c') diff --git a/xlators/storage/posix/src/posix-metadata.c b/xlators/storage/posix/src/posix-metadata.c index 872a693f513..6e324c8e608 100644 --- a/xlators/storage/posix/src/posix-metadata.c +++ b/xlators/storage/posix/src/posix-metadata.c @@ -74,17 +74,14 @@ static int posix_fetch_mdata_xattr(xlator_t *this, const char *real_path_arg, int _fd, inode_t *inode, posix_mdata_t *metadata, int *op_errno) { - size_t size = -1; + size_t size = 256; int op_ret = -1; char *value = NULL; gf_boolean_t fd_based_fop = _gf_false; char gfid_str[64] = {0}; char *real_path = NULL; - char *key = GF_XATTR_MDATA_KEY; - if (!metadata) { - op_ret = -1; goto out; } @@ -95,82 +92,98 @@ posix_fetch_mdata_xattr(xlator_t *this, const char *real_path_arg, int _fd, GF_VALIDATE_OR_GOTO(this->name, inode, out); MAKE_HANDLE_PATH(real_path, this, inode->gfid, NULL); if (!real_path) { + *op_errno = errno; uuid_utoa_r(inode->gfid, gfid_str); - gf_msg(this->name, GF_LOG_WARNING, errno, P_MSG_LSTAT_FAILED, + gf_msg(this->name, GF_LOG_WARNING, *op_errno, P_MSG_LSTAT_FAILED, "lstat on gfid %s failed", gfid_str); - op_ret = -1; - *op_errno = errno; goto out; } } + value = GF_MALLOC(size * sizeof(char), gf_posix_mt_char); + if (!value) { + *op_errno = ENOMEM; + goto out; + } + if (fd_based_fop) { - size = sys_fgetxattr(_fd, key, NULL, 0); + size = sys_fgetxattr(_fd, GF_XATTR_MDATA_KEY, value, size); } else if (real_path_arg) { - size = sys_lgetxattr(real_path_arg, key, NULL, 0); + size = sys_lgetxattr(real_path_arg, GF_XATTR_MDATA_KEY, value, size); } else if (real_path) { - size = sys_lgetxattr(real_path, key, NULL, 0); + size = sys_lgetxattr(real_path, GF_XATTR_MDATA_KEY, value, size); } if (size == -1) { *op_errno = errno; + if (value) { + GF_FREE(value); + value = NULL; + } if ((*op_errno == ENOTSUP) || (*op_errno == ENOSYS)) { GF_LOG_OCCASIONALLY(gf_posix_xattr_enotsup_log, this->name, GF_LOG_WARNING, - "Extended attributes not " - "supported (try remounting" - " brick with 'user_xattr' " + "Extended attributes not supported" + " (try remounting brick with 'user xattr' " "flag)"); } else if (*op_errno == ENOATTR || *op_errno == ENODATA) { gf_msg_debug(this->name, 0, - "No such attribute:%s for file %s " - "gfid: %s", - key, + "No such attribute:%s for file %s gfid: %s", + GF_XATTR_MDATA_KEY, real_path ? real_path : (real_path_arg ? real_path_arg : "null"), 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 ", + goto out; + } + + if (fd_based_fop) { + size = sys_fgetxattr(_fd, GF_XATTR_MDATA_KEY, NULL, 0); + } else if (real_path_arg) { + size = sys_lgetxattr(real_path_arg, GF_XATTR_MDATA_KEY, NULL, 0); + } else if (real_path) { + size = sys_lgetxattr(real_path, GF_XATTR_MDATA_KEY, NULL, 0); + } + + if (size == -1) { /* give up now and exist with an error */ + *op_errno = errno; + gf_msg(this->name, GF_LOG_ERROR, *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"), - inode ? uuid_utoa(inode->gfid) : "null", key); + inode ? uuid_utoa(inode->gfid) : "null", GF_XATTR_MDATA_KEY); + goto out; } - op_ret = -1; - goto out; - } - value = GF_CALLOC(size + 1, sizeof(char), gf_posix_mt_char); - if (!value) { - op_ret = -1; - *op_errno = ENOMEM; - goto out; - } + value = GF_MALLOC(size * sizeof(char), gf_posix_mt_char); + if (!value) { + *op_errno = ENOMEM; + goto out; + } - if (fd_based_fop) { - size = sys_fgetxattr(_fd, key, value, size); - } else if (real_path_arg) { - size = sys_lgetxattr(real_path_arg, key, value, size); - } else if (real_path) { - size = sys_lgetxattr(real_path, key, value, size); - } - if (size == -1) { - op_ret = -1; - *op_errno = errno; - gf_msg(this->name, GF_LOG_ERROR, errno, P_MSG_XATTR_FAILED, - "getxattr failed on " - " on %s gfid: %s key: %s ", - real_path ? real_path : (real_path_arg ? real_path_arg : "null"), - inode ? uuid_utoa(inode->gfid) : "null", key); - goto out; + if (fd_based_fop) { + size = sys_fgetxattr(_fd, GF_XATTR_MDATA_KEY, value, size); + } else if (real_path_arg) { + size = sys_lgetxattr(real_path_arg, GF_XATTR_MDATA_KEY, value, + size); + } else if (real_path) { + size = sys_lgetxattr(real_path, GF_XATTR_MDATA_KEY, value, size); + } + if (size == -1) { + *op_errno = errno; + gf_msg(this->name, GF_LOG_ERROR, *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"), + inode ? uuid_utoa(inode->gfid) : "null", GF_XATTR_MDATA_KEY); + goto out; + } } - posix_mdata_from_disk(metadata, (posix_mdata_disk_t *)value); op_ret = 0; out: - GF_FREE(value); + if (value) + GF_FREE(value); return op_ret; } -- cgit