From 7e7ffc4cc56b6b6ed460a49344082c3c25c1a23d Mon Sep 17 00:00:00 2001 From: Kotresh HR Date: Mon, 5 Nov 2018 11:46:41 +0530 Subject: [PATCH 435/444] geo-rep: Fix traceback with symlink metadata sync While syncing metadata, 'os.chmod', 'os.chown', 'os.utime' should be used without de-reference. But python supports only 'os.chown' without de-reference. That's mostly because Linux doesn't support 'chmod' on symlink file itself but it does support 'chown'. So while syncing metadata ops, if it's symlink we should only sync 'chown' and not do 'chmod' and 'utime'. It will lead to tracebacks with errors like EROFS, EPERM, ACCESS, ENOENT. All the three errors (EPERM, ACCESS, ENOENT) were handled except EROFS. But the way it was handled was not fool proof. The operation is tried and failure was handled based on the errors. All the errors with symlink file for 'chown', 'utime' had to be passed to safe errors list of 'errno_wrap'. This patch handles it better by avoiding 'chmod' and 'utime' if it's symlink file. Backport of: > Patch: https://review.gluster.org/21546 > fixes: bz#1646104 > Change-Id: Ic354206455cdc7ab2a87d741d81f4efe1f19d77d > Signed-off-by: Kotresh HR BUG: 1645916 Change-Id: Ic354206455cdc7ab2a87d741d81f4efe1f19d77d Signed-off-by: Kotresh HR Reviewed-on: https://code.engineering.redhat.com/gerrit/155049 Tested-by: RHGS Build Bot Reviewed-by: Aravinda Vishwanathapura Krishna Murthy Reviewed-by: Sunny Kumar Reviewed-by: Sunil Kumar Heggodu Gopala Acharya --- geo-replication/syncdaemon/resource.py | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/geo-replication/syncdaemon/resource.py b/geo-replication/syncdaemon/resource.py index eb696f3..b289b3b 100644 --- a/geo-replication/syncdaemon/resource.py +++ b/geo-replication/syncdaemon/resource.py @@ -790,10 +790,8 @@ class Server(object): # 'lchown' 'lchmod' 'utime with no-deference' blindly. # But since 'lchmod' and 'utime with no de-reference' is # not supported in python3, we have to rely on 'chmod' - # and 'utime with de-reference'. But 'chmod' - # de-reference the symlink and gets ENOENT, EACCES, - # EPERM errors, hence ignoring those errors if it's on - # symlink file. + # and 'utime with de-reference'. Hence avoiding 'chmod' + # and 'utime' if it's symlink file. is_symlink = False cmd_ret = errno_wrap(os.lchown, [go, uid, gid], [ENOENT], @@ -801,19 +799,17 @@ class Server(object): if isinstance(cmd_ret, int): continue - cmd_ret = errno_wrap(os.chmod, [go, mode], - [ENOENT, EACCES, EPERM], [ESTALE, EINVAL]) - if isinstance(cmd_ret, int): - is_symlink = os.path.islink(go) - if not is_symlink: + is_symlink = os.path.islink(go) + + if not is_symlink: + cmd_ret = errno_wrap(os.chmod, [go, mode], + [ENOENT, EACCES, EPERM], [ESTALE, EINVAL]) + if isinstance(cmd_ret, int): failures.append((e, cmd_ret, "chmod")) - cmd_ret = errno_wrap(os.utime, [go, (atime, mtime)], - [ENOENT, EACCES, EPERM], [ESTALE, EINVAL]) - if isinstance(cmd_ret, int): - if not is_symlink: - is_symlink = os.path.islink(go) - if not is_symlink: + cmd_ret = errno_wrap(os.utime, [go, (atime, mtime)], + [ENOENT, EACCES, EPERM], [ESTALE, EINVAL]) + if isinstance(cmd_ret, int): failures.append((e, cmd_ret, "utime")) return failures -- 1.8.3.1