Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge
gerrit-hooks has posted comments on this change. Change subject: Live Merge: Teardown volume on HSM after live merge .. Patch Set 8: * #1377849::Update tracker: OK * #64301::Update tracker: OK * Set MODIFIED::bug 1377849#1377849::IGNORE, skipping for branch 'master' -- 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: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge
Nir Soffer has submitted this change and it was merged. Change subject: Live Merge: Teardown volume on HSM after live merge .. Live Merge: Teardown volume on HSM after live merge If a VM is running on HSM and live merge is performed, the LV isn't deactivated because the deactivation is done when deleting the volume. However, deleting the volume is done on SPM and this means that the LV is not deactivated on the HSM. In this patch, a logic to teardown the volume is added after live merge has completed. Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549 Bug-Url: https://bugzilla.redhat.com/1377849 Signed-off-by: Ala HinoReviewed-on: https://gerrit.ovirt.org/64301 Reviewed-by: Nir Soffer Continuous-Integration: Nir Soffer Continuous-Integration: Jenkins CI Reviewed-by: Adam Litke --- M vdsm/storage/blockSD.py M vdsm/storage/sd.py M vdsm/virt/vm.py 3 files changed, 19 insertions(+), 0 deletions(-) Approvals: Adam Litke: Looks good to me, approved Nir Soffer: Looks good to me, but someone else must approve; Passed CI tests Jenkins CI: Passed CI tests Ala Hino: Verified -- To view, visit https://gerrit.ovirt.org/64301 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge
Adam Litke has posted comments on this change. Change subject: Live Merge: Teardown volume on HSM after live merge .. Patch Set 7: Code-Review+2 -- 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: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge
Nir Soffer has posted comments on this change. Change subject: Live Merge: Teardown volume on HSM after live merge .. Patch Set 7: Continuous-Integration+1 CI is broken, we will ignore it. -- 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: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge
Nir Soffer has posted comments on this change. Change subject: Live Merge: Teardown volume on HSM after live merge .. Patch Set 7: Code-Review+1 Waiting for more reviews. -- 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: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge
Ala Hino has posted comments on this change. Change subject: Live Merge: Teardown volume on HSM after live merge .. Patch Set 7: Verified+1 Verified both on SPM and HSM that after live merge the volume is deactivated. Verified top volume merge and internal volume merge -- 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: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge
gerrit-hooks has posted comments on this change. Change subject: Live Merge: Teardown volume on HSM after live merge .. Patch Set 7: * update_tracker: OK * Check Bug-Url::OK * Check Public Bug::#1377849::OK, public bug * Check Product::#1377849::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge
Ala Hino has posted comments on this change. Change subject: Live Merge: Teardown volume on HSM after live merge .. Patch Set 6: (2 comments) https://gerrit.ovirt.org/#/c/64301/6/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 4779: "(job %s)", self.job['jobID']) Line 4780: self.vm._syncVolumeChain(self.drive) Line 4781: if self.doPivot: Line 4782: self.vm.enableDriveMonitor() Line 4783: self.teardown_top_volume() > How is this needed only after pivot? Done Line 4784: self.success = True Line 4785: self.vm.log.info("Synchronization completed (job %s)", Line 4786: self.job['jobID']) Line 4787: # At this point, merge has succesfully completed and the top volume is Line 4788: # not part of the chain. Now, we want to teardown the top volume. Note Line 4789: # that if volume deactivation fails, we don't want to fail the merge Line 4790: # whole operation as the VM is running without issues. It is worth to Line 4791: # note that if volume deactivation fails, chances are high that the Line 4792: # environment is severely damaged. > This comment conflict with the code now. Removed Line 4793: Line 4794: def isSuccessful(self): Line 4795: """ Line 4796: 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: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge
Nir Soffer has posted comments on this change. Change subject: Live Merge: Teardown volume on HSM after live merge .. Patch Set 6: (2 comments) Did you verify the current code with both top layer merge and internal layer merge, or you just copied the verified flag from a previous version? https://gerrit.ovirt.org/#/c/64301/6/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 4779: "(job %s)", self.job['jobID']) Line 4780: self.vm._syncVolumeChain(self.drive) Line 4781: if self.doPivot: Line 4782: self.vm.enableDriveMonitor() Line 4783: self.teardown_top_volume() How is this needed only after pivot? We need to teardown the volume after any merge, not only top layer merge. I think I suggested to add teardown in this line, but not inside the if block. Line 4784: self.success = True Line 4785: self.vm.log.info("Synchronization completed (job %s)", Line 4786: self.job['jobID']) Line 4787: # At this point, merge has succesfully completed and the top volume is Line 4788: # not part of the chain. Now, we want to teardown the top volume. Note Line 4789: # that if volume deactivation fails, we don't want to fail the merge Line 4790: # whole operation as the VM is running without issues. It is worth to Line 4791: # note that if volume deactivation fails, chances are high that the Line 4792: # environment is severely damaged. This comment conflict with the code now. Line 4793: Line 4794: def isSuccessful(self): Line 4795: """ Line 4796: 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: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge
Nir Soffer has posted comments on this change. Change subject: Live Merge: Teardown volume on HSM after live merge .. Patch Set 6: Code-Review-1 -- 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: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge
Ala Hino has posted comments on this change. Change subject: Live Merge: Teardown volume on HSM after live merge .. Patch Set 6: Verified+1 -- 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: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge
gerrit-hooks has posted comments on this change. Change subject: Live Merge: Teardown volume on HSM after live merge .. Patch Set 6: * #1377849::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1377849::OK, public bug * Check Product::#1377849::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge
Ala Hino has posted comments on this change. Change subject: Live Merge: Teardown volume on HSM after live merge .. Patch Set 5: (5 comments) https://gerrit.ovirt.org/#/c/64301/5/vdsm/storage/sd.py File vdsm/storage/sd.py: PS5, Line 531: teared > torn Done PS5, Line 532: teared > torn Done https://gerrit.ovirt.org/#/c/64301/5/vdsm/virt/vm.py File vdsm/virt/vm.py: PS5, Line 4786: nvolume > volume Done PS5, Line 4788: megre > merge Done Line 4787: # not part of the chain. Now, we want to teardown the top volume. Note Line 4788: # that if volume deactivation fails, we don't want to fail the megre Line 4789: # whole operation as the VM is running without issues. It is worth to Line 4790: # note that if volume deactivation fails, chances are high that the Line 4791: # environment is severely damaged. > I do want to the operation to fail, we cannot continue to use this environm Moved Line 4792: self.teardown_top_volume() Line 4793: Line 4794: def isSuccessful(self): Line 4795: """ -- 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: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge
Nir Soffer has posted comments on this change. Change subject: Live Merge: Teardown volume on HSM after live merge .. Patch Set 5: (2 comments) https://gerrit.ovirt.org/#/c/64301/5/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 4764:self.drive.imageID, baseVolUUID, Line 4765:topVolInfo['capacity']) Line 4766: Line 4767: def teardown_top_volume(self): Line 4768: # TODO move this method to storage public API > This needs to happen before we can merge. We can't have code like this in The plan is to backport this to 4.0, where it is ok to add these dependencies. Nobody is going to break vdsm to multiple processes in 4.0 :-) In master I expect to see a patch adding a proper storage api soon. Line 4769: sd_manifest = sdc.sdCache.produce_manifest(self.drive.domainID) Line 4770: sd_manifest.teardownVolume(self.drive.imageID, Line 4771:self.job['topVolume']) Line 4772: Line 4787: # not part of the chain. Now, we want to teardown the top volume. Note Line 4788: # that if volume deactivation fails, we don't want to fail the megre Line 4789: # whole operation as the VM is running without issues. It is worth to Line 4790: # note that if volume deactivation fails, chances are high that the Line 4791: # environment is severely damaged. I do want to the operation to fail, we cannot continue to use this environment. Line 4792: self.teardown_top_volume() Line 4793: Line 4794: def isSuccessful(self): Line 4795: """ -- 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: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge
Adam Litke has posted comments on this change. Change subject: Live Merge: Teardown volume on HSM after live merge .. Patch Set 5: (5 comments) https://gerrit.ovirt.org/#/c/64301/5/vdsm/storage/sd.py File vdsm/storage/sd.py: PS5, Line 531: teared torn PS5, Line 532: teared torn https://gerrit.ovirt.org/#/c/64301/5/vdsm/virt/vm.py File vdsm/virt/vm.py: PS5, Line 4768: TODO This needs to happen before we can merge. We can't have code like this in virt. PS5, Line 4786: nvolume volume PS5, Line 4788: megre merge -- 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: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge
gerrit-hooks has posted comments on this change. Change subject: Live Merge: Teardown volume on HSM after live merge .. Patch Set 5: * #1377849::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1377849::OK, public bug * Check Product::#1377849::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge
Nir Soffer has posted comments on this change. Change subject: Live Merge: Teardown volume on HSM after live merge .. Patch Set 4: Code-Review-1 -- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge
Nir Soffer has posted comments on this change. Change subject: Live Merge: Teardown volume on HSM after live merge .. Patch Set 3: (1 comment) https://gerrit.ovirt.org/#/c/64301/3/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 4782: self.vm.enableDriveMonitor() Line 4783: self.success = True Line 4784: self.vm.log.info("Synchronization completed (job %s)", Line 4785: self.job['jobID']) Line 4786: self.teardown_top_volume() > After another thought, I think it is better not to fail merge in this case, I don't think this is a good idea, if we cannot deactivate the lv, or remove the symlinks in /run/vdsm, the system is fucked up and we should fail hard and loud. If you do not agree you should add a big comment here explaining why the clean must succeed when we cannot teardown the volume. Line 4787: Line 4788: def isSuccessful(self): Line 4789: """ Line 4790: 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: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge
Nir Soffer has posted comments on this change. Change subject: Live Merge: Teardown volume on HSM after live merge .. Patch Set 2: (1 comment) 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 > Actually, _syncVolumeChain is defined on the VM and the drive isn't held as Right, but this operation should have been defined in the cleanup thread. A method should use the same abstraction level (e.g operations on self, or operations on self.vm etc.) -- 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 HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge
Ala Hino has posted comments on this change. Change subject: Live Merge: Teardown volume on HSM after live merge .. Patch Set 2: (1 comment) 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. Actually, _syncVolumeChain is defined on the VM and the drive isn't held as a variable of the VM -- 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 HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge
Ala Hino has posted comments on this change. Change subject: Live Merge: Teardown volume on HSM after live merge .. Patch Set 3: (4 comments) https://gerrit.ovirt.org/#/c/64301/3//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2016-09-22 15:47:20 +0300 Line 4: Commit: Ala HinoLine 5: CommitDate: 2016-09-23 00:01:49 +0300 Line 6: Line 7: Live Merge: teardown volume on HSM after live merge > Teardown Done Line 8: Line 9: If a VM is running on HSM and live merge is performed, the LV isn't Line 10: deactivated because, the deactivation is done when deleting the volume. Line 11: However, deleting the volume is done on SPM and this means that the LV PS3, Line 10: , > This comma is redundant. Done PS3, Line 13: > missing "is" Done https://gerrit.ovirt.org/#/c/64301/3/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 4782: self.vm.enableDriveMonitor() Line 4783: self.success = True Line 4784: self.vm.log.info("Synchronization completed (job %s)", Line 4785: self.job['jobID']) Line 4786: self.teardown_top_volume() > This must be done *before* we set self.success to True, so it this fails, w After another thought, I think it is better not to fail merge in this case, as the chances to fail here are really low and if failed, probably the env is severely damaged Line 4787: Line 4788: def isSuccessful(self): Line 4789: """ Line 4790: 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: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge
gerrit-hooks has posted comments on this change. Change subject: Live Merge: Teardown volume on HSM after live merge .. Patch Set 4: * #1377849::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1377849::OK, public bug * Check Product::#1377849::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge
Nir Soffer has posted comments on this change. Change subject: Live Merge: teardown volume on HSM after live merge .. Patch Set 3: (1 comment) https://gerrit.ovirt.org/#/c/64301/3//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2016-09-22 15:47:20 +0300 Line 4: Commit: Ala HinoLine 5: CommitDate: 2016-09-23 00:01:49 +0300 Line 6: Line 7: Live Merge: teardown volume on HSM after live merge Teardown Line 8: Line 9: If a VM is running on HSM and live merge is performed, the LV isn't Line 10: deactivated because, the deactivation is done when deleting the volume. Line 11: However, deleting the volume is done on SPM and this means that the LV -- 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: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge
Nir Soffer has posted comments on this change. Change subject: Live Merge: teardown volume on HSM after live merge .. Patch Set 3: (2 comments) https://gerrit.ovirt.org/#/c/64301/3/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 4771 Line 4772 Line 4773 Line 4774 Line 4775 This can be good place for tearing down the volume. Line 4782: self.vm.enableDriveMonitor() Line 4783: self.success = True Line 4784: self.vm.log.info("Synchronization completed (job %s)", Line 4785: self.job['jobID']) Line 4786: self.teardown_top_volume() This must be done *before* we set self.success to True, so it this fails, we will try again. Line 4787: Line 4788: def isSuccessful(self): Line 4789: """ Line 4790: 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: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge
Allon Mureinik has posted comments on this change. Change subject: Live Merge: teardown volume on HSM after live merge .. Patch Set 3: Code-Review+1 (2 comments) https://gerrit.ovirt.org/#/c/64301/3//COMMIT_MSG Commit Message: PS3, Line 10: , This comma is redundant. PS3, Line 13: missing "is" -- 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: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge
gerrit-hooks has posted comments on this change. Change subject: Live Merge: teardown volume on HSM after live merge .. Patch Set 3: * #1377849::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1377849::OK, public bug * Check Product::#1377849::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge
Ala Hino 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. separate patch ... 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 Done 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: 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/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 HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge
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 HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge
gerrit-hooks has posted comments on this change. Change subject: Live Merge: teardown volume on HSM after live merge .. Patch Set 2: * #1377849::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1377849::OK, public bug * Check Product::#1377849::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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 HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge
Ala Hino has posted comments on this change. Change subject: Live Merge: teardown volume on HSM after live merge .. Patch Set 1: (8 comments) https://gerrit.ovirt.org/#/c/64301/1/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, volUUID): > Add image uuid, standard api for volume methods. Done Line 815: lvm.deactivateLVs(self.sdUUID, [volUUID]) Line 816: Line 817: Line 818: class BlockStorageDomain(sd.StorageDomain): https://gerrit.ovirt.org/#/c/64301/1/vdsm/storage/sd.py File vdsm/storage/sd.py: Line 524: Line 525: if preallocate is not None and preallocate not in sc.VOL_TYPE: Line 526: raise se.IncorrectType(preallocate) Line 527: Line 528: def teardownVolume(self, volUUID): > Volume is defined by domain id, image id and volume id. Please add the imag Done Line 529: """ Line 530: By default, this method does nothing. It is overriden in blockSD Line 531: to do the necessary cleaning. Line 532: """ Line 527: Line 528: def teardownVolume(self, volUUID): Line 529: """ Line 530: By default, this method does nothing. It is overriden in blockSD Line 531: to do the necessary cleaning. > Replace the text with something like: Done Line 532: """ Line 533: pass Line 534: Line 535: Line 529: """ Line 530: By default, this method does nothing. It is overriden in blockSD Line 531: to do the necessary cleaning. Line 532: """ Line 533: pass > pass is not needed, having a docstring is enough. Done Line 534: Line 535: Line 536: class StorageDomain(object): Line 537: log = logging.getLogger("storage.StorageDomain") https://gerrit.ovirt.org/#/c/64301/1/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 64: from vdsm.virt.vmpowerdown import VmShutdown, VmReboot Line 65: from vdsm.virt.utils import isVdsmImage, cleanup_guest_socket Line 66: from storage import outOfProcess as oop Line 67: from storage import sd Line 68: from storage.sdc import sdCache > Please avoid this bad practice - this must be: Done Line 69: Line 70: # local imports Line 71: # In future those should be imported via .. Line 72: import caps 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 teardownVolume(self, volUUID): > This should match other methods like update_base_size - no arguments, lower Done Line 4768: dom_manifest = sdCache.produce_manifest(self.drive.domainID) Line 4769: dom_manifest.teardownVolume(volUUID) Line 4770: Line 4771: @utils.traceback() Line 4764:self.drive.imageID, baseVolUUID, Line 4765:topVolInfo['capacity']) Line 4766: Line 4767: def teardownVolume(self, volUUID): Line 4768: dom_manifest = sdCache.produce_manifest(self.drive.domainID) > We call this sd_manifest elsewhere. Done Line 4769: dom_manifest.teardownVolume(volUUID) Line 4770: Line 4771: @utils.traceback() Line 4772: def run(self): Line 4780: self.vm.enableDriveMonitor() 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 > The comment is not needed, the function name should reveal the intent. If y Done Line 4785: self.teardownVolume(self.job['topVolume']) Line 4786: Line 4787: def isSuccessful(self): Line 4788: """ -- 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: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge
Nir Soffer has posted comments on this change. Change subject: Live Merge: teardown volume on HSM after live merge .. Patch Set 1: (8 comments) Nice! https://gerrit.ovirt.org/#/c/64301/1/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, volUUID): Add image uuid, standard api for volume methods. Line 815: lvm.deactivateLVs(self.sdUUID, [volUUID]) Line 816: Line 817: Line 818: class BlockStorageDomain(sd.StorageDomain): https://gerrit.ovirt.org/#/c/64301/1/vdsm/storage/sd.py File vdsm/storage/sd.py: Line 524: Line 525: if preallocate is not None and preallocate not in sc.VOL_TYPE: Line 526: raise se.IncorrectType(preallocate) Line 527: Line 528: def teardownVolume(self, volUUID): Volume is defined by domain id, image id and volume id. Please add the image id to this function in this patch (you added it in the next patch). Line 529: """ Line 530: By default, this method does nothing. It is overriden in blockSD Line 531: to do the necessary cleaning. Line 532: """ Line 527: Line 528: def teardownVolume(self, volUUID): Line 529: """ Line 530: By default, this method does nothing. It is overriden in blockSD Line 531: to do the necessary cleaning. Replace the text with something like: Called when a volume is detached from a prepared image during live merge flow. In this case the volume will not be teardown when the image is tear down. This does nothing, subclass should override this if needed. Line 532: """ Line 533: pass Line 534: Line 535: Line 529: """ Line 530: By default, this method does nothing. It is overriden in blockSD Line 531: to do the necessary cleaning. Line 532: """ Line 533: pass pass is not needed, having a docstring is enough. Line 534: Line 535: Line 536: class StorageDomain(object): Line 537: log = logging.getLogger("storage.StorageDomain") https://gerrit.ovirt.org/#/c/64301/1/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 64: from vdsm.virt.vmpowerdown import VmShutdown, VmReboot Line 65: from vdsm.virt.utils import isVdsmImage, cleanup_guest_socket Line 66: from storage import outOfProcess as oop Line 67: from storage import sd Line 68: from storage.sdc import sdCache Please avoid this bad practice - this must be: from storage import sdc Line 69: Line 70: # local imports Line 71: # In future those should be imported via .. Line 72: import caps 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 teardownVolume(self, volUUID): This should match other methods like update_base_size - no arguments, lower_case: def teardown_top_volume(self): The flow in _run() should read like documentation. Add TODO to move this to storage public api. This is a layering violation, virt code should not call storage code except via public storage apis (e.g. irs.getVolumeSize()) Line 4768: dom_manifest = sdCache.produce_manifest(self.drive.domainID) Line 4769: dom_manifest.teardownVolume(volUUID) Line 4770: Line 4771: @utils.traceback() Line 4764:self.drive.imageID, baseVolUUID, Line 4765:topVolInfo['capacity']) Line 4766: Line 4767: def teardownVolume(self, volUUID): Line 4768: dom_manifest = sdCache.produce_manifest(self.drive.domainID) We call this sd_manifest elsewhere. Get volume and image id here, from self.job. Line 4769: dom_manifest.teardownVolume(volUUID) Line 4770: Line 4771: @utils.traceback() Line 4772: def run(self): Line 4780: self.vm.enableDriveMonitor() 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 The comment is not needed, the function name should reveal the intent. If you need more documentation - why we must do this, add a docstring to the function. Line 4785: self.teardownVolume(self.job['topVolume']) Line 4786: Line 4787: def isSuccessful(self): Line 4788: """ -- 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: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala
Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge
Ala Hino has posted comments on this change. Change subject: Live Merge: teardown volume on HSM after live merge .. Patch Set 1: Verified+1 -- 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: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge
gerrit-hooks has posted comments on this change. Change subject: Live Merge: teardown volume on HSM after live merge .. Patch Set 1: * #1377849::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1377849::OK, public bug * Check Product::#1377849::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Jenkins CI Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge
Ala Hino has uploaded a new change for review. Change subject: Live Merge: teardown volume on HSM after live merge .. Live Merge: teardown volume on HSM after live merge If a VM is running on HSM and live merge is performed, the LV isn't deactivated because, the deactivation is done when deleting the volume. However, deleting the volume is done on SPM and this means that the LV is not deactivated on the HSM. In this patch, a logic to teardown the volume added after live merge has completed. Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549 Bug-Url: https://bugzilla.redhat.com/1377849 Signed-off-by: Ala Hino--- M vdsm/storage/blockSD.py M vdsm/storage/sd.py M vdsm/virt/vm.py 3 files changed, 17 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/01/64301/1 diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py index 8ac7433..a538581 100644 --- a/vdsm/storage/blockSD.py +++ b/vdsm/storage/blockSD.py @@ -811,6 +811,9 @@ sd.LEASE_BLOCKS) return clusterlock.Lease(volUUID, self.getLeasesFilePath(), offset) +def teardownVolume(self, volUUID): +lvm.deactivateLVs(self.sdUUID, [volUUID]) + class BlockStorageDomain(sd.StorageDomain): manifestClass = BlockStorageDomainManifest diff --git a/vdsm/storage/sd.py b/vdsm/storage/sd.py index b5b902e..29dcf53 100644 --- a/vdsm/storage/sd.py +++ b/vdsm/storage/sd.py @@ -525,6 +525,13 @@ if preallocate is not None and preallocate not in sc.VOL_TYPE: raise se.IncorrectType(preallocate) +def teardownVolume(self, volUUID): +""" +By default, this method does nothing. It is overriden in blockSD +to do the necessary cleaning. +""" +pass + class StorageDomain(object): log = logging.getLogger("storage.StorageDomain") diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py index 1d9a3d5..7004615 100644 --- a/vdsm/virt/vm.py +++ b/vdsm/virt/vm.py @@ -65,6 +65,7 @@ from vdsm.virt.utils import isVdsmImage, cleanup_guest_socket from storage import outOfProcess as oop from storage import sd +from storage.sdc import sdCache # local imports # In future those should be imported via .. @@ -4763,6 +4764,10 @@ self.drive.imageID, baseVolUUID, topVolInfo['capacity']) +def teardownVolume(self, volUUID): +dom_manifest = sdCache.produce_manifest(self.drive.domainID) +dom_manifest.teardownVolume(volUUID) + @utils.traceback() def run(self): self.update_base_size() @@ -4776,6 +4781,8 @@ self.success = True self.vm.log.info("Synchronization completed (job %s)", self.job['jobID']) +# teardown the merged volume +self.teardownVolume(self.job['topVolume']) def isSuccessful(self): """ -- To view, visit https://gerrit.ovirt.org/64301 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge
Ala Hino has posted comments on this change. Change subject: Live Merge: teardown volume on HSM after live merge .. Patch Set 4: Verified-1 -- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge
Nir Soffer has posted comments on this change. Change subject: Live Merge: teardown volume on HSM after live merge .. Patch Set 4: Code-Review-1 Did we agree that this is not the right approach? -- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge
Ala Hino has posted comments on this change. Change subject: Live Merge: teardown volume on HSM after live merge .. Patch Set 4: Verified+1 Need to verify wipe after delete logic when deactivating the LV before deleting the volume -- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge
gerrit-hooks has posted comments on this change. Change subject: Live Merge: teardown volume on HSM after live merge .. Patch Set 4: * #1377849::Update tracker: OK * #1321018::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1377849::OK, public bug * Check Public Bug::#1321018::OK, public bug * Check Product::#1377849::OK, Correct classification oVirt * Check Product::#1321018::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala HinoGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org