From 4e654c7f0510a8fefc41971a533cca7b9d393dba Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Tue, 21 Sep 2021 01:02:53 +0100 Subject: [PATCH] copy: Reduce calls to stat() in chown_recursive This reduces the number of times `stat()` or `chown()` are called by the copy module. Reductions come from - combining loops for owner and group, thus not looping twice - not looping over the list of directories returned by `os.walk()`, each directory is already handled when `os.walk()` descends into it Possible further reductions could be achieved by - exiting early from the check_mode loop - Replacing `m.set_owner_if_different()` & `m.set_group_if_different()` calls with a single method. I propose to do these separately, if there is interest. --- .../fragments/75741-copy-reduce-stats.yml | 3 + lib/ansible/modules/copy.py | 84 +++++++------------ 2 files changed, 31 insertions(+), 56 deletions(-) create mode 100644 changelogs/fragments/75741-copy-reduce-stats.yml diff --git a/changelogs/fragments/75741-copy-reduce-stats.yml b/changelogs/fragments/75741-copy-reduce-stats.yml new file mode 100644 index 00000000000..db49a33196b --- /dev/null +++ b/changelogs/fragments/75741-copy-reduce-stats.yml @@ -0,0 +1,3 @@ +minor_changes: + - copy - Reduce the number of disk accesses required to set the owner and + group of copied files and directories diff --git a/lib/ansible/modules/copy.py b/lib/ansible/modules/copy.py index 8a5297466f4..32a7898725c 100644 --- a/lib/ansible/modules/copy.py +++ b/lib/ansible/modules/copy.py @@ -340,62 +340,34 @@ def chown_recursive(path, module): owner = module.params['owner'] group = module.params['group'] - if owner is not None: - if not module.check_mode: - for dirpath, dirnames, filenames in os.walk(path): - owner_changed = module.set_owner_if_different(dirpath, owner, False) - if owner_changed is True: - changed = owner_changed - for dir in [os.path.join(dirpath, d) for d in dirnames]: - owner_changed = module.set_owner_if_different(dir, owner, False) - if owner_changed is True: - changed = owner_changed - for file in [os.path.join(dirpath, f) for f in filenames]: - owner_changed = module.set_owner_if_different(file, owner, False) - if owner_changed is True: - changed = owner_changed - else: - uid = pwd.getpwnam(owner).pw_uid - for dirpath, dirnames, filenames in os.walk(path): - owner_changed = (os.stat(dirpath).st_uid != uid) - if owner_changed is True: - changed = owner_changed - for dir in [os.path.join(dirpath, d) for d in dirnames]: - owner_changed = (os.stat(dir).st_uid != uid) - if owner_changed is True: - changed = owner_changed - for file in [os.path.join(dirpath, f) for f in filenames]: - owner_changed = (os.stat(file).st_uid != uid) - if owner_changed is True: - changed = owner_changed - if group is not None: - if not module.check_mode: - for dirpath, dirnames, filenames in os.walk(path): - group_changed = module.set_group_if_different(dirpath, group, False) - if group_changed is True: - changed = group_changed - for dir in [os.path.join(dirpath, d) for d in dirnames]: - group_changed = module.set_group_if_different(dir, group, False) - if group_changed is True: - changed = group_changed - for file in [os.path.join(dirpath, f) for f in filenames]: - group_changed = module.set_group_if_different(file, group, False) - if group_changed is True: - changed = group_changed - else: - gid = grp.getgrnam(group).gr_gid - for dirpath, dirnames, filenames in os.walk(path): - group_changed = (os.stat(dirpath).st_gid != gid) - if group_changed is True: - changed = group_changed - for dir in [os.path.join(dirpath, d) for d in dirnames]: - group_changed = (os.stat(dir).st_gid != gid) - if group_changed is True: - changed = group_changed - for file in [os.path.join(dirpath, f) for f in filenames]: - group_changed = (os.stat(file).st_gid != gid) - if group_changed is True: - changed = group_changed + if owner is None and group is None: + return changed + + if module.check_mode: + uid = pwd.getpwnam(owner).pw_uid if owner is not None else None + gid = grp.getgrnam(group).gr_gid if group is not None else None + + for dirpath, dummy, filenames in os.walk(path): + stat = os.stat(dirpath) + changed = changed or (uid is not None and uid != stat.st_uid) + changed = changed or (gid is not None and gid != stat.st_gid) + + for filename in filenames: + filepath = os.path.join(dirpath, filename) + stat = os.stat(filepath) + changed = changed or (uid is not None and uid != stat.st_uid) + changed = changed or (gid is not None and gid != stat.st_gid) + + return changed + + for dirpath, dummy, filenames in os.walk(path): + changed = module.set_owner_if_different(dirpath, owner, changed) + changed = module.set_group_if_different(dirpath, group, changed) + + for filename in filenames: + filepath = os.path.join(dirpath, filename) + changed = module.set_owner_if_different(filepath, owner, changed) + changed = module.set_group_if_different(filepath, group, changed) return changed