Arik Hadas has posted comments on this change. Change subject: always teardown snapshot's memory volume ......................................................................
Patch Set 1: (3 comments) http://gerrit.ovirt.org/#/c/26544/1/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 3649: # is to prevent spurious libvirt errors about missing drive paths Line 3650: # (since we're changing them), and also to prevent to trigger a drive Line 3651: # extension for the new volume with the apparent size of the old one Line 3652: # (the apparentsize is updated as last step in updateDriveParameters) Line 3653: self.stopDisksStatsCollection() > Arik, just wondering…once we do the real live RAM snapshot the above must n I'm not sure, but I see that it is also called from diskReplicateFinish which is part of live-storage-migration - similar flow to real live snapshot since in both cases the VM is running and the volume we're writing to is changed. so I think it would still be needed Line 3654: Line 3655: try: Line 3656: try: Line 3657: self._dom.snapshotCreateXML(snapxml, snapFlags) Line 3679: # This code should be removed once qemu-img will handle files Line 3680: # with size that is not multiple of block size correctly. Line 3681: if memoryParams: Line 3682: _padMemoryVolume(memoryVolPath, memoryVol['domainID']) Line 3683: # Teardown should happen anyway (also in case of exceptions). > not if an exception is raised from within _padMemoryVolume(). right right, it was just a TODO for me :) sorry for the confusion Line 3684: self.cif.teardownVolumePath(memoryVol) Line 3685: Line 3686: for drive in newDrives.values(): # Update the drive information Line 3687: try: Line 3680: # with size that is not multiple of block size correctly. Line 3681: if memoryParams: Line 3682: _padMemoryVolume(memoryVolPath, memoryVol['domainID']) Line 3683: # Teardown should happen anyway (also in case of exceptions). Line 3684: self.cif.teardownVolumePath(memoryVol) > how about a "if memoryVolPath" in the "finally" section? Done Line 3685: Line 3686: for drive in newDrives.values(): # Update the drive information Line 3687: try: Line 3688: self.updateDriveParameters(drive) -- To view, visit http://gerrit.ovirt.org/26544 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If59047f7d5ba6cfa942b593683f6f9987619a7ea Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas <[email protected]> Gerrit-Reviewer: Arik Hadas <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
