[node-patches] Change in ovirt-node[master]: fs: Config.exist() now also checks the file contents

[fabiand at redhat.com]

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 <fabiand at fedoraproject.org>
---
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 <fabiand at redhat.com>