From b87d96b97d4a0cdc0883bec8ea8b4730b82fb3ba Mon Sep 17 00:00:00 2001 From: Santosh Kumar Pradhan Date: Thu, 21 Nov 2013 17:13:58 +0530 Subject: gNFS: More clean up for Gluster NFS 1) Fix the typo in NFS default ACL The typo was introduced as part of the Fix to BZ 1009210 i.e. http://review.gluster.org/5980. The user ACL xattr structure was passed to default ACL xattr. 2) Clean up NFS code to avoid unnecessary SEGV in rpcsvc_drc_reconfigure() which was not validating the svc->drc. Add a routine rpcsvc_drc_deinit() to handle the clean up of DRC specific data structures. For init(), use rpcsvc_drc_init(). 3) nfs_init_state() was returning wrong value even if the registration with portmapper failed, causing the NFS server process to hang around. As a result it used to get SEGV during rpcsvc_drc_reconfigure(). 4) Clean up memfactor usage across nfs.c nfs3.c. Change-Id: I5cea26cb68dd8a822ec0ae104952f67fe63fa703 BUG: 1009210 Signed-off-by: Santosh Kumar Pradhan Reviewed-on: http://review.gluster.org/6329 Reviewed-by: Rajesh Joseph Tested-by: Gluster Build System Reviewed-by: Vijay Bellur --- rpc/rpc-lib/src/rpc-drc.c | 104 +++++++++++++++++++++++++++--------------- rpc/rpc-lib/src/rpc-drc.h | 4 +- xlators/nfs/server/src/acl3.c | 2 +- xlators/nfs/server/src/nfs.c | 74 ++++++++++++++++++------------ xlators/nfs/server/src/nfs3.c | 6 +-- xlators/nfs/server/src/nfs3.h | 2 - 6 files changed, 116 insertions(+), 76 deletions(-) diff --git a/rpc/rpc-lib/src/rpc-drc.c b/rpc/rpc-lib/src/rpc-drc.c index 8181e6aeed8..e7ba114dd27 100644 --- a/rpc/rpc-lib/src/rpc-drc.c +++ b/rpc/rpc-lib/src/rpc-drc.c @@ -708,15 +708,10 @@ rpcsvc_drc_init (rpcsvc_t *svc, dict_t *options) uint32_t drc_size = 0; uint32_t drc_factor = 0; rpcsvc_drc_globals_t *drc = NULL; - static gf_boolean_t drc_inited = _gf_false; GF_ASSERT (svc); GF_ASSERT (options); - /* Already inited */ - if (drc_inited) - return 0; - if (!svc->drc) { drc = GF_CALLOC (1, sizeof (rpcsvc_drc_globals_t), gf_common_mt_drc_globals_t); @@ -742,11 +737,10 @@ rpcsvc_drc_init (rpcsvc_t *svc, dict_t *options) gf_log (GF_RPCSVC, GF_LOG_INFO, "drc user options need second look"); ret = _gf_true; } - drc->enable_drc = ret; if (ret == _gf_false) { /* drc off */ - gf_log (GF_RPCSVC, GF_LOG_DEBUG, "DRC is off"); + gf_log (GF_RPCSVC, GF_LOG_INFO, "DRC is manually turned OFF"); ret = 0; goto out; } @@ -802,8 +796,6 @@ rpcsvc_drc_init (rpcsvc_t *svc, dict_t *options) gf_log (GF_RPCSVC, GF_LOG_DEBUG, "drc init successful"); drc->status = DRC_INITIATED; - drc_inited = _gf_true; - out: UNLOCK (&drc->lock); if (ret == -1) { @@ -817,6 +809,32 @@ rpcsvc_drc_init (rpcsvc_t *svc, dict_t *options) return ret; } +int +rpcsvc_drc_deinit (rpcsvc_t *svc) +{ + rpcsvc_drc_globals_t *drc = NULL; + + if (!svc) + return (-1); + + drc = svc->drc; + if (!drc) + return (0); + + LOCK (&drc->lock); + (void) rpcsvc_unregister_notify (svc, rpcsvc_drc_notify, THIS); + if (drc->mempool) { + mem_pool_destroy (drc->mempool); + drc->mempool = NULL; + } + UNLOCK (&drc->lock); + + GF_FREE (drc); + svc->drc = NULL; + + return (0); +} + int rpcsvc_drc_reconfigure (rpcsvc_t *svc, dict_t *options) { @@ -825,48 +843,58 @@ rpcsvc_drc_reconfigure (rpcsvc_t *svc, dict_t *options) rpcsvc_drc_globals_t *drc = NULL; uint32_t drc_size = 0; + /* Input sanitization */ if ((!svc) || (!options)) return (-1); + /* If DRC was not enabled before, Let rpcsvc_drc_init() to + * take care of DRC initialization part. + */ drc = svc->drc; - /* reconfig for drc-size */ - if (dict_get_uint32 (options, "nfs.drc-size", &drc_size)) - drc_size = DRC_DEFAULT_CACHE_SIZE; - - if (drc->global_cache_size != drc_size) { - gf_log (GF_RPCSVC, GF_LOG_DEBUG, "nfs.drc-size size can not " - "be reconfigured without NFS server restart."); - return (-1); + if (!drc) { + return rpcsvc_drc_init(svc, options); } - /* reconfig for nfs.drc */ + /* DRC was already enabled before. Going to be reconfigured. Check + * if reconfigured options contain "nfs.drc" and "nfs.drc-size". + * + * NB: If DRC is "OFF", "drc-size" has no role to play. + * So, "drc-size" gets evaluated IFF DRC is "ON". + * + * If DRC is reconfigured, + * case 1: DRC is "ON" + * sub-case 1: drc-size remains same + * ACTION: Nothing to do. + * sub-case 2: drc-size just changed + * ACTION: rpcsvc_drc_deinit() followed by + * rpcsvc_drc_init(). + * + * case 2: DRC is "OFF" + * ACTION: rpcsvc_drc_deinit() + */ ret = dict_get_str_boolean (options, "nfs.drc", _gf_true); if (ret < 0) { - ret = _gf_true; + enable_drc = _gf_true; + } else { + enable_drc = ret; } - enable_drc = ret; - - if (drc->enable_drc == enable_drc) - return 0; - drc->enable_drc = enable_drc; + /* case 1: DRC is "ON"*/ if (enable_drc) { - if (drc == NULL) - return rpcsvc_drc_init(svc, options); - } else { - if (drc == NULL) + /* Fetch drc-size if reconfigured */ + if (dict_get_uint32 (options, "nfs.drc-size", &drc_size)) + drc_size = DRC_DEFAULT_CACHE_SIZE; + + /* case 1: sub-case 1*/ + if (drc->global_cache_size == drc_size) return (0); - LOCK (&drc->lock); - (void) rpcsvc_unregister_notify (svc, rpcsvc_drc_notify, THIS); - if (drc->mempool) { - mem_pool_destroy (drc->mempool); - drc->mempool = NULL; - } - UNLOCK (&drc->lock); - GF_FREE (drc); - svc->drc = NULL; + /* case 1: sub-case 2*/ + (void) rpcsvc_drc_deinit (svc); + return rpcsvc_drc_init (svc, options); } - return (0); + /* case 2: DRC is "OFF" */ + gf_log (GF_RPCSVC, GF_LOG_INFO, "DRC is manually turned OFF"); + return rpcsvc_drc_deinit (svc); } diff --git a/rpc/rpc-lib/src/rpc-drc.h b/rpc/rpc-lib/src/rpc-drc.h index 7dfaef9783c..c42c2a2c2fe 100644 --- a/rpc/rpc-lib/src/rpc-drc.h +++ b/rpc/rpc-lib/src/rpc-drc.h @@ -71,7 +71,6 @@ struct drc_globals { struct list_head cache_head; uint32_t client_count; struct list_head clients_head; - gf_boolean_t enable_drc; }; int @@ -98,6 +97,9 @@ rpcsvc_drc_priv (rpcsvc_drc_globals_t *drc); int rpcsvc_drc_init (rpcsvc_t *svc, dict_t *options); +int +rpcsvc_drc_deinit (rpcsvc_t *svc); + int rpcsvc_drc_reconfigure (rpcsvc_t *svc, dict_t *options); diff --git a/xlators/nfs/server/src/acl3.c b/xlators/nfs/server/src/acl3.c index 08b099b4e60..a9843c7108b 100644 --- a/xlators/nfs/server/src/acl3.c +++ b/xlators/nfs/server/src/acl3.c @@ -562,7 +562,7 @@ acl3svc_setacl (rpcsvc_request_t *req) } /* Populate xattr buffer for Default ACL */ - bufheader = (struct posix_acl_xattr_header *)(cs->aclxattr); + bufheader = (struct posix_acl_xattr_header *)(cs->daclxattr); bufheader->version = htole32(POSIX_ACL_VERSION); bufentry = bufheader->entries; for (i = 0; i < cs->daclcount; i++) { diff --git a/xlators/nfs/server/src/nfs.c b/xlators/nfs/server/src/nfs.c index 75c8fe44eaf..8c895c66d92 100644 --- a/xlators/nfs/server/src/nfs.c +++ b/xlators/nfs/server/src/nfs.c @@ -959,7 +959,8 @@ nfs_init_state (xlator_t *this) nfs->enable_nlm = _gf_false; } - nfs->rpcsvc = rpcsvc_init (this, this->ctx, this->options, 0); + nfs->rpcsvc = rpcsvc_init (this, this->ctx, + this->options, fopspoolsize); if (!nfs->rpcsvc) { ret = -1; gf_log (GF_NFS, GF_LOG_ERROR, "RPC service init failed"); @@ -995,6 +996,9 @@ nfs_drc_init (xlator_t *this) int ret = -1; rpcsvc_t *svc = NULL; + GF_VALIDATE_OR_GOTO (GF_NFS, this, out); + GF_VALIDATE_OR_GOTO (GF_NFS, this->private, out); + svc = ((struct nfs_state *)(this->private))->rpcsvc; if (!svc) goto out; @@ -1227,6 +1231,23 @@ reconfigure (xlator_t *this, dict_t *options) return (0); } +/* Main init() routine for NFS server xlator. It inits NFS v3 protocol + * and its dependent protocols e.g. ACL, MOUNT v3 (mount3), NLM and + * DRC. + * + * Usage: glusterfsd: + * glusterfs_process_volfp() => + * glusterfs_graph_activate() => + * glusterfs_graph_init() => + * xlator_init () => NFS init() routine + * + * If init() routine fails, the glusterfsd cleans up the NFS process + * by invoking cleanup_and_exit(). + * + * RETURN: + * 0 (SUCCESS) if all protocol specific inits PASS. + * -1 (FAILURE) if any of them FAILS. + */ int init (xlator_t *this) { @@ -1234,59 +1255,52 @@ init (xlator_t *this) { int ret = -1; if (!this) - return -1; + return (-1); nfs = nfs_init_state (this); if (!nfs) { gf_log (GF_NFS, GF_LOG_ERROR, "Failed to init nfs option"); - return -1; + return (-1); } ret = nfs_add_all_initiators (nfs); - if (ret == -1) { + if (ret) { gf_log (GF_NFS, GF_LOG_ERROR, "Failed to add initiators"); - goto err; + return (-1); } ret = nfs_init_subvolumes (nfs, this->children); - if (ret == -1) { - gf_log (GF_NFS, GF_LOG_CRITICAL, "Failed to init NFS " - "exports"); - goto err; + if (ret) { + gf_log (GF_NFS, GF_LOG_CRITICAL, "Failed to init NFS exports"); + return (-1); } ret = mount_init_state (this); - if (ret == -1) { - gf_log (GF_NFS, GF_LOG_CRITICAL, "Failed to init Mount" - "state"); - goto err; + if (ret) { + gf_log (GF_NFS, GF_LOG_CRITICAL, "Failed to init Mount state"); + return (-1); } ret = nlm4_init_state (this); - if (ret == -1) { - gf_log (GF_NFS, GF_LOG_CRITICAL, "Failed to init NLM" - "state"); - goto err; + if (ret) { + gf_log (GF_NFS, GF_LOG_CRITICAL, "Failed to init NLM state"); + return (-1); } ret = nfs_init_versions (nfs, this); - if (ret == -1) { - gf_log (GF_NFS, GF_LOG_ERROR, "Failed to initialize " - "protocols"); - /* Do not return an error on this. If we dont return - * an error, the process keeps running and it helps - * to point out where the log is by doing ps ax|grep gluster. - */ - ret = 0; - goto err; + if (ret) { + gf_log (GF_NFS, GF_LOG_ERROR, "Failed to initialize protocols"); + return (-1); } ret = nfs_drc_init (this); - if (ret == 0) - gf_log (GF_NFS, GF_LOG_INFO, "NFS service started"); -err: + if (ret) { + gf_log (GF_NFS, GF_LOG_ERROR, "Failed to initialize DRC"); + return (-1); + } - return ret; + gf_log (GF_NFS, GF_LOG_INFO, "NFS service started"); + return (0); /* SUCCESS */ } diff --git a/xlators/nfs/server/src/nfs3.c b/xlators/nfs/server/src/nfs3.c index f914c31937d..0fea135c784 100644 --- a/xlators/nfs/server/src/nfs3.c +++ b/xlators/nfs/server/src/nfs3.c @@ -5256,8 +5256,6 @@ nfs3_init_options (struct nfs3_state *nfs3, dict_t *options) * accommodate the NFS headers also in the same buffer. */ nfs3->iobsize = nfs3->iobsize * 2; - /* mem-factor */ - nfs3->memfactor = GF_NFS3_DEFAULT_MEMFACTOR; ret = 0; err: return ret; @@ -5507,7 +5505,7 @@ nfs3_init_state (xlator_t *nfsx) unsigned int localpool = 0; struct nfs_state *nfs = NULL; - if (!nfsx) + if ((!nfsx) || (!nfsx->private)) return NULL; nfs3 = (struct nfs3_state *)GF_CALLOC (1, sizeof (*nfs3), @@ -5526,7 +5524,7 @@ nfs3_init_state (xlator_t *nfsx) nfs3->iobpool = nfsx->ctx->iobuf_pool; - localpool = nfs3->memfactor * GF_NFS_CONCURRENT_OPS_MULT; + localpool = nfs->memfactor * GF_NFS_CONCURRENT_OPS_MULT; gf_log (GF_NFS3, GF_LOG_TRACE, "local pool: %d", localpool); nfs3->localpool = mem_pool_new (nfs3_call_state_t, localpool); if (!nfs3->localpool) { diff --git a/xlators/nfs/server/src/nfs3.h b/xlators/nfs/server/src/nfs3.h index 0c35445a471..023b394cf5a 100644 --- a/xlators/nfs/server/src/nfs3.h +++ b/xlators/nfs/server/src/nfs3.h @@ -143,8 +143,6 @@ typedef struct nfs3_state { */ uint64_t iobsize; - unsigned int memfactor; - struct list_head fdlru; gf_lock_t fdlrulock; int fdcount; -- cgit