Ala Hino has posted comments on this change. Change subject: Live Merge: Remove volume run link after live merge ......................................................................
Patch Set 6: (9 comments) https://gerrit.ovirt.org/#/c/59725/6//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2016-09-22 16:41:56 +0300 Line 4: Commit: Ala Hino <ah...@redhat.com> Line 5: CommitDate: 2016-09-22 17:01:55 +0300 Line 6: Line 7: Live Merge: Unlink volume runtime dir after live merge > Remove volume run link after live merge? Done Line 8: Line 9: Unlink volume runtime dir after live megre. Line 10: Line 11: Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4 Line 5: CommitDate: 2016-09-22 17:01:55 +0300 Line 6: Line 7: Live Merge: Unlink volume runtime dir after live merge Line 8: Line 9: Unlink volume runtime dir after live megre. > Please explain the context like you did in the previous patch. Done Line 10: Line 11: Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4 Line 12: Bug-Url: https://bugzilla.redhat.com/1321018 Line 5: CommitDate: 2016-09-22 17:01:55 +0300 Line 6: Line 7: Live Merge: Unlink volume runtime dir after live merge Line 8: Line 9: Unlink volume runtime dir after live megre. > Please also explain here why this is not needed in file storage. Done Line 10: Line 11: Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4 Line 12: Bug-Url: https://bugzilla.redhat.com/1321018 https://gerrit.ovirt.org/#/c/59725/6/vdsm/storage/blockSD.py File vdsm/storage/blockSD.py: Line 810: offset = ((slot + blockVolume.RESERVED_LEASES) * self.logBlkSize * Line 811: sd.LEASE_BLOCKS) Line 812: return clusterlock.Lease(volUUID, self.getLeasesFilePath(), offset) Line 813: Line 814: def teardownVolume(self, imgUUID, volUUID): > This change belongs to the previous patch. Done Line 815: lvm.deactivateLVs(self.sdUUID, [volUUID]) Line 816: self.removeVolumeRunDir(imgUUID, volUUID) Line 817: Line 818: def removeVolumeRunDir(self, imgUUID, volUUID): Line 814: def teardownVolume(self, imgUUID, volUUID): Line 815: lvm.deactivateLVs(self.sdUUID, [volUUID]) Line 816: self.removeVolumeRunDir(imgUUID, volUUID) Line 817: Line 818: def removeVolumeRunDir(self, imgUUID, volUUID): > removeVolumeRunLink Done Line 819: """ Line 820: Remove /run/vdsm/storage/sdUUID/imgUUID/volUUID Line 821: """ Line 822: sdRunDir = os.path.join(constants.P_VDSM_STORAGE, self.sdUUID) Line 820: Remove /run/vdsm/storage/sdUUID/imgUUID/volUUID Line 821: """ Line 822: sdRunDir = os.path.join(constants.P_VDSM_STORAGE, self.sdUUID) Line 823: imgRunDir = os.path.join(sdRunDir, imgUUID) Line 824: volRunDir = os.path.join(imgRunDir, volUUID) > volSymlink, this is not a directory Done Line 825: try: Line 826: self.log.info("Unlinking volme runtime dir: %s", volRunDir) Line 827: os.unlink(volRunDir) Line 828: except OSError as e: Line 822: sdRunDir = os.path.join(constants.P_VDSM_STORAGE, self.sdUUID) Line 823: imgRunDir = os.path.join(sdRunDir, imgUUID) Line 824: volRunDir = os.path.join(imgRunDir, volUUID) Line 825: try: Line 826: self.log.info("Unlinking volme runtime dir: %s", volRunDir) > The log cannot raise OSError, it should always be out of the try block. Done Line 827: os.unlink(volRunDir) Line 828: except OSError as e: Line 829: if e.error == errno.ENOENT: Line 830: self.log.debug("Link doesn't exist") Line 830: self.log.debug("Link doesn't exist") Line 831: else: Line 832: self.log.error("Failed to unlink vol runtime dir: %s", Line 833: volRunDir) Line 834: raise > Logging and raising is bad practice. If you cannot handle the error, raise, Done Line 835: Line 836: Line 837: class BlockStorageDomain(sd.StorageDomain): Line 838: manifestClass = BlockStorageDomainManifest https://gerrit.ovirt.org/#/c/59725/6/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 4781: self.success = True Line 4782: self.vm.log.info("Synchronization completed (job %s)", Line 4783: self.job['jobID']) Line 4784: # teardown the merged volume Line 4785: self.teardownVolume(self.drive.imageID, self.job['topVolume']) > All these changes belong to the previous patch. Done Line 4786: Line 4787: def isSuccessful(self): Line 4788: """ Line 4789: Returns True if this phase completed successfully. -- 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: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino <ah...@redhat.com> Gerrit-Reviewer: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Ala Hino <ah...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
