Fabian Deutsch has uploaded a new change for review. Change subject: fs: Config.exist() now also checks the file contents ......................................................................
fs: Config.exist() now also checks the file contents Before, Config.exists() only checked if a file was persisted, by checking that a copy resided in /config. A situation could be encountered where the contens of the persisted and the in-place file differed. This situation happened when a file was persisted, the bind-mount got removed, the in-place file got modified. In the end the problem was that when these two files were out of sync, but a persisted copy existsed, the files were not re-synced. To mitigate this risk, the exists call now also checks the contents to ensure that a out-of-sync persisted copy also counts as unpersisted. In the end the semantics changed from "Is the file persisted?" to "Are the contents of the file persisted?" Example: 1. echo abc > /tmp/foo 2. persist /tmp/foo 3. unmount_config /tmp/fooo 4. echo def > /tmp/foo 5. persist /tmp/foo Before the patch, after step 5, the contents of /config/tmp/foo were abc, after this patch the contents are "def" Change-Id: I10107c53aa466998fb4bfb0c9e8750f86613eaa7 Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1251867 Signed-off-by: Fabian Deutsch <[email protected]> --- M src/ovirt/node/utils/fs/__init__.py 1 file changed, 16 insertions(+), 17 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-node refs/changes/86/46186/1 diff --git a/src/ovirt/node/utils/fs/__init__.py b/src/ovirt/node/utils/fs/__init__.py index dcf312c..27a7422 100644 --- a/src/ovirt/node/utils/fs/__init__.py +++ b/src/ovirt/node/utils/fs/__init__.py @@ -467,12 +467,8 @@ self._logger.info('Directory "%s" successfully persisted', abspath) self._add_path_entry(abspath) - def cksum(self, filename): - try: - m = hashlib.md5() - except: - m = hashlib.sha1() - + def checksum(self, filename): + m = hashlib.sha1() with open(filename) as f: data = f.read(4096) while data: @@ -486,8 +482,8 @@ persisted_path = self._config_path(abspath) if os.path.exists(persisted_path): - current_checksum = self.cksum(abspath) - stored_checksum = self.cksum(persisted_path) + current_checksum = self.checksum(abspath) + stored_checksum = self.checksum(persisted_path) if stored_checksum == current_checksum: self._logger.warn('File "%s" had already been persisted', abspath) @@ -679,22 +675,25 @@ from ovirtnode import ovirtfunctions return ovirtfunctions.ovirt_safe_delete_config(filename) - def exists(self, filename): + def exists(self, filename, and_is_in_sync=True): """Check if the given file is persisted """ filename = os.path.abspath(filename) persisted_path = self._config_path(filename) - if not os.path.exists(persisted_path) or \ - not os.path.exists(filename): - return False + exists = True - current_checksum = self.cksum(filename) - stored_checksum = self.cksum(persisted_path) - if stored_checksum == current_checksum: - return True + # Check that the files exist + exists &= os.path.exists(persisted_path) + exists &= os.path.exists(filename) - return False + if exists and and_is_in_sync: + # If requested, also check that the contents match + current_checksum = self.checksum(filename) + stored_checksum = self.checksum(persisted_path) + exists &= stored_checksum == current_checksum: + + return exists def is_enabled(self): return File("/proc").exists() and is_bind_mount(self.basedir) -- To view, visit https://gerrit.ovirt.org/46186 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I10107c53aa466998fb4bfb0c9e8750f86613eaa7 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-node Gerrit-Branch: master Gerrit-Owner: Fabian Deutsch <[email protected]> _______________________________________________ node-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/node-patches
