6:06 a.m.
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 <fromani(a)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 <fromani(a)redhat.com>
8:38 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
10:30 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
10:16 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
3:10 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
3:10 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
4:37 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
4:26 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
2:31 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
10:47 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
10:18 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
12:33 p.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
10:52 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
9:58 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
7:19 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
7:11 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
8:48 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
7:26 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
7:18 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
7:13 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
7:06 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
6:54 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
6:18 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
6:09 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
12:17 p.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
6:09 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
1:31 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
12:13 p.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
12:08 p.m.
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 <fromani(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/25978
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
Reviewed-by: Nir Soffer <nsoffer(a)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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
9:35 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
4:04 p.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
10:01 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
9:41 a.m.
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 infra(a)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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
9:39 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
9:07 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
2:39 a.m.
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 infra(a)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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
2:36 p.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
11:14 a.m.
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 infra(a)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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
11:11 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
11:10 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
4:24 a.m.
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 infra(a)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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
5:07 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
2:54 a.m.
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... :
There was an infra issue, please contact infra(a)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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
2:40 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
2:28 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
2:28 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
9:10 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
5:26 p.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
5:03 p.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
1:05 p.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
10:19 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
3 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
2:51 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
2:25 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
10:55 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
8 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
8 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
7:28 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
7:26 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
6:16 a.m.
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 <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
3598
days inactive
3711
days old
vdsm-patches@lists.fedorahosted.org
59 comments
7 participants
participants (7)
-
asegurap＠redhat.com -
Dan Kenigsberg -
Federico Simoncelli -
fromani＠redhat.com -
Jenkins CI RO -
Nir Soffer -
Vinzenz Feenstra