Bug 1234610

I suspect that the issue is a side effect of xlators/cluster/dht/src/dht-rebalance.c , function __dht_rebalance_create_dst_file :

static inline int
__dht_rebalance_create_dst_file (xlator_t *to, xlator_t *from, loc_t *loc, struct iatt *stbuf,
                                 dict_t *dict, fd_t **dst_fd, dict_t *xattr)
{
[...]
        /* TODO: move all xattr related operations to fd based operations */
        ret = syncop_listxattr (from, loc, &xattr);
        if (ret < 0) {
                gf_msg (this->name, GF_LOG_WARNING, 0,
                        DHT_MSG_MIGRATE_FILE_FAILED,
                        "Migrate file failed:"
                        "%s: failed to get xattr from %s (%s)",
                        loc->path, from->name, strerror (-ret));
                ret = -1;
        }

        /* create the destination, with required modes/xattr */
        ret = __dht_rebalance_create_dst_file (to, from, loc, &stbuf,
                                               dict, &dst_fd, xattr);
        if (ret)
                goto out;

        ret = __dht_check_free_space (to, from, loc, &stbuf, flag);
        if (ret) {
                goto out;
        }


Where the xattr are copied over (syncop_listxattr(); __dht_rebalance_create_dst_file()) but then we might cancel the migration if __dht_check_free_space() returns a failure ... leading to xattr having been modified on the destination.

or maybe we want to change the behavior or readdir/getdents ?

(In reply to Cedric Buissart from comment #2)
> I suspect that the issue is a side effect of
> xlators/cluster/dht/src/dht-rebalance.c , function
> __dht_rebalance_create_dst_file :
> 
I think the function which Cedric wanted to point is :
int
dht_migrate_file (xlator_t *this, loc_t *loc, xlator_t *from, xlator_t *to,
                  int flag)
> {
> [...]
>         /* TODO: move all xattr related operations to fd based operations */
>         ret = syncop_listxattr (from, loc, &xattr);
>         if (ret < 0) {
>                 gf_msg (this->name, GF_LOG_WARNING, 0,
>                         DHT_MSG_MIGRATE_FILE_FAILED,
>                         "Migrate file failed:"
>                         "%s: failed to get xattr from %s (%s)",
>                         loc->path, from->name, strerror (-ret));
>                 ret = -1;
>         }
> 
>         /* create the destination, with required modes/xattr */
>         ret = __dht_rebalance_create_dst_file (to, from, loc, &stbuf,
>                                                dict, &dst_fd, xattr);
>         if (ret)
>                 goto out;
> 
>         ret = __dht_check_free_space (to, from, loc, &stbuf, flag);
>         if (ret) {
>                 goto out;
>         }
> 
> 
> Where the xattr are copied over (syncop_listxattr();
> __dht_rebalance_create_dst_file()) but then we might cancel the migration if
> __dht_check_free_space() returns a failure ... leading to xattr having been
> modified on the destination.
> 
> or maybe we want to change the behavior or readdir/getdents ?

I think we should be calling function __dht_check_free_space() at least prior to __dht_rebalance_create_dst_file else we should have proper cleanup activity in the "out" section.
 

Thanks,
Bipin Kunal

(In reply to Bipin Kunal from comment #3)
> 
> I think we should be calling function __dht_check_free_space() at least
> prior to __dht_rebalance_create_dst_file else we should have proper cleanup
> activity in the "out" section.
>  
> 
> Thanks,
> Bipin Kunal

Yes, I was thinking the same. After a quick try, it seems to work :
I have the "data movement attempted from node [..] with higher disk space to a node" in the logs, but this time, did not lead to dups in the ls output.

However, additionally : should we also look at the readdir behaviour and check why the ACLs on the linkto file leads to showing dups ?

Doc text is edited. Please sign off to be included in Known Issues.

Tested with build " glusterfs-libs-3.7.1-14" and ran following tests and all tests passed so marking this as a verified 

1. Migration of big file which has posix ACLS and while migration is going on do IO from existing FD and mount it on another client and do FOP 
2. Migration of Big file and while migration is going on do IO from existing FD and make sure destination brick is down (after transfer of first half modify content in the first half)
3. Migration of Big file and while migration is going do IO from new client and while IO is going on make sure destination brick is down (Modify both already transferred data and new data)
4. While migration of big file is going on change the ACLs (modify and delete few)
5. while migration of big file which has ACL change the ACL (modify and delete few)
6. Abort migration of big file and check the clean up 
7. Abort the migration of big file which has ACLs
8. Make sure that by renaming the file it has both link and actual file and then change the ACL of the re-named file

Please review and sign-off the edited doc text.

Made some minor edits.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://rhn.redhat.com/errata/RHSA-2015-1845.html