From 430ad405294993ebb16387232281cc5a4f854c75 Mon Sep 17 00:00:00 2001 From: N Balachandran Date: Wed, 16 Dec 2015 21:09:22 +0530 Subject: cluster/dht : Ftruncate on migrating file fails with EINVAL What: If dht_open is called on a migrating file after the inode_ctx is set, subsequent FOPs on that fd do not open the fd on the dst subvol. This is seen when the open-ftruncate-close sequence is repeatedly called on a migrating file. A second call to the sequence described above causes dht_truncate_cbk to call dht_truncate2 as the dht_inode_ctx was already set by the first call. As dht_rebalance_in_progress_check is not called, the fd is not opened on the dst subvol. On a distributed-replicate volume, this causes AFR to open the fd using afr_fix_open, but with the wrong flags, causing posix_ftruncate to fail with EINVAL. The fix: We require fd specific information to make a decision while handling migrating files. Set the fd_ctx to indicate the fd has been opened on the dst subvol and check if it has been set while processing Phase1/Phase2 checks in the FOP callback functions. Change-Id: I43cdcd8017b4a11e18afdd210469de7cd9a5ef14 BUG: 1284823 Signed-off-by: N Balachandran Reviewed-on: http://review.gluster.org/12985 Reviewed-by: Raghavendra G Tested-by: Gluster Build System Reviewed-by: Dan Lambright Tested-by: Dan Lambright --- xlators/cluster/dht/src/dht-inode-read.c | 82 +++++++++++++++++++------------- 1 file changed, 49 insertions(+), 33 deletions(-) (limited to 'xlators/cluster/dht/src/dht-inode-read.c') diff --git a/xlators/cluster/dht/src/dht-inode-read.c b/xlators/cluster/dht/src/dht-inode-read.c index ad284ae4be8..d40ac7d4c61 100644 --- a/xlators/cluster/dht/src/dht-inode-read.c +++ b/xlators/cluster/dht/src/dht-inode-read.c @@ -142,7 +142,8 @@ int dht_file_attr_cbk (call_frame_t *frame, void *cookie, xlator_t *this, int op_ret, int op_errno, struct iatt *stbuf, dict_t *xdata) { - xlator_t *subvol = 0; + xlator_t *subvol1 = 0; + xlator_t *subvol2 = 0; dht_local_t *local = NULL; call_frame_t *prev = NULL; int ret = -1; @@ -172,21 +173,31 @@ dht_file_attr_cbk (call_frame_t *frame, void *cookie, xlator_t *this, /* Check if the rebalance phase2 is true */ if ((op_ret == -1) || IS_DHT_MIGRATION_PHASE2 (stbuf)) { + + local->rebalance.target_op_fn = dht_attr2; + dht_set_local_rebalance (this, local, NULL, NULL, + stbuf, xdata); inode = (local->fd) ? local->fd->inode : local->loc.inode; - ret = dht_inode_ctx_get_mig_info (this, inode, NULL, &subvol); - if (!subvol) { + + dht_inode_ctx_get_mig_info (this, inode, &subvol1, &subvol2); + if (dht_mig_info_is_invalid (local->cached_subvol, + subvol1, subvol2)){ /* Phase 2 of migration */ - local->rebalance.target_op_fn = dht_attr2; - dht_set_local_rebalance (this, local, NULL, NULL, - stbuf, xdata); ret = dht_rebalance_complete_check (this, frame); if (!ret) return 0; } else { - /* value is already set in fd_ctx, that means no need - to check for whether its complete or not. */ - dht_attr2 (this, subvol, frame, 0); - return 0; + /* it is a non-fd op or it is an fd based Fop and + opened on the dst.*/ + if (local->fd && + !dht_fd_open_on_dst (this, local->fd, subvol2)) { + ret = dht_rebalance_complete_check (this, frame); + if (!ret) + return 0; + } else { + dht_attr2 (this, subvol2, frame, 0); + return 0; + } } } @@ -431,17 +442,19 @@ dht_readv_cbk (call_frame_t *frame, void *cookie, xlator_t *this, local->op_errno = op_errno; if ((op_ret == -1) || IS_DHT_MIGRATION_PHASE2 (stbuf)) { + + local->op_ret = op_ret; + local->rebalance.target_op_fn = dht_readv2; + dht_set_local_rebalance (this, local, NULL, NULL, + stbuf, xdata); /* File would be migrated to other node */ ret = dht_inode_ctx_get_mig_info (this, local->fd->inode, &src_subvol, &dst_subvol); if (dht_mig_info_is_invalid (local->cached_subvol, - src_subvol, dst_subvol)) { - local->op_ret = op_ret; - local->rebalance.target_op_fn = dht_readv2; - dht_set_local_rebalance (this, local, NULL, NULL, - stbuf, xdata); + src_subvol, dst_subvol) + || !dht_fd_open_on_dst(this, local->fd, dst_subvol)) { ret = dht_rebalance_complete_check (this, frame); if (!ret) @@ -691,7 +704,7 @@ dht_flush_cbk (call_frame_t *frame, void *cookie, xlator_t *this, /* If context is set, then send flush() it to the destination */ dht_inode_ctx_get_mig_info (this, local->fd->inode, NULL, &subvol); - if (subvol) { + if (subvol && dht_fd_open_on_dst (this, local->fd, subvol)) { dht_flush2 (this, subvol, frame, 0); return 0; } @@ -805,32 +818,35 @@ dht_fsync_cbk (call_frame_t *frame, void *cookie, xlator_t *this, int op_ret, local->op_ret = op_ret; inode = local->fd->inode; - dht_inode_ctx_get_mig_info (this, inode, &src_subvol, &dst_subvol); + local->rebalance.target_op_fn = dht_fsync2; + dht_set_local_rebalance (this, local, NULL, prebuf, + postbuf, xdata); + + /* Check if the rebalance phase1 is true */ + if (IS_DHT_MIGRATION_PHASE1 (postbuf)) { - if (dht_mig_info_is_invalid (local->cached_subvol, - src_subvol, dst_subvol)) { + dht_iatt_merge (this, &local->stbuf, postbuf, NULL); + dht_iatt_merge (this, &local->prebuf, prebuf, NULL); - local->rebalance.target_op_fn = dht_fsync2; - dht_set_local_rebalance (this, local, NULL, prebuf, - postbuf, xdata); + dht_inode_ctx_get_mig_info (this, inode, &src_subvol, &dst_subvol); - /* Check if the rebalance phase1 is true */ - if (IS_DHT_MIGRATION_PHASE1 (postbuf)) { - dht_iatt_merge (this, &local->stbuf, postbuf, NULL); - dht_iatt_merge (this, &local->prebuf, prebuf, NULL); + if (dht_mig_info_is_invalid (local->cached_subvol, src_subvol, + dst_subvol) || + !dht_fd_open_on_dst (this, local->fd, dst_subvol)) { ret = dht_rebalance_in_progress_check (this, frame); + if (!ret) + return 0; + } else { + dht_fsync2 (this, dst_subvol, frame, 0); + return 0; } + } - /* Check if the rebalance phase2 is true */ - if (IS_DHT_MIGRATION_PHASE2 (postbuf)) { - ret = dht_rebalance_complete_check (this, frame); - } + if (IS_DHT_MIGRATION_PHASE2 (postbuf)) { + ret = dht_rebalance_complete_check (this, frame); if (!ret) return 0; - } else { - dht_fsync2 (this, dst_subvol, frame, 0); - return 0; } out: -- cgit