Change in vdsm[master]: virt: migration: use contextmanager for monitor
Dan Kenigsberg has submitted this change and it was merged. Change subject: virt: migration: use contextmanager for monitor .. virt: migration: use contextmanager for monitor This patch factors the monitor thread control in a context manager to make code clearer and less cluttered. Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Signed-off-by: Francesco Romani from...@redhat.com Reviewed-on: http://gerrit.ovirt.org/25978 Reviewed-by: Dan Kenigsberg dan...@redhat.com Reviewed-by: Nir Soffer nsof...@redhat.com --- M lib/vdsm/utils.py M vdsm/virt/migration.py 2 files changed, 37 insertions(+), 28 deletions(-) Approvals: Nir Soffer: Looks good to me, but someone else must approve Dan Kenigsberg: Looks good to me, approved Francesco Romani: Verified -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 28 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 28: Build Successful http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5598/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3756/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_create-rpms_merged/1614/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 28 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Francesco Romani has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 27: Verified+1 Verified running a migration and observing output on engine and vdsClient - OK. -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 27 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 26: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/10129/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10914/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1216/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/11071/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 26 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Dan Kenigsberg has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 26: Code-Review-1 Would you rebase on top of the master branch? I like to merge it regardless of the debated patches below it. -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 26 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Francesco Romani has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 27: rebased against master -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 27 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 27: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/10152/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10937/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1228/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/11094/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 27 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Dan Kenigsberg has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 27: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 27 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Nir Soffer has posted comments on this change.
Change subject: virt: migration: use contextmanager for monitor
..
Patch Set 27: Code-Review+1
(1 comment)
http://gerrit.ovirt.org/#/c/25978/27/vdsm/virt/migration.py
File vdsm/virt/migration.py:
Line 307: self._vm.log.debug('starting migration to %s '
Line 308:'with miguri %s', duri, muri)
Line 309:
Line 310: t = DowntimeThread(self._vm, int(self._downtime))
Line 311: self._monitorThread = MonitorThread(self._vm, startTime)
If this raises, you will not cancel the DowntimeThread. This is not new - the
same issue was in the previous version, so I don't think this should block this
change.
But given the new running decorator, you can do:
downtimeThread = ...
monitorThread = ...
with utils.running(downtimeThread):
with utils.running(monitorThread):
self._perfom_migration(duri, muri)
Assuming that both threads implement the running interface.
The double with is little ugly, but does cleanup properly.
Line 312: with utils.running(self._monitorThread):
Line 313: try:
Line 314: self._perform_migration(duri, muri)
Line 315: finally:
--
To view, visit http://gerrit.ovirt.org/25978
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8
Gerrit-PatchSet: 27
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 24: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9711/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10496/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1045/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/10653/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 24 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Francesco Romani has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 25: Verified doing a few migration back and forth. Tested-With: 25976, 25977, 25978, 26279 -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 25 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Francesco Romani has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 25: Verified+1 -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 25 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 25: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9736/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10521/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1063/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/10678/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 25 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Nir Soffer has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 25: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 25 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 23: Build Successful http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9521/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10305/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10461/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5387/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3545/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/944/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 23 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Francesco Romani has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 21: (1 comment) http://gerrit.ovirt.org/#/c/25978/21/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 342: runnable.start() Line 343: try: Line 344: yield runnable Line 345: finally: Line 346: runnable.stop() Now when this generic utility, why not move it to lib/vdsm/utils.py? Done Line 347: Line 348: Line 349: class MonitorThread(threading.Thread): Line 350: _MONITOR_TICK = 1.0 # unit: seconds -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Francesco Romani has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 21: moved running() into utils; rebased -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Nir Soffer has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 22: Code-Review+1 Nice! -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 22 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 22: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9349/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10133/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/889/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10290/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5216/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3374/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_storage_functional_tests_localfs_gerrit/1310/ : There was an infra issue, please contact in...@ovirt.org -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 22 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Francesco Romani has posted comments on this change.
Change subject: virt: migration: use contextmanager for monitor
..
Patch Set 20:
(3 comments)
http://gerrit.ovirt.org/#/c/25978/20/vdsm/virt/migration.py
File vdsm/virt/migration.py:
Line 62: self._tunneled = utils.tobool(tunneled)
Line 63: self._abortOnError = utils.tobool(abortOnError)
Line 64: self._dstqemu = dstqemu
Line 65: self._downtime = int(kwargs.get('downtime') or
Line 66: config.get('vars', 'migration_downtime'))
Is this related? Because eventually this may be better:
Not stricly related, will split in a (trivial) patch.
I like the current code slightly more because, as you pointed out, the
short-circuit behaviour of the 'or' operator.
Line 67: self.status = {
Line 68: 'status': {
Line 69: 'code': 0,
Line 70: 'message': 'Migration in progress'},
Line 298:'with miguri %s', duri, muri)
Line 299:
Line 300: with migrationMonitor(self._vm,
Line 301: startTime,
Line 302: self._downtime) as
self._monitorThread:
Setting self._monitorThread in the as part is surprising, while the old c
I agree your version is better in every way. Will change accordingly.
Line 303: if self._vm.hasSpice and
self._vm.conf.get('clientIp'):
Line 304: SPICE_MIGRATION_HANDOVER_TIME = 120
Line 305:
self._vm._reviveTicket(SPICE_MIGRATION_HANDOVER_TIME)
Line 306:
Line 340: monitorThread.start()
Line 341: try:
Line 342: yield monitorThread
Line 343: finally:
Line 344: monitorThread.stop()
This can be much more useful as general utility for things that you can sta
Right. Will change.
Line 345:
Line 346:
Line 347: class MonitorThread(threading.Thread):
Line 348: _MONITOR_TICK = 1.0 # unit: seconds
--
To view, visit http://gerrit.ovirt.org/25978
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8
Gerrit-PatchSet: 20
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Francesco Romani has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 21: addressed comments from Federico and Nir. -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 21: Build Successful http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9298/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10082/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/865/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10238/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5164/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3322/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Nir Soffer has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 21: (1 comment) http://gerrit.ovirt.org/#/c/25978/21/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 342: runnable.start() Line 343: try: Line 344: yield runnable Line 345: finally: Line 346: runnable.stop() Now when this generic utility, why not move it to lib/vdsm/utils.py? Line 347: Line 348: Line 349: class MonitorThread(threading.Thread): Line 350: _MONITOR_TICK = 1.0 # unit: seconds -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 20: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9234/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10018/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/845/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10173/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5100/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3257/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 20 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Federico Simoncelli has posted comments on this change.
Change subject: virt: migration: use contextmanager for monitor
..
Patch Set 20: Code-Review+1
(2 comments)
http://gerrit.ovirt.org/#/c/25978/20/vdsm/virt/migration.py
File vdsm/virt/migration.py:
Line 62: self._tunneled = utils.tobool(tunneled)
Line 63: self._abortOnError = utils.tobool(abortOnError)
Line 64: self._dstqemu = dstqemu
Line 65: self._downtime = int(kwargs.get('downtime') or
Line 66: config.get('vars', 'migration_downtime'))
Is this related? Because eventually this may be better:
self._downtime = int(kwargs.get('downtime', config.get('vars',
'migration_downtime')))
Or is it better the current code because the right part of or is evaluated
only when really needed?
Line 67: self.status = {
Line 68: 'status': {
Line 69: 'code': 0,
Line 70: 'message': 'Migration in progress'},
Line 298:'with miguri %s', duri, muri)
Line 299:
Line 300: with migrationMonitor(self._vm,
Line 301: startTime,
Line 302: self._downtime) as
self._monitorThread:
I am not a fan of this indentation but it's just taste. Incidentally I would
have used:
with migrationMonitor(
self._vm, startTime, self._downtime) as self._monitorThread:
(PS. maybe it wouldn't fit 79 chars)
(PPS. I can't recall but I think pep8 had some bug regarding the with
statement indentation).
Anyway I'm fine with the current code too.
Line 303: if self._vm.hasSpice and
self._vm.conf.get('clientIp'):
Line 304: SPICE_MIGRATION_HANDOVER_TIME = 120
Line 305:
self._vm._reviveTicket(SPICE_MIGRATION_HANDOVER_TIME)
Line 306:
--
To view, visit http://gerrit.ovirt.org/25978
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8
Gerrit-PatchSet: 20
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Nir Soffer has posted comments on this change.
Change subject: virt: migration: use contextmanager for monitor
..
Patch Set 20:
(1 comment)
http://gerrit.ovirt.org/#/c/25978/20/vdsm/virt/migration.py
File vdsm/virt/migration.py:
Line 298:'with miguri %s', duri, muri)
Line 299:
Line 300: with migrationMonitor(self._vm,
Line 301: startTime,
Line 302: self._downtime) as
self._monitorThread:
I am not a fan of this indentation but it's just taste. Incidentally I woul
Setting self._monitorThread in the as part is surprising, while the old code
was very clear.
How about this:
self._monitorThread = MonitorThread(...)
with self._monitorThread:
do stuff
Where MonitorThread implements:
def __enter__(self):
self.start()
return self
def __exit__(self, *args):
self.stop()
But looking around, the issue in the old code was not the try-finally, but
having a lot of code in the try finally block.
So changing the old code to:
self._monitorThread = MonitorThread(...)
self._monitorThread.start()
try:
self.peform_migration(...)
finally:
self._monitorThread.stop()
Is better - it is safe and very easy to read and change.
Line 303: if self._vm.hasSpice and
self._vm.conf.get('clientIp'):
Line 304: SPICE_MIGRATION_HANDOVER_TIME = 120
Line 305:
self._vm._reviveTicket(SPICE_MIGRATION_HANDOVER_TIME)
Line 306:
--
To view, visit http://gerrit.ovirt.org/25978
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8
Gerrit-PatchSet: 20
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Nir Soffer has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 20: (1 comment) http://gerrit.ovirt.org/#/c/25978/20/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 340: monitorThread.start() Line 341: try: Line 342: yield monitorThread Line 343: finally: Line 344: monitorThread.stop() This can be much more useful as general utility for things that you can start and stop. @contextmanager def running(thing): thing.start() try: yield finally: thing.stop() Usage: self._thread = FooThread() with running(self._thread): do staff and some more Line 345: Line 346: Line 347: class MonitorThread(threading.Thread): Line 348: _MONITOR_TICK = 1.0 # unit: seconds -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 20 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 19: Build Successful http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8915/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9699/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/682/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9854/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Francesco Romani has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 18: Verified+1 Verified with 25976, 25977 and 25979 by running a few migrations. -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Francesco Romani has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 18: s/25979/26279/ -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 18: Build Successful http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9691/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8758/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9544/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Vinzenz Feenstra has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 18: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Antoni Segura Puimedon has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 17: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 17: Build Successful http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8275/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7485/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8393/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 16: Build Successful http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8040/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8153/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7250/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Antoni Segura Puimedon has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 16: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Federico Simoncelli has posted comments on this change.
Change subject: virt: migration: use contextmanager for monitor
..
Patch Set 15:
(1 comment)
http://gerrit.ovirt.org/#/c/25978/15/vdsm/virt/migration.py
File vdsm/virt/migration.py:
Line 61: self._tunneled = utils.tobool(tunneled)
Line 62: self._abortOnError = utils.tobool(abortOnError)
Line 63: self._dstqemu = dstqemu
Line 64: self._downtime = int(kwargs.get('downtime') or
Line 65: config.get('vars', 'migration_downtime'))
Minor: I suppose this change (moving int here) could be split out.
Line 66: self.status = {
Line 67: 'status': {
Line 68: 'code': 0,
Line 69: 'message': 'Migration in progress'},
--
To view, visit http://gerrit.ovirt.org/25978
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 15: Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/7006/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7908/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7797/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Francesco Romani has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 15: Patch set 15: rebased -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Vinzenz Feenstra has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 12: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Antoni Segura Puimedon has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 12: Code-Review+1 (1 comment) I really, really like this patch! http://gerrit.ovirt.org/#/c/25978/12/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 332: raise Line 333: Line 334: Line 335: @contextmanager Line 336: def migrationMonitor(vm, startTime, downTime): what about just monitor so that we have, in line 297, with monitor(... Line 337: monitorThread = MonitorThread(vm, startTime, downTime) Line 338: monitorThread.start() Line 339: try: Line 340: yield monitorThread -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 13: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6974/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7876/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7765/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 14: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6981/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7883/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7772/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 11: Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6949/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7851/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7740/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 12: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6956/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7858/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7747/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 9: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6913/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7703/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7815/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 10: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6926/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7716/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7828/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Antoni Segura Puimedon has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 9: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 7: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6823/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7613/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7723/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 8: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6828/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7618/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7728/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Francesco Romani has uploaded a new change for review.
Change subject: virt: migration: use contextmanager for monitor
..
virt: migration: use contextmanager for monitor
This patch factors the MigrationMonitorThread
control in a context manager to make code
clearer and less cluttered.
Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8
Signed-off-by: Francesco Romani from...@redhat.com
---
M vdsm/migration.py
1 file changed, 13 insertions(+), 9 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/78/25978/1
diff --git a/vdsm/migration.py b/vdsm/migration.py
index 8c33f1e..73d29e8 100644
--- a/vdsm/migration.py
+++ b/vdsm/migration.py
@@ -18,6 +18,7 @@
# Refer to the README and COPYING files for full details of the license
#
+from contextlib import contextmanager
import pickle
import threading
import time
@@ -255,6 +256,17 @@
self._recover(str(e))
self.log.error(Failed to migrate, exc_info=True)
+@contextmanager
+def migrationMonitor(self, startTime):
+self._monitorThread = MigrationMonitorThread(self._vm,
+ startTime,
+ int(self._downtime))
+self._monitorThread.start()
+try:
+yield
+finally:
+self._monitorThread.stop()
+
def _startUnderlyingMigration(self, startTime):
if self._mode == 'file':
hooks.before_vm_hibernate(self._vm._dom.XMLDesc(0), self._vm.conf)
@@ -293,12 +305,7 @@
self._vm.log.debug('starting migration to %s '
'with miguri %s', duri, muri)
-self._monitorThread = MigrationMonitorThread(self._vm,
- startTime,
- int(self._downtime))
-self._monitorThread.start()
-
-try:
+with self.migrationMonitor(startTime):
if ('qxl' in self._vm.conf['display'] and
self._vm.conf.get('clientIp')):
SPICE_MIGRATION_HANDOVER_TIME = 120
@@ -322,9 +329,6 @@
None, maxBandwidth)
else:
self._raiseAbortError()
-
-finally:
-self._monitorThread.stop()
def stop(self):
# if its locks we are before the migrateToURI2()
--
To view, visit http://gerrit.ovirt.org/25978
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 1: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6764/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7554/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7664/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 2: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6765/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7555/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7665/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 3: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6770/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7560/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7670/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Antoni Segura Puimedon has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 3: Code-Review-1 (1 comment) small recommendation to simplify. http://gerrit.ovirt.org/#/c/25978/3/vdsm/migration.py File vdsm/migration.py: Line 256: self._recover(str(e)) Line 257: self.log.error(Failed to migrate, exc_info=True) Line 258: Line 259: @contextmanager Line 260: def migrationMonitor(self, startTime): We could move this outside of the class (receiving the parameters it needs for creating a MigrationMonitorThread, which would be an object local to the context manager. Line 261: self._monitorThread = MigrationMonitorThread(self._vm, Line 262: startTime, Line 263: int(self._downtime)) Line 264: self._monitorThread.start() -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Francesco Romani has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 3: (1 comment) http://gerrit.ovirt.org/#/c/25978/3/vdsm/migration.py File vdsm/migration.py: Line 256: self._recover(str(e)) Line 257: self.log.error(Failed to migrate, exc_info=True) Line 258: Line 259: @contextmanager Line 260: def migrationMonitor(self, startTime): We could move this outside of the class (receiving the parameters it needs We can and I will, with just the minor caveat that MigrationSourceThread will still need an handle to the MigrationMonitorThread to report the progress. But the move is definitely feasible and I'll do it. Line 261: self._monitorThread = MigrationMonitorThread(self._vm, Line 262: startTime, Line 263: int(self._downtime)) Line 264: self._monitorThread.start() -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 4: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6779/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7569/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7679/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 5: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6780/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7570/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7680/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
Antoni Segura Puimedon has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 5: Code-Review+1 Much better with the context manager out :-) thanks! -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: migration: use contextmanager for monitor
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: migration: use contextmanager for monitor .. Patch Set 6: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6785/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7575/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7685/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/25978 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7fcd0bedf4f30cc0bcab03339322df1fc5434e8 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
