Change in vdsm[master]: Avoid hsm image deletions.

Eduardo has uploaded a new change for review.

Change subject: Avoid hsm image deletions.
......................................................................

Avoid hsm image deletions.

This code was introduced to fix BZ#560389.
The bug was in Image.delete() which was already removed.

Related to BZ#965184.

Change-Id: Ie1ec2ea8793a4ad63453559bc5f663b65f9b9336
Signed-off-by: Eduardo 
---
M vdsm/storage/blockSD.py
M vdsm/storage/fileSD.py
M vdsm/storage/sd.py
3 files changed, 0 insertions(+), 24 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/93/17193/1

diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py
index 72b3ef0..6507cbf 100644
--- a/vdsm/storage/blockSD.py
+++ b/vdsm/storage/blockSD.py
@@ -416,7 +416,6 @@
         # _extendlock is used to prevent race between
         # VG extend and LV extend.
         self._extendlock = threading.Lock()
-        self.imageGarbageCollector()
         self._registerResourceNamespaces()
         self._lastUncachedSelftest = 0

diff --git a/vdsm/storage/fileSD.py b/vdsm/storage/fileSD.py
index 1651825..0042aab 100644
--- a/vdsm/storage/fileSD.py
+++ b/vdsm/storage/fileSD.py
@@ -165,7 +165,6 @@

if not self.oop.fileUtils.pathExists(self.metafile):
             raise se.StorageDomainMetadataNotFound(sdUUID, self.metafile)
-        self.imageGarbageCollector()
         self._registerResourceNamespaces()

@property
@@ -515,20 +514,6 @@
                 mount.getMountFromTarget(self.mountpoint).umount()
                 raise se.FileStorageDomainStaleNFSHandle()
             raise
-
-    def imageGarbageCollector(self):
-        """
-        Image Garbage Collector
-        remove the remnants of the removed images (they could be left sometimes
-        (on NFS mostly) due to lazy file removal
-        """
-        removedPattern = os.path.join(self.domaindir, sd.DOMAIN_IMAGES,
-                                      sd.REMOVED_IMAGE_PREFIX + '*')
-        removedImages = self.oop.glob.glob(removedPattern)
-        self.log.debug("Removing remnants of deleted images %s" %
-                       removedImages)
-        for imageDir in removedImages:
-            self.oop.fileUtils.cleanupdir(imageDir)

def templateRelink(self, imgUUID, volUUID):
         """
diff --git a/vdsm/storage/sd.py b/vdsm/storage/sd.py
index fd79059..c764d2d 100644
--- a/vdsm/storage/sd.py
+++ b/vdsm/storage/sd.py
@@ -786,11 +786,3 @@

def isData(self):
         return self.getMetaParam(DMDK_CLASS) == DATA_DOMAIN
-
-    def imageGarbageCollector(self):
-        """
-        Image Garbage Collector
-        remove the remnants of the removed images (they could be left sometimes
-        (on NFS mostly) due to lazy file removal
-        """
-        pass

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Avoid hsm image deletions.
......................................................................

Patch Set 1:

Build Successful

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/2602/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/3409/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/3493/ : SUCCESS

Ayal Baron has posted comments on this change.

Change subject: Avoid hsm image deletions.
......................................................................

Patch Set 1: I would prefer that you didn't submit this

(1 inline comment)

....................................................
File vdsm/storage/fileSD.py
Line 512:                 self.log.debug("Unmounting stale file system %s",
Line 513:                                self.mountpoint)
Line 514:                 mount.getMountFromTarget(self.mountpoint).umount()
Line 515:                 raise se.FileStorageDomainStaleNFSHandle()
Line 516:             raise
I agree that this method is being called from the wrong place, I don't agree that it should be removed.
It should be called when SPM starts to make sure to clean up garbage.
Line 517: 
Line 518:     def templateRelink(self, imgUUID, volUUID):
Line 519:         """
Line 520:         Relink all hardlinks of the template 'volUUID' in all VMs based on it.

Eduardo has posted comments on this change.

Change subject: Avoid hsm image deletions.
......................................................................

Patch Set 1: (1 inline comment)

....................................................
File vdsm/storage/fileSD.py
Line 512:                 self.log.debug("Unmounting stale file system %s",
Line 513:                                self.mountpoint)
Line 514:                 mount.getMountFromTarget(self.mountpoint).umount()
Line 515:                 raise se.FileStorageDomainStaleNFSHandle()
Line 516:             raise
This method is dangerous, redundant since repeats the implementation of the logic for partially deleted images and was an ad-hoc fix for issues in the removed Image.delete(). Partially deleted images do not affect other image operations any more. Only they take space.

For clean up we should call xxSD.getRemnants()

Starting clean-ups automatically or as a side effect of other operation may be undesirable sometimes, like an SPM failing during a long operation. Will be better if the new SPM resumes the operation instead of remove the partials.(*)
This is unrelated to this patch.

Disclaimer: This is not related with Tasks.
Line 517: 
Line 518:     def templateRelink(self, imgUUID, volUUID):
Line 519:         """
Line 520:         Relink all hardlinks of the template 'volUUID' in all VMs based on it.

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Avoid hsm image deletions.
......................................................................

Patch Set 3:

Build Successful

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/2699/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/3506/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/3590/ : SUCCESS

Ayal Baron has posted comments on this change.

Change subject: Avoid hsm image deletions.
......................................................................

Patch Set 3: I would prefer that you didn't submit this

(1 inline comment)

....................................................
File vdsm/storage/fileSD.py
Line 512:                 self.log.debug("Unmounting stale file system %s",
Line 513:                                self.mountpoint)
Line 514:                 mount.getMountFromTarget(self.mountpoint).umount()
Line 515:                 raise se.FileStorageDomainStaleNFSHandle()
Line 516:             raise
in continuation with comments on patch set 1 (you got -1 and did not make any changes, resubmitting with only rebase is annoying).
anyway, it does not repeat any logic.
currently the system leaves garbage in some (too many) cases.
Point here is for spm to do some cleanup.
Your removal of it means that we will not be cleaning anything up.
Since the image has already been marked for deletion and since this is on files, it is perfectly safe to delete it (even when called from the wrong place).

The call should be moved to someplace where it makes more sense in general (i.e. for block domains as well)
Line 517: 
Line 518:     def templateRelink(self, imgUUID, volUUID):
Line 519:         """
Line 520:         Relink all hardlinks of the template 'volUUID' in all VMs based on it.

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Avoid hsm image deletions.
......................................................................

Patch Set 4:

Build Successful

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/2721/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/3528/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/3612/ : SUCCESS

Ayal Baron has posted comments on this change.

Change subject: Avoid hsm image deletions.
......................................................................

Patch Set 4: Code-Review-2

-2 for visibility since you keep on rebasing without addressing the comments

Eduardo has posted comments on this change.

Change subject: Avoid hsm image deletions.
......................................................................

Patch Set 4:

Comments addressed.

This is fossilized code, failing to solve flaws from code that is not existing anymore, cluttering the log and wasting IOs.

A very bad pattern modifying the storage as HSM.

Proper garbage collection will be added at some point in the future.

Itamar Heim has posted comments on this change.

Change subject: Avoid hsm image deletions.
......................................................................

Patch Set 4:

ping?

Itamar Heim has abandoned this change.

Change subject: Avoid hsm image deletions.
......................................................................

Abandoned

abandoning per no reply. please restore if still relevant.