Change in vdsm[master]: misc: remove cp parameter

Yeela Kaplan has uploaded a new change for review.

Change subject: misc: remove cp parameter
......................................................................

misc: remove cp parameter

All usages of cp parameter have been extinguished.
Therefore, no longer needed.

Change-Id: I0906bfd7dfa128c323aa399810bbd75883618434
Signed-off-by: Yeela Kaplan 
---
M vdsm/storage/misc.py
1 file changed, 5 insertions(+), 13 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/13/45613/1

diff --git a/vdsm/storage/misc.py b/vdsm/storage/misc.py
index b14fd26..b0146c8 100644
--- a/vdsm/storage/misc.py
+++ b/vdsm/storage/misc.py
@@ -449,7 +449,7 @@
     return n

-def rotateFiles(directory, prefixName, gen, cp=False, persist=False):
+def rotateFiles(directory, prefixName, gen, persist=False):
     log.debug("dir: %s, prefixName: %s, versions: %s" %
               (directory, prefixName, gen))
     gen = int(gen)
@@ -477,24 +477,16 @@
     for key in keys:
         oldName = os.path.join(directory, fd[key]['old'])
         newName = os.path.join(directory, fd[key]['new'])
-        if utils.isOvirtNode() and persist and not cp:
+        if utils.isOvirtNode() and persist:
             try:
                 utils.unpersist(oldName)
                 utils.unpersist(newName)
             except:
                 pass
-        try:
-            if cp:
-                execCmd([constants.EXT_CP, oldName, newName], sudo=True)
-                if (utils.isOvirtNode() and
-                        persist and not os.path.exists(newName)):
-                    utils.persist(newName)

-            else:
-                os.rename(oldName, newName)
-        except:
-            pass
-        if utils.isOvirtNode() and persist and not cp:
+        os.rename(oldName, newName)
+
+        if utils.isOvirtNode() and persist:
             try:
                 utils.persist(newName)
             except:

[e-mail] has posted comments on this change.

Change subject: misc: remove cp parameter
......................................................................

Patch Set 1:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: misc: remove cp parameter
......................................................................

Patch Set 1: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/45613/1/vdsm/storage/misc.py
File vdsm/storage/misc.py:

Line 483:                 utils.unpersist(newName)
Line 484:             except:
Line 485:                 pass
Line 486: 
Line 487:         os.rename(oldName, newName)
the original except:pass was horrible, but you should not change the logic in such a patch, so please keep it.
Line 488: 
Line 489:         if utils.isOvirtNode() and persist:
Line 490:             try:
Line 491:                 utils.persist(newName)

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: misc: remove cp parameter
......................................................................

Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/45613/1/vdsm/storage/misc.py
File vdsm/storage/misc.py:

Line 488: 
EXT_CP should be dropped from constants, too.

Yeela Kaplan has posted comments on this change.

Change subject: misc: remove cp parameter
......................................................................

Patch Set 1:

(2 comments)

https://gerrit.ovirt.org/#/c/45613/1/vdsm/storage/misc.py
File vdsm/storage/misc.py:

Line 488:
> EXT_CP should be dropped from constants, too.

EXT_CP is still used in multipath configurator

Line 483:                 utils.unpersist(newName)
Line 484:             except:
Line 485:                 pass
Line 486: 
Line 487:         os.rename(oldName, newName)
> the original except:pass was horrible, but you should not change the logic

Done
Line 488: 
Line 489:         if utils.isOvirtNode() and persist:
Line 490:             try:
Line 491:                 utils.persist(newName)

[e-mail] has posted comments on this change.

Change subject: misc: remove cp parameter
......................................................................

Patch Set 2:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: misc: remove cp parameter
......................................................................

Patch Set 2: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/45613/2/vdsm/storage/misc.py
File vdsm/storage/misc.py:

Line 448:         raise se.InvalidParameterException(name, number)
Line 449:     return n
Line 450: 
Line 451: 
Line 452: def rotateFiles(directory, prefixName, gen, persist=False):
This function does not work and its tests are wrong. We also cannot use this code as it is extremely slow on node, and it is used for backing up domain metadata in /var/log/vdsm/backup/

Please remove this function.
Line 453:     log.debug("dir: %s, prefixName: %s, versions: %s" %
Line 454:               (directory, prefixName, gen))
Line 455:     gen = int(gen)
Line 456:     files = os.listdir(directory)

Yeela Kaplan has posted comments on this change.

Change subject: misc: remove cp parameter
......................................................................

Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/45613/2/vdsm/storage/misc.py
File vdsm/storage/misc.py:

Line 448:         raise se.InvalidParameterException(name, number)
Line 449:     return n
Line 450: 
Line 451: 
Line 452: def rotateFiles(directory, prefixName, gen, persist=False):
> This function does not work and its tests are wrong. We also cannot use thi

If it's used for backing up domain metadata why would you want me to remove it? what do you want instead?
Line 453:     log.debug("dir: %s, prefixName: %s, versions: %s" %
Line 454:               (directory, prefixName, gen))
Line 455:     gen = int(gen)
Line 456:     files = os.listdir(directory)

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: misc: remove cp parameter
......................................................................

Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/45613/2/vdsm/storage/misc.py
File vdsm/storage/misc.py:

Line 448:         raise se.InvalidParameterException(name, number)
Line 449:     return n
Line 450: 
Line 451: 
Line 452: def rotateFiles(directory, prefixName, gen, persist=False):
> If it's used for backing up domain metadata why would you want me to remove

This function does *nothing* since 2012 (since commit 8bc23a66b409f47e9b652f82c950e04e4816af19). The tests are also broken - they pass while the function does nothing.

Since we are living happily without the domain metadata backup for about 3 years, we will continue without it.

On node, this way of backup - keeping 30 copies and renaming all of them on each backup, doing 30 unpresist and 30 persist operations per run is crazy.
Line 453:     log.debug("dir: %s, prefixName: %s, versions: %s" %
Line 454:               (directory, prefixName, gen))
Line 455:     gen = int(gen)
Line 456:     files = os.listdir(directory)

Change in vdsm[master]: Remove unused sd setMetadata and rotateFiles

[e-mail] has posted comments on this change.

Change subject: Remove unused sd setMetadata and rotateFiles
......................................................................

Patch Set 3:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

Change in vdsm[master]: Remove unused sd setMetadata and rotateFiles

[e-mail] has posted comments on this change.

Change subject: Remove unused sd setMetadata and rotateFiles
......................................................................

Patch Set 4:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

Change in vdsm[master]: Remove unused sd setMetadata and rotateFiles

[e-mail] has posted comments on this change.

Change subject: Remove unused sd setMetadata and rotateFiles
......................................................................

Patch Set 5:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

Change in vdsm[master]: Remove unused sd setMetadata and rotateFiles

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: Remove unused sd setMetadata and rotateFiles
......................................................................

Patch Set 5:

Thanks for this cleanup!

Can you separate the removal of rotateFiles and setMetadata?

I want to check the second and I would like to remove the first now.

Change in vdsm[master]: Remove unused sd setMetadata and rotateFiles

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: Remove unused sd setMetadata and rotateFiles
......................................................................

Patch Set 5: Code-Review-1

-1 for visibility.

Change in vdsm[master]: Remove unused sd setMetadata and rotateFiles

Yeela Kaplan has posted comments on this change.

Change subject: Remove unused sd setMetadata and rotateFiles
......................................................................

Patch Set 5:

Done

Change in vdsm[master]: sd: Remove unused setMetadata

gerrit-hooks has posted comments on this change.

Change subject: sd: Remove unused setMetadata
......................................................................

Patch Set 6:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

Change in vdsm[master]: sd: Remove unused setMetadata

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: sd: Remove unused setMetadata
......................................................................

Patch Set 6: Code-Review-1

(4 comments)

https://gerrit.ovirt.org/#/c/45613/6//COMMIT_MSG
Commit Message:

Line 5: CommitDate: 2015-11-15 14:21:54 +0200
Line 6: 
Line 7: sd: Remove unused setMetadata
Line 8: 
Line 9: All usages of sd setMetadata have been removed.
Which code that was calling setMetadata was removed?  can you point me to the relevant patch?
Line 10: 
Line 11: Therefore, no longer needed.
Line 12: As a result, the backup directory for sd MD
Line 13: creation and default values are also removed.

https://gerrit.ovirt.org/#/c/45613/6/vdsm/storage/sd.py
File vdsm/storage/sd.py:

Line 874
Line 875
Line 876
Line 877
Line 878
This does nothing, so it makes sense to remove.

Line 878
Line 879
Line 880
Line 881
Line 882
This does work, backing up last metadata to backup dir. Why do you want to remove this code?

Line 882
Line 883
Line 884
Line 885
Line 886
And why do you want to remove this code, updating metadata on shared storage?!

Are you sure we don't call this code?

Change in vdsm[master]: sd: Remove unused setMetadata

[Oved Ourfali]

Oved Ourfali has posted comments on this change.

Change subject: sd: Remove unused setMetadata
......................................................................

Patch Set 10:

Nir, please abandon this if it isn't needed anymore.