Nir Soffer has posted comments on this change. Change subject: Live Merge: teardown volume on HSM after live merge ......................................................................
Patch Set 2: (3 comments) https://gerrit.ovirt.org/#/c/64301/2/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 4769 Line 4770 Line 4771 Line 4772 Line 4773 Another place where we should not pass an argument. Line 4763: self.vm._setVolumeSize(self.drive.domainID, self.drive.poolID, Line 4764: self.drive.imageID, baseVolUUID, Line 4765: topVolInfo['capacity']) Line 4766: Line 4767: def teardown_top_volume(self, imgUUID, volUUID): Don't pass arguments to this method, it make the code harder to read, and is not needed. We have al the context needed to get the arguments inside this helper. Line 4768: # TODO move this method to storage public API Line 4769: sd_manifest = sdc.sdCache.produce_manifest(self.drive.domainID) Line 4770: sd_manifest.teardownVolume(imgUUID, volUUID) Line 4771: Line 4781: self.vm.enableDriveMonitor() Line 4782: self.success = True Line 4783: self.vm.log.info("Synchronization completed (job %s)", Line 4784: self.job['jobID']) Line 4785: self.teardown_top_volume(self.drive.imageID, self.job['topVolume']) This would be much nicer as: self.teardown_top_volume() 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/64301 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549 Gerrit-PatchSet: 2 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
