From 5845a758fde8def526f89b050f2b51e6b3458ddd Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Tue, 6 Jun 2023 12:37:14 -0400 Subject: [PATCH] cephadm: convert more temporary file writes to use write_new Some functions are using the pattern: ``` with open(os.open(name + '.new, os.O_CREAT | os.O_WRONLY, 0o600), 'w') as f: f.write(...) os.rename(name + '.new', name) ``` While it is not entirely clear why this pattern was first used, it accomplishes the same goal as `write_new` only directly calling the posix open call. I analyzed the open flags for `write_new` and these calls using `strace` and noted that the only significant difference was the lack of O_TRUNC in these cases. Since the ".new" files should not exist the lack of O_TRUC ought not make any difference. With this decided we can convert these instances to `write_new`. Signed-off-by: John Mulligan --- src/cephadm/cephadm.py | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/cephadm/cephadm.py b/src/cephadm/cephadm.py index 0434d011c12..86ac0067a8f 100755 --- a/src/cephadm/cephadm.py +++ b/src/cephadm/cephadm.py @@ -4384,9 +4384,8 @@ class MgrListener(Thread): for filename in config: if filename in self.agent.required_files: file_path = os.path.join(self.agent.daemon_dir, filename) - with open(os.open(file_path + '.new', os.O_CREAT | os.O_WRONLY, 0o600), 'w') as f: + with write_new(file_path, perms=0o600) as f: f.write(config[filename]) - os.rename(file_path + '.new', file_path) self.agent.pull_conf_settings() self.agent.wakeup() @@ -4447,27 +4446,23 @@ class CephadmAgent(): for filename in config: if filename in self.required_files: file_path = os.path.join(self.daemon_dir, filename) - with open(os.open(file_path + '.new', os.O_CREAT | os.O_WRONLY, 0o600), 'w') as f: + with write_new(file_path, perms=0o600) as f: f.write(config[filename]) - os.rename(file_path + '.new', file_path) unit_run_path = os.path.join(self.daemon_dir, 'unit.run') - with open(os.open(unit_run_path + '.new', os.O_CREAT | os.O_WRONLY, 0o600), 'w') as f: + with write_new(unit_run_path, perms=0o600) as f: f.write(self.unit_run()) - os.rename(unit_run_path + '.new', unit_run_path) meta: Dict[str, Any] = {} meta_file_path = os.path.join(self.daemon_dir, 'unit.meta') if 'meta_json' in self.ctx and self.ctx.meta_json: meta = json.loads(self.ctx.meta_json) or {} - with open(os.open(meta_file_path + '.new', os.O_CREAT | os.O_WRONLY, 0o600), 'w') as f: + with write_new(meta_file_path, perms=0o600) as f: f.write(json.dumps(meta, indent=4) + '\n') - os.rename(meta_file_path + '.new', meta_file_path) unit_file_path = os.path.join(self.ctx.unit_dir, self.unit_name()) - with open(os.open(unit_file_path + '.new', os.O_CREAT | os.O_WRONLY, 0o600), 'w') as f: + with write_new(unit_file_path, perms=0o600) as f: f.write(self.unit_file()) - os.rename(unit_file_path + '.new', unit_file_path) call_throws(self.ctx, ['systemctl', 'daemon-reload']) call(self.ctx, ['systemctl', 'stop', self.unit_name()], -- 2.39.5