diff options
author | N Balachandran <nbalacha@redhat.com> | 2018-01-19 11:42:46 +0530 |
---|---|---|
committer | jiffin tony Thottan <jthottan@redhat.com> | 2018-02-06 07:06:47 +0000 |
commit | e64cc2afe0f8111cf14ebe5a2d6a7c6e70af764b (patch) | |
tree | f145d71533133ba093e1ed34b6b8e7d54743c98b /xlators/cluster/dht/src/dht-rebalance.c | |
parent | 4389b4fd93b85814c950aeafbeda59538b70414d (diff) |
cluster/dht: Add migration checks to dht_(f)xattrop
The dht_(f)xattrop implementation did not implement
migration phase1/phase2 checks which could cause issues
with rebalance on sharded volumes.
This does not solve the issue where fops may reach the target
out of order.
> Change-Id: I2416fc35115e60659e35b4b717fd51f20746586c
> BUG: 1471031
> Signed-off-by: N Balachandran <nbalacha@redhat.com>
Change-Id: I2416fc35115e60659e35b4b717fd51f20746586c
BUG: 1540224
Signed-off-by: N Balachandran <nbalacha@redhat.com>
Diffstat (limited to 'xlators/cluster/dht/src/dht-rebalance.c')
-rw-r--r-- | xlators/cluster/dht/src/dht-rebalance.c | 76 |
1 files changed, 45 insertions, 31 deletions
diff --git a/xlators/cluster/dht/src/dht-rebalance.c b/xlators/cluster/dht/src/dht-rebalance.c index d0945650c9e..bed57ac47cd 100644 --- a/xlators/cluster/dht/src/dht-rebalance.c +++ b/xlators/cluster/dht/src/dht-rebalance.c @@ -17,6 +17,7 @@ #include <fnmatch.h> #include <signal.h> #include "events.h" +#include "glusterfs-acl.h" #define GF_DISK_SECTOR_SIZE 512 #define DHT_REBALANCE_PID 4242 /* Change it if required */ @@ -168,12 +169,11 @@ dht_strip_out_acls (dict_t *dict) { if (dict) { dict_del (dict, "trusted.SGI_ACL_FILE"); - dict_del (dict, "POSIX_ACL_ACCESS_XATTR"); + dict_del (dict, POSIX_ACL_ACCESS_XATTR); } } - static int dht_write_with_holes (xlator_t *to, fd_t *fd, struct iovec *vec, int count, int32_t size, off_t offset, struct iobref *iobref, @@ -665,7 +665,7 @@ out: static int __dht_rebalance_create_dst_file (xlator_t *this, xlator_t *to, xlator_t *from, loc_t *loc, struct iatt *stbuf, fd_t **dst_fd, - dict_t *xattr, int *fop_errno) + int *fop_errno) { int ret = -1; fd_t *fd = NULL; @@ -810,16 +810,6 @@ __dht_rebalance_create_dst_file (xlator_t *this, xlator_t *to, xlator_t *from, goto out; } - ret = syncop_fsetxattr (to, fd, xattr, 0, NULL, NULL); - if (ret < 0) { - *fop_errno = -ret; - gf_msg (this->name, GF_LOG_WARNING, -ret, - DHT_MSG_MIGRATE_FILE_FAILED, - "%s: failed to set xattr on %s", - loc->path, to->name); - - } - ret = syncop_fsetattr (to, fd, stbuf, (GF_SET_ATTR_UID | GF_SET_ATTR_GID), NULL, NULL, NULL, NULL); @@ -1663,24 +1653,9 @@ dht_migrate_file (xlator_t *this, loc_t *loc, xlator_t *from, xlator_t *to, } - /* TODO: move all xattr related operations to fd based operations */ - ret = syncop_listxattr (from, loc, &xattr, NULL, NULL); - if (ret < 0) { - *fop_errno = -ret; - ret = -1; - gf_msg (this->name, GF_LOG_WARNING, *fop_errno, - DHT_MSG_MIGRATE_FILE_FAILED, - "Migrate file failed:" - "%s: failed to get xattr from %s", - loc->path, from->name); - } - - /* Copying posix acls to the linkto file messes up the permissions*/ - dht_strip_out_acls (xattr); - /* create the destination, with required modes/xattr */ ret = __dht_rebalance_create_dst_file (this, to, from, loc, &stbuf, - &dst_fd, xattr, fop_errno); + &dst_fd, fop_errno); if (ret) { gf_msg (this->name, GF_LOG_ERROR, 0, 0, "Create dst failed" " on - %s for file - %s", to->name, loc->path); @@ -1726,7 +1701,7 @@ dht_migrate_file (xlator_t *this, loc_t *loc, xlator_t *from, xlator_t *to, * as in case of failure the linkto needs to point to the source * subvol */ ret = __dht_rebalance_create_dst_file (this, to, from, loc, &stbuf, - &dst_fd, xattr, fop_errno); + &dst_fd, fop_errno); if (ret) { gf_log (this->name, GF_LOG_ERROR, "Create dst failed" " on - %s for file - %s", to->name, loc->path); @@ -1752,6 +1727,43 @@ dht_migrate_file (xlator_t *this, loc_t *loc, xlator_t *from, xlator_t *to, loc->path, from->name); goto out; } + + /* TODO: move all xattr related operations to fd based operations */ + ret = syncop_listxattr (from, loc, &xattr, NULL, NULL); + if (ret < 0) { + *fop_errno = -ret; + gf_msg (this->name, GF_LOG_WARNING, *fop_errno, + DHT_MSG_MIGRATE_FILE_FAILED, + "Migrate file failed:" + "%s: failed to get xattr from %s", + loc->path, from->name); + ret = -1; + goto out; + } + + /* Copying posix acls to the linkto file messes up the permissions*/ + dht_strip_out_acls (xattr); + + /* Remove the linkto xattr as we don't want to overwrite the value + * set on the dst. + */ + dict_del (xattr, conf->link_xattr_name); + + /* We need to error out if this fails as having the wrong shard xattrs + * set on the dst could cause data corruption + */ + ret = syncop_fsetxattr (to, dst_fd, xattr, 0, NULL, NULL); + if (ret < 0) { + *fop_errno = -ret; + gf_msg (this->name, GF_LOG_WARNING, -ret, + DHT_MSG_MIGRATE_FILE_FAILED, + "%s: failed to set xattr on %s", + loc->path, to->name); + ret = -1; + goto out; + } + + if (xattr_rsp) { /* we no more require this key */ dict_del (dict, conf->link_xattr_name); @@ -2054,7 +2066,9 @@ dht_migrate_file (xlator_t *this, loc_t *loc, xlator_t *from, xlator_t *to, xattr = NULL; } - ret = syncop_listxattr (from, loc, &xattr, NULL, NULL); + /* Set only the Posix ACLs this time */ + ret = syncop_getxattr (from, loc, &xattr, POSIX_ACL_ACCESS_XATTR, + NULL, NULL); if (ret < 0) { gf_msg (this->name, GF_LOG_WARNING, -ret, DHT_MSG_MIGRATE_FILE_FAILED, |