Change in vdsm[master]: core: Unlink image run directory when deleting a snapshot

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: core: Unlink image run directory when deleting a snapshot
......................................................................


Patch Set 1:

(5 comments)

https://gerrit.ovirt.org/#/c/59725/1/vdsm/storage/blockVolume.py
File vdsm/storage/blockVolume.py:

Line 604
Line 605
Line 606
Line 607
Line 608
This code is bad, don't copy it. We will not fix it in this patch but we should not
duplicate bad code.


Line 610:                            self.sdUUID, self.volUUID, vol_path, exc_info=True)
Line 611: 
Line 612:         try:
Line 613:             imgRundir = os.path.join(constants.P_VDSM_STORAGE, self.sdUUID,
Line 614:                                      self.imgUUID, self.volUUID)
This is the volume run path, not the image directory
Line 615:             self.log.debug("Unlinking %s", imgRundir)
Line 616:             os.unlink(imgRundir)
Line 617:             return True
Line 618:         except Exception as e:


Line 611: 
Line 612:         try:
Line 613:             imgRundir = os.path.join(constants.P_VDSM_STORAGE, self.sdUUID,
Line 614:                                      self.imgUUID, self.volUUID)
Line 615:             self.log.debug("Unlinking %s", imgRundir)
Use %r
Line 616:             os.unlink(imgRundir)
Line 617:             return True
Line 618:         except Exception as e:
Line 619:             eFound = e


Line 614:                                      self.imgUUID, self.volUUID)
Line 615:             self.log.debug("Unlinking %s", imgRundir)
Line 616:             os.unlink(imgRundir)
Line 617:             return True
Line 618:         except Exception as e:
This will log errors when the run link path does not exists - very bad. Should log debug
message in this case. Use separate handler for OSError and errno == ENOENT
Line 619:             eFound = e
Line 620:             self.log.error("cannot delete volume's %s/%s link path:
%s",
Line 621:                            self.sdUUID, self.volUUID, imgRundir, exc_info=True)
Line 622: 


Line 616:             os.unlink(imgRundir)
Line 617:             return True
Line 618:         except Exception as e:
Line 619:             eFound = e
Line 620:             self.log.error("cannot delete volume's %s/%s link path:
%s",
This is the volume run link
Line 621:                            self.sdUUID, self.volUUID, imgRundir, exc_info=True)
Line 622: 
Line 623:         raise eFound
Line 624: 


-- 
To view, visit https://gerrit.ovirt.org/59725
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino <ahino(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes