|
 |
887953 |
From f42b8789cdcd93cb9fa93f35ed067268ce75f789 Mon Sep 17 00:00:00 2001
|
|
|
887953 |
From: Kotresh HR <khiremat@redhat.com>
|
|
|
887953 |
Date: Thu, 25 Oct 2018 03:23:56 -0400
|
|
|
887953 |
Subject: [PATCH 436/444] geo-rep: Fix issue in gfid-conflict-resolution
|
|
|
887953 |
|
|
|
887953 |
Problem:
|
|
|
887953 |
During gfid-conflict-resolution, geo-rep crashes
|
|
|
887953 |
with 'ValueError: list.remove(x): x not in list'
|
|
|
887953 |
|
|
|
887953 |
Cause and Analysis:
|
|
|
887953 |
During gfid-conflict-resolution, the entry blob is
|
|
|
887953 |
passed back to master along with additional
|
|
|
887953 |
information to verify it's integrity. If everything
|
|
|
887953 |
looks fine, the entry creation is ignored and is
|
|
|
887953 |
deleted from the original list. But it is crashing
|
|
|
887953 |
during removal of entry from the list saying entry
|
|
|
887953 |
not in list. The reason is that the stat information
|
|
|
887953 |
in the entry blob was modified and sent back to
|
|
|
887953 |
master if present.
|
|
|
887953 |
|
|
|
887953 |
Fix:
|
|
|
887953 |
Send back the correct stat information for
|
|
|
887953 |
gfid-conflict-resolution.
|
|
|
887953 |
|
|
|
887953 |
Backport of:
|
|
|
887953 |
> Patch: https://review.gluster.org/21483
|
|
|
887953 |
> fixes: bz#1642865
|
|
|
887953 |
> Change-Id: I47a6aa60b2a495465aa9314eebcb4085f0b1c4fd
|
|
|
887953 |
> Signed-off-by: Kotresh HR <khiremat@redhat.com>
|
|
|
887953 |
|
|
|
887953 |
BUG: 1640347
|
|
|
887953 |
Change-Id: I47a6aa60b2a495465aa9314eebcb4085f0b1c4fd
|
|
|
887953 |
Signed-off-by: Kotresh HR <khiremat@redhat.com>
|
|
|
887953 |
Reviewed-on: https://code.engineering.redhat.com/gerrit/155038
|
|
|
887953 |
Tested-by: RHGS Build Bot <nigelb@redhat.com>
|
|
|
887953 |
Reviewed-by: Aravinda Vishwanathapura Krishna Murthy <avishwan@redhat.com>
|
|
|
887953 |
Reviewed-by: Sunny Kumar <sunkumar@redhat.com>
|
|
|
887953 |
Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>
|
|
|
887953 |
---
|
|
|
887953 |
geo-replication/syncdaemon/resource.py | 42 +++++++++++++++++++---------------
|
|
|
887953 |
1 file changed, 24 insertions(+), 18 deletions(-)
|
|
|
887953 |
|
|
|
887953 |
diff --git a/geo-replication/syncdaemon/resource.py b/geo-replication/syncdaemon/resource.py
|
|
|
887953 |
index b289b3b..f16066e 100644
|
|
|
887953 |
--- a/geo-replication/syncdaemon/resource.py
|
|
|
887953 |
+++ b/geo-replication/syncdaemon/resource.py
|
|
|
887953 |
@@ -456,7 +456,7 @@ class Server(object):
|
|
|
887953 |
st['uid'], st['gid'],
|
|
|
887953 |
gf, st['mode'], bn, lnk)
|
|
|
887953 |
|
|
|
887953 |
- def entry_purge(op, entry, gfid, e):
|
|
|
887953 |
+ def entry_purge(op, entry, gfid, e, uid, gid):
|
|
|
887953 |
# This is an extremely racy code and needs to be fixed ASAP.
|
|
|
887953 |
# The GFID check here is to be sure that the pargfid/bname
|
|
|
887953 |
# to be purged is the GFID gotten from the changelog.
|
|
|
887953 |
@@ -470,7 +470,7 @@ class Server(object):
|
|
|
887953 |
return
|
|
|
887953 |
|
|
|
887953 |
if not matching_disk_gfid(gfid, entry):
|
|
|
887953 |
- collect_failure(e, EEXIST)
|
|
|
887953 |
+ collect_failure(e, EEXIST, uid, gid)
|
|
|
887953 |
return
|
|
|
887953 |
|
|
|
887953 |
if op == 'UNLINK':
|
|
|
887953 |
@@ -486,7 +486,7 @@ class Server(object):
|
|
|
887953 |
if er == ENOTEMPTY:
|
|
|
887953 |
return er
|
|
|
887953 |
|
|
|
887953 |
- def collect_failure(e, cmd_ret, dst=False):
|
|
|
887953 |
+ def collect_failure(e, cmd_ret, uid, gid, dst=False):
|
|
|
887953 |
slv_entry_info = {}
|
|
|
887953 |
slv_entry_info['gfid_mismatch'] = False
|
|
|
887953 |
slv_entry_info['name_mismatch'] = False
|
|
|
887953 |
@@ -499,6 +499,11 @@ class Server(object):
|
|
|
887953 |
if cmd_ret is None:
|
|
|
887953 |
return False
|
|
|
887953 |
|
|
|
887953 |
+ if e.get("stat", {}):
|
|
|
887953 |
+ # Copy actual UID/GID value back to entry stat
|
|
|
887953 |
+ e['stat']['uid'] = uid
|
|
|
887953 |
+ e['stat']['gid'] = gid
|
|
|
887953 |
+
|
|
|
887953 |
if cmd_ret == EEXIST:
|
|
|
887953 |
if dst:
|
|
|
887953 |
en = e['entry1']
|
|
|
887953 |
@@ -559,7 +564,7 @@ class Server(object):
|
|
|
887953 |
|
|
|
887953 |
errno_wrap(os.rmdir, [path], [ENOENT, ESTALE], [EBUSY])
|
|
|
887953 |
|
|
|
887953 |
- def rename_with_disk_gfid_confirmation(gfid, entry, en):
|
|
|
887953 |
+ def rename_with_disk_gfid_confirmation(gfid, entry, en, uid, gid):
|
|
|
887953 |
if not matching_disk_gfid(gfid, entry):
|
|
|
887953 |
logging.error(lf("RENAME ignored: source entry does not match "
|
|
|
887953 |
"with on-disk gfid",
|
|
|
887953 |
@@ -567,14 +572,13 @@ class Server(object):
|
|
|
887953 |
gfid=gfid,
|
|
|
887953 |
disk_gfid=get_gfid_from_mnt(entry),
|
|
|
887953 |
target=en))
|
|
|
887953 |
- collect_failure(e, EEXIST)
|
|
|
887953 |
+ collect_failure(e, EEXIST, uid, gid)
|
|
|
887953 |
return
|
|
|
887953 |
|
|
|
887953 |
cmd_ret = errno_wrap(os.rename,
|
|
|
887953 |
[entry, en],
|
|
|
887953 |
[ENOENT, EEXIST], [ESTALE, EBUSY])
|
|
|
887953 |
- collect_failure(e, cmd_ret)
|
|
|
887953 |
-
|
|
|
887953 |
+ collect_failure(e, cmd_ret, uid, gid)
|
|
|
887953 |
|
|
|
887953 |
for e in entries:
|
|
|
887953 |
blob = None
|
|
|
887953 |
@@ -595,7 +599,7 @@ class Server(object):
|
|
|
887953 |
if op in ['RMDIR', 'UNLINK']:
|
|
|
887953 |
# Try once, if rmdir failed with ENOTEMPTY
|
|
|
887953 |
# then delete recursively.
|
|
|
887953 |
- er = entry_purge(op, entry, gfid, e)
|
|
|
887953 |
+ er = entry_purge(op, entry, gfid, e, uid, gid)
|
|
|
887953 |
if isinstance(er, int):
|
|
|
887953 |
if er == ENOTEMPTY and op == 'RMDIR':
|
|
|
887953 |
# Retry if ENOTEMPTY, ESTALE
|
|
|
887953 |
@@ -632,7 +636,7 @@ class Server(object):
|
|
|
887953 |
cmd_ret = errno_wrap(os.link,
|
|
|
887953 |
[slink, entry],
|
|
|
887953 |
[ENOENT, EEXIST], [ESTALE])
|
|
|
887953 |
- collect_failure(e, cmd_ret)
|
|
|
887953 |
+ collect_failure(e, cmd_ret, uid, gid)
|
|
|
887953 |
elif op == 'MKDIR':
|
|
|
887953 |
en = e['entry']
|
|
|
887953 |
slink = os.path.join(pfx, gfid)
|
|
|
887953 |
@@ -676,7 +680,7 @@ class Server(object):
|
|
|
887953 |
cmd_ret = errno_wrap(os.link,
|
|
|
887953 |
[slink, entry],
|
|
|
887953 |
[ENOENT, EEXIST], [ESTALE])
|
|
|
887953 |
- collect_failure(e, cmd_ret)
|
|
|
887953 |
+ collect_failure(e, cmd_ret, uid, gid)
|
|
|
887953 |
elif op == 'SYMLINK':
|
|
|
887953 |
en = e['entry']
|
|
|
887953 |
st = lstat(entry)
|
|
|
887953 |
@@ -684,7 +688,7 @@ class Server(object):
|
|
|
887953 |
blob = entry_pack_symlink(gfid, bname, e['link'],
|
|
|
887953 |
e['stat'])
|
|
|
887953 |
elif not matching_disk_gfid(gfid, en):
|
|
|
887953 |
- collect_failure(e, EEXIST)
|
|
|
887953 |
+ collect_failure(e, EEXIST, uid, gid)
|
|
|
887953 |
elif op == 'RENAME':
|
|
|
887953 |
en = e['entry1']
|
|
|
887953 |
# The matching disk gfid check validates two things
|
|
|
887953 |
@@ -704,7 +708,7 @@ class Server(object):
|
|
|
887953 |
blob = entry_pack_symlink(gfid, bname,
|
|
|
887953 |
e['link'], e['stat'])
|
|
|
887953 |
elif not matching_disk_gfid(gfid, en):
|
|
|
887953 |
- collect_failure(e, EEXIST, True)
|
|
|
887953 |
+ collect_failure(e, EEXIST, uid, gid, True)
|
|
|
887953 |
else:
|
|
|
887953 |
slink = os.path.join(pfx, gfid)
|
|
|
887953 |
st = lstat(slink)
|
|
|
887953 |
@@ -716,12 +720,13 @@ class Server(object):
|
|
|
887953 |
else:
|
|
|
887953 |
cmd_ret = errno_wrap(os.link, [slink, en],
|
|
|
887953 |
[ENOENT, EEXIST], [ESTALE])
|
|
|
887953 |
- collect_failure(e, cmd_ret)
|
|
|
887953 |
+ collect_failure(e, cmd_ret, uid, gid)
|
|
|
887953 |
else:
|
|
|
887953 |
st = lstat(entry)
|
|
|
887953 |
st1 = lstat(en)
|
|
|
887953 |
if isinstance(st1, int):
|
|
|
887953 |
- rename_with_disk_gfid_confirmation(gfid, entry, en)
|
|
|
887953 |
+ rename_with_disk_gfid_confirmation(gfid, entry, en,
|
|
|
887953 |
+ uid, gid)
|
|
|
887953 |
else:
|
|
|
887953 |
if st.st_ino == st1.st_ino:
|
|
|
887953 |
# we have a hard link, we can now unlink source
|
|
|
887953 |
@@ -746,15 +751,16 @@ class Server(object):
|
|
|
887953 |
else:
|
|
|
887953 |
raise
|
|
|
887953 |
elif not matching_disk_gfid(gfid, en):
|
|
|
887953 |
- collect_failure(e, EEXIST, True)
|
|
|
887953 |
+ collect_failure(e, EEXIST, uid, gid, True)
|
|
|
887953 |
else:
|
|
|
887953 |
- rename_with_disk_gfid_confirmation(gfid, entry, en)
|
|
|
887953 |
+ rename_with_disk_gfid_confirmation(gfid, entry, en,
|
|
|
887953 |
+ uid, gid)
|
|
|
887953 |
if blob:
|
|
|
887953 |
cmd_ret = errno_wrap(Xattr.lsetxattr,
|
|
|
887953 |
[pg, 'glusterfs.gfid.newfile', blob],
|
|
|
887953 |
[EEXIST, ENOENT],
|
|
|
887953 |
[ESTALE, EINVAL, EBUSY])
|
|
|
887953 |
- failed = collect_failure(e, cmd_ret)
|
|
|
887953 |
+ collect_failure(e, cmd_ret, uid, gid)
|
|
|
887953 |
|
|
|
887953 |
# If UID/GID is different than zero that means we are trying
|
|
|
887953 |
# create Entry with different UID/GID. Create Entry with
|
|
|
887953 |
@@ -763,7 +769,7 @@ class Server(object):
|
|
|
887953 |
path = os.path.join(pfx, gfid)
|
|
|
887953 |
cmd_ret = errno_wrap(os.lchown, [path, uid, gid], [ENOENT],
|
|
|
887953 |
[ESTALE, EINVAL])
|
|
|
887953 |
- collect_failure(e, cmd_ret)
|
|
|
887953 |
+ collect_failure(e, cmd_ret, uid, gid)
|
|
|
887953 |
|
|
|
887953 |
return failures
|
|
|
887953 |
|
|
|
887953 |
--
|
|
|
887953 |
1.8.3.1
|
|
|
887953 |
|