Change in vdsm[master]: utils: Use ovirt node for persist and unpersist files

Yeela Kaplan has uploaded a new change for review.

Change subject: utils: Use ovirt node for persist and unpersist files
......................................................................

utils: Use ovirt node for persist and unpersist files

Change-Id: Ifaf19e5f8f5beca38d34a606c3eb370385dc206d
Signed-off-by: Yeela Kaplan 
---
M lib/vdsm/utils.py
1 file changed, 19 insertions(+), 15 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/84/31584/1

diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py
index e88007c..b02daa8 100644
--- a/lib/vdsm/utils.py
+++ b/lib/vdsm/utils.py
@@ -143,6 +143,18 @@
             raise

+def persistFile(name):
+    if isOvirtNode():
+        from ovirt.node.utils.fs import Config as NodeCfg
+        NodeCfg().persist(name)
+
+
+def unpersistFile(name):
+    if isOvirtNode():
+        from ovirt.node.utils.fs import Config as NodeCfg
+        NodeCfg().unpersist(name)
+
+
 def rotateFiles(directory, prefixName, gen, cp=False, persist=False):
     logging.debug("dir: %s, prefixName: %s, versions: %s" %
                   (directory, prefixName, gen))
@@ -171,37 +183,29 @@
     for key in keys:
         oldName = os.path.join(directory, fd[key]['old'])
         newName = os.path.join(directory, fd[key]['new'])
-        if isOvirtNode() and persist and not cp:
+        if persist and not cp:
             try:
-                execCmd([constants.EXT_UNPERSIST, oldName],
-                        sudo=True)
-                execCmd([constants.EXT_UNPERSIST, newName],
-                        sudo=True)
+                unpersistFile(oldName)
+                unpersistFile(newName)
             except:
                 pass
         try:
             if cp:
                 execCmd([constants.EXT_CP, oldName, newName], sudo=True)
-                if (isOvirtNode() and
-                        persist and not os.path.exists(newName)):
-                    execCmd([constants.EXT_PERSIST, newName],
-                            sudo=True)
+                if (persist and not os.path.exists(newName)):
+                    persistFile(newName)

else:
                 os.rename(oldName, newName)
         except:
             pass
-        if isOvirtNode() and persist and not cp:
+        if persist and not cp:
             try:
+                persistFile(newName)
                 execCmd([constants.EXT_PERSIST, newName],
                         sudo=True)
             except:
                 pass
-
-
-def persistFile(name):
-    if isOvirtNode():
-        execCmd([constants.EXT_PERSIST, name], sudo=True)

def _parseMemInfo(lines):

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: utils: Use ovirt node for persist and unpersist files
......................................................................

Patch Set 1:

Build Successful

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/10969/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/11911/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/11754/ : SUCCESS

Antoni Segura Puimedon has posted comments on this change.

Change subject: utils: Use ovirt node for persist and unpersist files
......................................................................

Patch Set 1: Code-Review+1

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: utils: Use ovirt node for persist and unpersist files
......................................................................

Patch Set 1:

(1 comment)

http://gerrit.ovirt.org/#/c/31584/1/lib/vdsm/utils.py
File lib/vdsm/utils.py:

Line 143:             raise
Line 144: 
Line 145: 
Line 146: def persistFile(name):
Line 147:     if isOvirtNode():
Can this change during runtime?

It seem that this check should be done on import, and the required functions can be set like this:

if isOvirtNode():
        from ovirt.node.utils.fs import Config
        cfg = Config()
        persistFile = cfg.persist
        unpersistFile = cfg.unpersist
        del cfg
    else:
        persistFile = lambda s: None
        unpersistFile = lambda s: None

Antoni: is this possible or node requires creation of a new instance for each call?
Line 148:         from ovirt.node.utils.fs import Config as NodeCfg
Line 149:         NodeCfg().persist(name)
Line 150: 
Line 151:

Antoni Segura Puimedon has posted comments on this change.

Change subject: utils: Use ovirt node for persist and unpersist files
......................................................................

Patch Set 1:

(1 comment)

http://gerrit.ovirt.org/#/c/31584/1/lib/vdsm/utils.py
File lib/vdsm/utils.py:

Line 143:             raise
Line 144: 
Line 145: 
Line 146: def persistFile(name):
Line 147:     if isOvirtNode():
> Can this change during runtime?

It does not require a new instance. I agree with doing it in import time (As you probably suspect, 'persist' and 'unpersist' are fake instance methods that should have been static.
Line 148:         from ovirt.node.utils.fs import Config as NodeCfg
Line 149:         NodeCfg().persist(name)
Line 150: 
Line 151:

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: utils: Use ovirt node for persist and unpersist files
......................................................................

Patch Set 1:

(1 comment)

http://gerrit.ovirt.org/#/c/31584/1/lib/vdsm/utils.py
File lib/vdsm/utils.py:

Line 143:             raise
Line 144: 
Line 145: 
Line 146: def persistFile(name):
Line 147:     if isOvirtNode():
> It does not require a new instance. I agree with doing it in import time (A

You mean - should have been functions in the fs module :-)
Line 148:         from ovirt.node.utils.fs import Config as NodeCfg
Line 149:         NodeCfg().persist(name)
Line 150: 
Line 151:

Fabian Deutsch has posted comments on this change.

Change subject: utils: Use ovirt node for persist and unpersist files
......................................................................

Patch Set 2: Code-Review+1

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: utils: Use ovirt node for persist and unpersist files
......................................................................

Patch Set 2:

Build Successful

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11358/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12302/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12147/ : SUCCESS

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: utils: Use ovirt node for persist and unpersist files
......................................................................

Patch Set 2:

Please address my comments from patchset 1.

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: utils: Use ovirt node for persist and unpersist files
......................................................................

Patch Set 2:

Part of this patch should be replacement of code that used to copy and paste this code with calling utils.un/persistFile()

For example, here:
http://gerrit.ovirt.org/#/c/30909/7/lib/vdsm/tool/configurators/multipath.py

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: utils: Use ovirt node for persist and unpersist files
......................................................................

Patch Set 3:

Build Successful

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11513/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12457/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12302/ : SUCCESS

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: utils: Use ovirt node for persist and unpersist files
......................................................................

Patch Set 3: Code-Review+1

Antoni Segura Puimedon has posted comments on this change.

Change subject: utils: Use ovirt node for persist and unpersist files
......................................................................

Patch Set 3: Code-Review+1

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: utils: Use ovirt node for persist and unpersist files
......................................................................

Patch Set 4:

Build Successful

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11601/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12545/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12390/ : SUCCESS

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: utils: Use ovirt node for persist and unpersist files
......................................................................

Patch Set 4:

Yeela, can you verify this? it blocks 4 patches, and can (and should) be merged separately.

Yeela Kaplan has abandoned this change.

Change subject: utils: Use ovirt node for persist and unpersist files
......................................................................

Abandoned

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: utils: Use ovirt node for persist and unpersist files
......................................................................

Patch Set 4:

Why abandon? Do we have another patch replacing this?