From 5459e74ef28bd08e48f95c2732f04144fdbbee56 Mon Sep 17 00:00:00 2001 From: linbaiye Date: Fri, 28 Sep 2012 18:56:26 +0800 Subject: Preventing client crashing as the callings of GF_CALLOC has been failed. As the callings of GF_CALLOC can seldom come to a failure, glusterfs client will crash due to segment fault. We should have returned once the variables of transaction's local can't be alloced. Change-Id: Ia3798b8349d832b23c7825e64dbad93ebe29cd1b BUG: 861335 Signed-off-by: linbaiye Reviewed-on: http://review.gluster.org/4005 Tested-by: Gluster Build System Reviewed-by: Anand Avati --- xlators/cluster/afr/src/afr-common.c | 7 ++-- xlators/cluster/afr/src/afr-dir-write.c | 48 ++++++++++++++++++++----- xlators/cluster/afr/src/afr-inode-write.c | 60 ++++++++++++++++++++++++------- xlators/cluster/afr/src/afr-transaction.c | 23 +++++++----- 4 files changed, 107 insertions(+), 31 deletions(-) (limited to 'xlators/cluster/afr') diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c index 4130adc6f..21272c0d7 100644 --- a/xlators/cluster/afr/src/afr-common.c +++ b/xlators/cluster/afr/src/afr-common.c @@ -2618,8 +2618,11 @@ afr_flush (call_frame_t *frame, xlator_t *this, fd_t *fd, dict_t *xdata) op_errno = -ret; goto out; } - afr_transaction (transaction_frame, this, AFR_DATA_TRANSACTION); - + ret = afr_transaction (transaction_frame, this, AFR_DATA_TRANSACTION); + if (ret < 0) { + op_errno = -ret; + goto out; + } ret = 0; out: diff --git a/xlators/cluster/afr/src/afr-dir-write.c b/xlators/cluster/afr/src/afr-dir-write.c index 304103541..49cf8d453 100644 --- a/xlators/cluster/afr/src/afr-dir-write.c +++ b/xlators/cluster/afr/src/afr-dir-write.c @@ -350,7 +350,11 @@ afr_create (call_frame_t *frame, xlator_t *this, local->transaction.main_frame = frame; local->transaction.basename = AFR_BASENAME (loc->path); - afr_transaction (transaction_frame, this, AFR_ENTRY_TRANSACTION); + ret = afr_transaction (transaction_frame, this, AFR_ENTRY_TRANSACTION); + if (ret < 0) { + op_errno = -ret; + goto out; + } ret = 0; out: @@ -538,7 +542,11 @@ afr_mknod (call_frame_t *frame, xlator_t *this, loc_t *loc, mode_t mode, local->transaction.main_frame = frame; local->transaction.basename = AFR_BASENAME (loc->path); - afr_transaction (transaction_frame, this, AFR_ENTRY_TRANSACTION); + ret = afr_transaction (transaction_frame, this, AFR_ENTRY_TRANSACTION); + if (ret < 0) { + op_errno = -ret; + goto out; + } ret = 0; out: @@ -727,7 +735,11 @@ afr_mkdir (call_frame_t *frame, xlator_t *this, local->transaction.main_frame = frame; local->transaction.basename = AFR_BASENAME (loc->path); - afr_transaction (transaction_frame, this, AFR_ENTRY_TRANSACTION); + ret = afr_transaction (transaction_frame, this, AFR_ENTRY_TRANSACTION); + if (ret < 0) { + op_errno = -ret; + goto out; + } ret = 0; out: @@ -912,7 +924,11 @@ afr_link (call_frame_t *frame, xlator_t *this, local->transaction.main_frame = frame; local->transaction.basename = AFR_BASENAME (newloc->path); - afr_transaction (transaction_frame, this, AFR_ENTRY_TRANSACTION); + ret = afr_transaction (transaction_frame, this, AFR_ENTRY_TRANSACTION); + if (ret < 0) { + op_errno = -ret; + goto out; + } ret = 0; out: @@ -1102,7 +1118,11 @@ afr_symlink (call_frame_t *frame, xlator_t *this, local->transaction.main_frame = frame; local->transaction.basename = AFR_BASENAME (loc->path); - afr_transaction (transaction_frame, this, AFR_ENTRY_TRANSACTION); + ret = afr_transaction (transaction_frame, this, AFR_ENTRY_TRANSACTION); + if (ret < 0) { + op_errno = -ret; + goto out; + } ret = 0; out: @@ -1295,7 +1315,11 @@ afr_rename (call_frame_t *frame, xlator_t *this, local->transaction.basename = AFR_BASENAME (oldloc->path); local->transaction.new_basename = AFR_BASENAME (newloc->path); - afr_transaction (transaction_frame, this, AFR_ENTRY_RENAME_TRANSACTION); + ret = afr_transaction (transaction_frame, this, AFR_ENTRY_RENAME_TRANSACTION); + if (ret < 0) { + op_errno = -ret; + goto out; + } ret = 0; out: @@ -1473,7 +1497,11 @@ afr_unlink (call_frame_t *frame, xlator_t *this, local->transaction.main_frame = frame; local->transaction.basename = AFR_BASENAME (loc->path); - afr_transaction (transaction_frame, this, AFR_ENTRY_TRANSACTION); + ret = afr_transaction (transaction_frame, this, AFR_ENTRY_TRANSACTION); + if (ret < 0) { + op_errno = -ret; + goto out; + } ret = 0; out: @@ -1657,7 +1685,11 @@ afr_rmdir (call_frame_t *frame, xlator_t *this, local->transaction.main_frame = frame; local->transaction.basename = AFR_BASENAME (loc->path); - afr_transaction (transaction_frame, this, AFR_ENTRY_TRANSACTION); + ret = afr_transaction (transaction_frame, this, AFR_ENTRY_TRANSACTION); + if (ret < 0) { + op_errno = -ret; + goto out; + } ret = 0; out: diff --git a/xlators/cluster/afr/src/afr-inode-write.c b/xlators/cluster/afr/src/afr-inode-write.c index 148a939ca..4c5502972 100644 --- a/xlators/cluster/afr/src/afr-inode-write.c +++ b/xlators/cluster/afr/src/afr-inode-write.c @@ -217,11 +217,15 @@ afr_do_writev (call_frame_t *frame, xlator_t *this) local->cont.writev.count); } - afr_transaction (transaction_frame, this, AFR_DATA_TRANSACTION); + op_ret = afr_transaction (transaction_frame, this, AFR_DATA_TRANSACTION); + if (op_ret < 0) { + op_errno = -op_ret; + goto out; + } op_ret = 0; out: - if (op_ret == -1) { + if (op_ret < 0) { if (transaction_frame) AFR_STACK_DESTROY (transaction_frame); AFR_STACK_UNWIND (writev, frame, op_ret, op_errno, NULL, NULL, NULL); @@ -676,7 +680,11 @@ afr_truncate (call_frame_t *frame, xlator_t *this, local->transaction.start = offset; local->transaction.len = 0; - afr_transaction (transaction_frame, this, AFR_DATA_TRANSACTION); + ret = afr_transaction (transaction_frame, this, AFR_DATA_TRANSACTION); + if (ret < 0) { + op_errno = -ret; + goto out; + } ret = 0; out: @@ -868,11 +876,15 @@ afr_do_ftruncate (call_frame_t *frame, xlator_t *this) local->transaction.start = local->cont.ftruncate.offset; local->transaction.len = 0; - afr_transaction (transaction_frame, this, AFR_DATA_TRANSACTION); + op_ret = afr_transaction (transaction_frame, this, AFR_DATA_TRANSACTION); + if (op_ret < 0) { + op_errno = -op_ret; + goto out; + } op_ret = 0; out: - if (op_ret == -1) { + if (op_ret < 0) { if (transaction_frame) AFR_STACK_DESTROY (transaction_frame); AFR_STACK_UNWIND (ftruncate, frame, op_ret, op_errno, NULL, @@ -1124,7 +1136,11 @@ afr_setattr (call_frame_t *frame, xlator_t *this, local->transaction.start = LLONG_MAX - 1; local->transaction.len = 0; - afr_transaction (transaction_frame, this, AFR_METADATA_TRANSACTION); + ret = afr_transaction (transaction_frame, this, AFR_METADATA_TRANSACTION); + if (ret < 0) { + op_errno = -ret; + goto out; + } ret = 0; out: @@ -1334,7 +1350,11 @@ afr_fsetattr (call_frame_t *frame, xlator_t *this, local->transaction.start = LLONG_MAX - 1; local->transaction.len = 0; - afr_transaction (transaction_frame, this, AFR_METADATA_TRANSACTION); + ret = afr_transaction (transaction_frame, this, AFR_METADATA_TRANSACTION); + if (ret < 0) { + op_errno = -ret; + goto out; + } ret = 0; out: @@ -1521,7 +1541,11 @@ afr_setxattr (call_frame_t *frame, xlator_t *this, local->transaction.start = LLONG_MAX - 1; local->transaction.len = 0; - afr_transaction (transaction_frame, this, AFR_METADATA_TRANSACTION); + ret = afr_transaction (transaction_frame, this, AFR_METADATA_TRANSACTION); + if (ret < 0) { + op_errno = -ret; + goto out; + } ret = 0; out: @@ -1712,7 +1736,11 @@ afr_fsetxattr (call_frame_t *frame, xlator_t *this, local->transaction.start = LLONG_MAX - 1; local->transaction.len = 0; - afr_transaction (transaction_frame, this, AFR_METADATA_TRANSACTION); + ret = afr_transaction (transaction_frame, this, AFR_METADATA_TRANSACTION); + if (ret < 0) { + op_errno = -ret; + goto out; + } ret = 0; out: @@ -1902,7 +1930,11 @@ afr_removexattr (call_frame_t *frame, xlator_t *this, local->transaction.start = LLONG_MAX - 1; local->transaction.len = 0; - afr_transaction (transaction_frame, this, AFR_METADATA_TRANSACTION); + ret = afr_transaction (transaction_frame, this, AFR_METADATA_TRANSACTION); + if (ret < 0) { + op_errno = -ret; + goto out; + } ret = 0; out: @@ -2091,11 +2123,15 @@ afr_fremovexattr (call_frame_t *frame, xlator_t *this, local->transaction.start = LLONG_MAX - 1; local->transaction.len = 0; - afr_transaction (transaction_frame, this, AFR_METADATA_TRANSACTION); + op_ret = afr_transaction (transaction_frame, this, AFR_METADATA_TRANSACTION); + if (op_ret < 0) { + op_errno = -op_ret; + goto out; + } op_ret = 0; out: - if (op_ret == -1) { + if (op_ret < 0) { if (transaction_frame) AFR_STACK_DESTROY (transaction_frame); AFR_STACK_UNWIND (fremovexattr, frame, op_ret, op_errno, NULL); diff --git a/xlators/cluster/afr/src/afr-transaction.c b/xlators/cluster/afr/src/afr-transaction.c index be4a914d4..265d93b61 100644 --- a/xlators/cluster/afr/src/afr-transaction.c +++ b/xlators/cluster/afr/src/afr-transaction.c @@ -1452,21 +1452,25 @@ afr_transaction (call_frame_t *frame, xlator_t *this, afr_transaction_type type) afr_local_t * local = NULL; afr_private_t * priv = NULL; fd_t *fd = NULL; + int ret = -1; local = frame->local; priv = this->private; - afr_transaction_local_init (local, this); + if (local->fd && priv->eager_lock && + local->transaction.type == AFR_DATA_TRANSACTION) + afr_set_lk_owner (frame, this, local->fd); + else + afr_set_lk_owner (frame, this, frame->root); + + ret = afr_transaction_local_init (local, this); + if (ret < 0) { + goto out; + } local->transaction.resume = afr_transaction_resume; local->transaction.type = type; - if (local->fd && priv->eager_lock && - local->transaction.type == AFR_DATA_TRANSACTION) - afr_set_lk_owner (frame, this, local->fd); - else - afr_set_lk_owner (frame, this, frame->root); - if (_does_transaction_conflict_with_delayed_post_op (frame) && local->loc.inode) { fd = fd_lookup (local->loc.inode, frame->root->pid); @@ -1481,6 +1485,7 @@ afr_transaction (call_frame_t *frame, xlator_t *this, afr_transaction_type type) } else { afr_lock (frame, this); } - - return 0; + ret = 0; +out: + return ret; } -- cgit