From 09d981409f1379265ee02db9cd5f62672fba4747 Mon Sep 17 00:00:00 2001 From: Jeff Darcy Date: Wed, 10 Oct 2012 09:12:50 -0400 Subject: nfs: do lookup on getattr after brick-status change By doing a lookup, we get a chance to do all of the self-heal checks that would occur if we were using native protocol, and return proper status if the self-heal fails. Best of all, we don't need to misrepresent times. Change-Id: I76477d1e5fce4d83e4029e02fcdd71e81e23110d BUG: 830134 Signed-off-by: Jeff Darcy Reviewed-on: http://review.gluster.org/4058 Tested-by: Gluster Build System Reviewed-by: Vijay Bellur --- xlators/nfs/server/src/nfs-common.c | 35 ++++++++++++++++++++ xlators/nfs/server/src/nfs-common.h | 3 ++ xlators/nfs/server/src/nfs-fops.c | 40 +++++++++++++++++++++-- xlators/nfs/server/src/nfs-mem-types.h | 1 + xlators/nfs/server/src/nfs.c | 35 +++++++++++--------- xlators/nfs/server/src/nfs.h | 6 ++++ xlators/nfs/server/src/nfs3.c | 32 ++++++++++++++++++ xlators/nfs/server/src/nlm4.c | 59 ++++++++++++++++++++++------------ 8 files changed, 171 insertions(+), 40 deletions(-) (limited to 'xlators/nfs') diff --git a/xlators/nfs/server/src/nfs-common.c b/xlators/nfs/server/src/nfs-common.c index d35df87b..67509b54 100644 --- a/xlators/nfs/server/src/nfs-common.c +++ b/xlators/nfs/server/src/nfs-common.c @@ -432,3 +432,38 @@ nfs_hash_gfid (uuid_t gfid) } +void +nfs_fix_generation (xlator_t *this, inode_t *inode) +{ + uint64_t raw_ctx = 0; + struct nfs_inode_ctx *ictx = NULL; + struct nfs_state *priv = NULL; + int ret = -1; + + if (!inode) { + return; + } + priv = this->private; + + if (inode_ctx_get(inode,this,&raw_ctx) == 0) { + ictx = (struct nfs_inode_ctx *)raw_ctx; + ictx->generation = priv->generation; + } + else { + ictx = GF_CALLOC (1, sizeof (struct nfs_inode_ctx), + gf_nfs_mt_inode_ctx); + if (!ictx) { + gf_log (this->name, GF_LOG_ERROR, + "could not allocate nfs inode ctx"); + return; + } + INIT_LIST_HEAD(&ictx->shares); + ictx->generation = priv->generation; + ret = inode_ctx_put (inode, this, (uint64_t)ictx); + if (ret) { + gf_log (this->name, GF_LOG_ERROR, + "could not store nfs inode ctx"); + return; + } + } +} diff --git a/xlators/nfs/server/src/nfs-common.h b/xlators/nfs/server/src/nfs-common.h index 88fc1496..f74bb318 100644 --- a/xlators/nfs/server/src/nfs-common.h +++ b/xlators/nfs/server/src/nfs-common.h @@ -84,4 +84,7 @@ nfs_hash_gfid (uuid_t gfid); extern int nfs_gfid_loc_fill (inode_table_t *itable, uuid_t gfid, loc_t *loc, int how); + +void +nfs_fix_generation (xlator_t *this, inode_t *inode); #endif diff --git a/xlators/nfs/server/src/nfs-fops.c b/xlators/nfs/server/src/nfs-fops.c index e2eedf43..5a2cf286 100644 --- a/xlators/nfs/server/src/nfs-fops.c +++ b/xlators/nfs/server/src/nfs-fops.c @@ -353,9 +353,6 @@ out: } \ } while (0) \ - - - /* Fops Layer Explained * The fops layer has three types of functions. They can all be identified by * their names. Here are the three patterns: @@ -388,6 +385,23 @@ nfs_fop_lookup_cbk (call_frame_t *frame, void *cookie, xlator_t *this, { struct nfs_fop_local *local = NULL; fop_lookup_cbk_t progcbk; + char *sh_fail_val = NULL; + + /* + * With native protocol, self-heal failures would be detected during + * open. NFS doesn't issue that open when revalidating cache, so we + * have to check for failures here instead. + */ + if (dict_get_str(xattr,"sh-failed",&sh_fail_val) == 0) { + if (strcmp(sh_fail_val,"1") == 0) { + op_ret = -1; + op_errno = EIO; + } + } + + if (op_ret == 0) { + nfs_fix_generation(this,inode); + } nfl_to_prog_data (local, progcbk, frame); nfs_fop_restore_root_ino (local, op_ret, buf, NULL, NULL, postparent); @@ -759,6 +773,10 @@ nfs_fop_create_cbk (call_frame_t *frame, void *cookie, xlator_t *this, struct nfs_fop_local *nfl = NULL; fop_create_cbk_t progcbk = NULL; + if (op_ret == 0) { + nfs_fix_generation(this,inode); + } + nfl_to_prog_data (nfl, progcbk, frame); nfs_fop_restore_root_ino (nfl, op_ret, buf, NULL, preparent, postparent); @@ -861,6 +879,10 @@ nfs_fop_mkdir_cbk (call_frame_t *frame, void *cookie, xlator_t *this, struct nfs_fop_local *nfl = NULL; fop_mkdir_cbk_t progcbk = NULL; + if (op_ret == 0) { + nfs_fix_generation(this,inode); + } + nfl_to_prog_data (nfl, progcbk, frame); nfs_fop_restore_root_ino (nfl, op_ret, buf, NULL,preparent, postparent); if (progcbk) @@ -910,6 +932,10 @@ nfs_fop_symlink_cbk (call_frame_t *frame, void *cookie, xlator_t *this, struct nfs_fop_local *nfl = NULL; fop_symlink_cbk_t progcbk = NULL; + if (op_ret == 0) { + nfs_fix_generation(this,inode); + } + nfl_to_prog_data (nfl, progcbk, frame); nfs_fop_restore_root_ino (nfl, op_ret,buf, NULL, preparent, postparent); if (progcbk) @@ -1006,6 +1032,10 @@ nfs_fop_mknod_cbk (call_frame_t *frame, void *cookie, xlator_t *this, struct nfs_fop_local *nfl = NULL; fop_mknod_cbk_t progcbk = NULL; + if (op_ret == 0) { + nfs_fix_generation(this,inode); + } + nfl_to_prog_data (nfl, progcbk, frame); nfs_fop_restore_root_ino (nfl, op_ret,buf, NULL, preparent, postparent); if (progcbk) @@ -1153,6 +1183,10 @@ nfs_fop_link_cbk (call_frame_t *frame, void *cookie, xlator_t *this, struct nfs_fop_local *nfl = NULL; fop_link_cbk_t progcbk = NULL; + if (op_ret == 0) { + nfs_fix_generation(this,inode); + } + nfl_to_prog_data (nfl, progcbk, frame); nfs_fop_restore_root_ino (nfl, op_ret, buf, NULL, preparent, postparent); diff --git a/xlators/nfs/server/src/nfs-mem-types.h b/xlators/nfs/server/src/nfs-mem-types.h index de25b08a..005598c1 100644 --- a/xlators/nfs/server/src/nfs-mem-types.h +++ b/xlators/nfs/server/src/nfs-mem-types.h @@ -53,6 +53,7 @@ enum gf_nfs_mem_types_ { gf_nfs_mt_nlm4_nlmclnt, gf_nfs_mt_nlm4_share, gf_nfs_mt_aux_gids, + gf_nfs_mt_inode_ctx, gf_nfs_mt_end }; #endif diff --git a/xlators/nfs/server/src/nfs.c b/xlators/nfs/server/src/nfs.c index 0a5a9d1e..033d9521 100644 --- a/xlators/nfs/server/src/nfs.c +++ b/xlators/nfs/server/src/nfs.c @@ -759,6 +759,7 @@ nfs_init_state (xlator_t *this) this->private = (void *)nfs; INIT_LIST_HEAD (&nfs->versions); + nfs->generation = 1965; ret = 0; @@ -843,24 +844,26 @@ int notify (xlator_t *this, int32_t event, void *data, ...) { xlator_t *subvol = NULL; + struct nfs_state *priv = NULL; subvol = (xlator_t *)data; gf_log (GF_NFS, GF_LOG_TRACE, "Notification received: %d", event); - switch (event) - { - case GF_EVENT_CHILD_UP: - { - nfs_startup_subvolume (this, subvol); - break; - } - case GF_EVENT_PARENT_UP: - { - default_notify (this, GF_EVENT_PARENT_UP, data); - break; - } + switch (event) { + case GF_EVENT_CHILD_UP: + nfs_startup_subvolume (this, subvol); + break; + + case GF_EVENT_CHILD_MODIFIED: + priv = this->private; + ++(priv->generation); + break; + + case GF_EVENT_PARENT_UP: + default_notify (this, GF_EVENT_PARENT_UP, data); + break; } return 0; @@ -882,14 +885,14 @@ fini (xlator_t *this) int32_t nfs_forget (xlator_t *this, inode_t *inode) { - uint64_t ctx = 0; - struct list_head *head = NULL; + uint64_t ctx = 0; + struct nfs_inode_ctx *ictx = NULL; if (inode_ctx_del (inode, this, &ctx)) return -1; - head = (struct list_head *)ctx; - GF_FREE (head); + ictx = (struct nfs_inode_ctx *)ctx; + GF_FREE (ictx); return 0; } diff --git a/xlators/nfs/server/src/nfs.h b/xlators/nfs/server/src/nfs.h index c3deba00..7d5163df 100644 --- a/xlators/nfs/server/src/nfs.h +++ b/xlators/nfs/server/src/nfs.h @@ -91,6 +91,12 @@ struct nfs_state { gf_boolean_t server_aux_gids; uint32_t server_aux_gids_max_age; gid_cache_t gid_cache; + uint32_t generation; +}; + +struct nfs_inode_ctx { + struct list_head shares; + uint32_t generation; }; #define gf_nfs_dvm_on(nfsstt) (((struct nfs_state *)nfsstt)->dynamicvolumes == GF_NFS_DVM_ON) diff --git a/xlators/nfs/server/src/nfs3.c b/xlators/nfs/server/src/nfs3.c index 1c379997..7c01d030 100644 --- a/xlators/nfs/server/src/nfs3.c +++ b/xlators/nfs/server/src/nfs3.c @@ -32,6 +32,7 @@ #include "nfs3.h" #include "mem-pool.h" #include "logging.h" +#include "nfs-common.h" #include "nfs-fops.h" #include "nfs-inodes.h" #include "nfs-generics.h" @@ -692,12 +693,21 @@ nfs3svc_getattr_lookup_cbk (call_frame_t *frame, void *cookie, xlator_t *this, cs = frame->local; + /* + * Somewhat counter-intuitively, we don't need to look for sh-failed + * here. Failing this getattr will generate a new lookup from the + * client, and nfs_fop_lookup_cbk will detect any self-heal failures. + */ + if (op_ret == -1) { gf_log (GF_NFS, GF_LOG_WARNING, "%x: %s => -1 (%s)", rpcsvc_request_xid (cs->req), cs->resolvedloc.path, strerror (op_errno)); status = nfs3_errno_to_nfsstat3 (op_errno); } + else { + nfs_fix_generation(this,inode); + } nfs3_log_common_res (rpcsvc_request_xid (cs->req), NFS3_GETATTR, status, op_errno); @@ -743,6 +753,9 @@ nfs3_getattr_resume (void *carg) int ret = -EFAULT; nfs_user_t nfu = {0, }; nfs3_call_state_t *cs = NULL; + uint64_t raw_ctx = 0; + struct nfs_inode_ctx *ictx = NULL; + struct nfs_state *priv = NULL; if (!carg) return ret; @@ -769,9 +782,28 @@ nfs3_getattr_resume (void *carg) goto nfs3err; } + /* + * If brick state changed, we need to force a proper lookup cycle (as + * would happen in native protocol) to do self-heal checks. We detect + * this by comparing the generation number for the last successful + * creation/lookup on the inode to the current number, so inodes that + * haven't been validated since the state change are affected. + */ + if (inode_ctx_get(cs->resolvedloc.inode,cs->nfsx,&raw_ctx) == 0) { + ictx = (struct nfs_inode_ctx *)raw_ctx; + priv = cs->nfsx->private; + if (ictx->generation != priv->generation) { + ret = nfs_lookup (cs->nfsx, cs->vol, &nfu, + &cs->resolvedloc, + nfs3svc_getattr_lookup_cbk, cs); + goto check_err; + } + } + ret = nfs_stat (cs->nfsx, cs->vol, &nfu, &cs->resolvedloc, nfs3svc_getattr_stat_cbk, cs); +check_err: if (ret < 0) { gf_log (GF_NFS3, GF_LOG_ERROR, "Stat fop failed: %s: %s", cs->oploc.path, strerror (-ret)); diff --git a/xlators/nfs/server/src/nlm4.c b/xlators/nfs/server/src/nlm4.c index 05de66d5..2e02a583 100644 --- a/xlators/nfs/server/src/nlm4.c +++ b/xlators/nfs/server/src/nlm4.c @@ -1789,25 +1789,38 @@ nlm4_add_share_to_inode (nlm_share_t *share) struct list_head *head = NULL; xlator_t *this = NULL; inode_t *inode = NULL; + struct nfs_inode_ctx *ictx = NULL; + struct nfs_state *priv = NULL; this = THIS; + priv = this->private; inode = share->inode; ret = inode_ctx_get (inode, this, &ctx); - head = (struct list_head *)ctx; - if (ret || !head) { - head = GF_CALLOC (1, sizeof (struct list_head), - gf_common_mt_list_head); - if (!head ) { + ictx = GF_CALLOC (1, sizeof (struct nfs_inode_ctx), + gf_nfs_mt_inode_ctx); + if (!ictx ) { + gf_log (this->name, GF_LOG_ERROR, + "could not allocate nfs inode ctx"); ret = -1; goto out; } + ictx->generation = priv->generation; + head = &ictx->shares; INIT_LIST_HEAD (head); - ret = inode_ctx_put (inode, this, (uint64_t)head); - if (ret) + + ret = inode_ctx_put (inode, this, (uint64_t)ictx); + if (ret) { + gf_log (this->name, GF_LOG_ERROR, + "could not store share list"); goto out; + } + } + else { + ictx = (struct nfs_inode_ctx *)ctx; + head = &ictx->shares; } list_add (&share->inode_list, head); @@ -1829,6 +1842,7 @@ nlm4_approve_share_reservation (nfs3_call_state_t *cs) inode_t *inode = NULL; nlm_share_t *share = NULL; struct list_head *head = NULL; + struct nfs_inode_ctx *ictx = NULL; if (!cs) goto out; @@ -1840,8 +1854,9 @@ nlm4_approve_share_reservation (nfs3_call_state_t *cs) ret = 0; goto out; } + ictx = (struct nfs_inode_ctx *)ctx; - head = (struct list_head *)ctx; + head = &ictx->shares; if (!head || list_empty (head)) goto out; @@ -2014,18 +2029,19 @@ nlm4svc_share (rpcsvc_request_t *req) int nlm4_remove_share_reservation (nfs3_call_state_t *cs) { - int ret = -1; - uint64_t ctx = 0; - fsh_mode req_mode = 0; - fsh_access req_access = 0; - nlm_share_t *share = NULL; - nlm_share_t *tmp = NULL; - nlm_client_t *client = NULL; - char *caller = NULL; - inode_t *inode = NULL; - xlator_t *this = NULL; - struct list_head *head = NULL; - nlm4_shareargs *args = NULL; + int ret = -1; + uint64_t ctx = 0; + fsh_mode req_mode = 0; + fsh_access req_access = 0; + nlm_share_t *share = NULL; + nlm_share_t *tmp = NULL; + nlm_client_t *client = NULL; + char *caller = NULL; + inode_t *inode = NULL; + xlator_t *this = NULL; + struct list_head *head = NULL; + nlm4_shareargs *args = NULL; + struct nfs_inode_ctx *ictx = NULL; LOCK (&nlm_client_list_lk); @@ -2055,8 +2071,9 @@ nlm4_remove_share_reservation (nfs3_call_state_t *cs) inode->gfid, caller); goto out; } + ictx = (struct nfs_inode_ctx *)ctx; - head = (struct list_head *)ctx; + head = &ictx->shares; if (list_empty (head)) { ret = -1; goto out; -- cgit