Change in vdsm[master]: migration: coalesce join() into stop()
Francesco Romani has posted comments on this change. Change subject: migration: coalesce join() into stop() .. Patch Set 8: Code-Review-1 Verified-1 controversial, needs more work -- To view, visit https://gerrit.ovirt.org/62587 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6def55d50a61ac983b45e826c05e09887fe5ee0 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: migration: coalesce join() into stop()
gerrit-hooks has posted comments on this change. Change subject: migration: coalesce join() into stop() .. Patch Set 8: * Update Tracker::IGNORE, no bug url/s found * Check Bug-Url::IGNORE, no bug url/s found * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/62587 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6def55d50a61ac983b45e826c05e09887fe5ee0 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: migration: coalesce join() into stop()
gerrit-hooks has posted comments on this change. Change subject: migration: coalesce join() into stop() .. Patch Set 7: * Update Tracker::IGNORE, no bug url/s found * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/62587 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6def55d50a61ac983b45e826c05e09887fe5ee0 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: migration: coalesce join() into stop()
gerrit-hooks has posted comments on this change. Change subject: migration: coalesce join() into stop() .. Patch Set 6: * Update Tracker::IGNORE, no bug url/s found * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/62587 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6def55d50a61ac983b45e826c05e09887fe5ee0 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: migration: coalesce join() into stop()
gerrit-hooks has posted comments on this change. Change subject: migration: coalesce join() into stop() .. Patch Set 5: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/62587 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6def55d50a61ac983b45e826c05e09887fe5ee0 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: migration: coalesce join() into stop()
Nir Soffer has posted comments on this change.
Change subject: migration: coalesce join() into stop()
..
Patch Set 4: Code-Review-1
(1 comment)
https://gerrit.ovirt.org/#/c/62587/4/vdsm/virt/migration.py
File vdsm/virt/migration.py:
Line 576: self._stop.set()
Line 577: if self._thread.is_alive():
Line 578: # on very short migrations, the downtime thread
Line 579: # may not be started at all.
Line 580: self._thread.join()
This is generally bad idea, making stop wait for the thread in all cases.
Separating stop() and join() will allow stopping multiple threads at once.
Line 581:
Line 582: def _set_downtime(self, downtime):
Line 583: self._vm.log.debug('setting migration downtime to %d',
downtime)
Line 584: self._vm._dom.migrateSetMaxDowntime(downtime, 0)
--
To view, visit https://gerrit.ovirt.org/62587
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6def55d50a61ac983b45e826c05e09887fe5ee0
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: gerrit-hooks
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: migration: coalesce join() into stop()
gerrit-hooks has posted comments on this change. Change subject: migration: coalesce join() into stop() .. Patch Set 4: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/62587 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6def55d50a61ac983b45e826c05e09887fe5ee0 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: migration: coalesce join() into stop()
Milan Zamazal has posted comments on this change. Change subject: migration: coalesce join() into stop() .. Patch Set 3: Code-Review+1 Good idea. -- To view, visit https://gerrit.ovirt.org/62587 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6def55d50a61ac983b45e826c05e09887fe5ee0 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: migration: coalesce join() into stop()
gerrit-hooks has posted comments on this change. Change subject: migration: coalesce join() into stop() .. Patch Set 3: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/62587 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6def55d50a61ac983b45e826c05e09887fe5ee0 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: migration: coalesce join() into stop()
gerrit-hooks has posted comments on this change. Change subject: migration: coalesce join() into stop() .. Patch Set 2: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/62587 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6def55d50a61ac983b45e826c05e09887fe5ee0 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: migration: coalesce join() into stop()
gerrit-hooks has posted comments on this change. Change subject: migration: coalesce join() into stop() .. Patch Set 1: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/62587 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6def55d50a61ac983b45e826c05e09887fe5ee0 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: migration: coalesce join() into stop()
Francesco Romani has uploaded a new change for review.
Change subject: migration: coalesce join() into stop()
..
migration: coalesce join() into stop()
we always call join() after stop(), so coalesce the two
method in stop().
Doing so, we can also get rid of most of the remnants of the thread
interface that pollutes DowntimeThread, MonitorThread and SourceThread.
Change-Id: Ib6def55d50a61ac983b45e826c05e09887fe5ee0
Signed-off-by: Francesco Romani
---
M vdsm/virt/migration.py
1 file changed, 5 insertions(+), 22 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/87/62587/1
diff --git a/vdsm/virt/migration.py b/vdsm/virt/migration.py
index 28963bf..e66fab2 100644
--- a/vdsm/virt/migration.py
+++ b/vdsm/virt/migration.py
@@ -495,13 +495,10 @@
with utils.running(self._monitorThread):
self._perform_migration(duri, muri)
-self._monitorThread.join()
-
def _perform_with_conv_schedule(self, duri, muri):
self._vm.log.debug('performing migration with conv schedule')
with utils.running(self._monitorThread):
self._perform_migration(duri, muri)
-self._monitorThread.join()
def set_max_bandwidth(self, bandwidth):
self._vm.log.debug('setting migration max bandwidth to %d', bandwidth)
@@ -556,12 +553,6 @@
def start(self):
self._thread.start()
-def join(self):
-return self._thread.join()
-
-def is_alive(self):
-return self._thread.is_alive()
-
@utils.traceback()
def run(self):
self._vm.log.debug('migration downtime thread started (%i steps)',
@@ -583,6 +574,10 @@
def stop(self):
self._vm.log.debug('stopping migration downtime thread')
self._stop.set()
+if self._thread.is_alive():
+# on very short migrations, the downtime thread
+# may not be started at all.
+self._thread.join()
def _set_downtime(self, downtime):
self._vm.log.debug('setting migration downtime to %d', downtime)
@@ -598,12 +593,6 @@
def stop(self):
pass
-
-def join(self):
-pass
-
-def is_alive(self):
-return False
def set_initial_downtime(self):
pass
@@ -628,9 +617,6 @@
def start(self):
self._thread.start()
-def join(self):
-self._thread.join()
-
@property
def enabled(self):
return MonitorThread._MIGRATION_MONITOR_INTERVAL > 0
@@ -643,10 +629,6 @@
self.monitor_migration()
finally:
self.downtime_thread.stop()
-if self.downtime_thread.is_alive():
-# on very short migrations, the downtime thread
-# may not be started at all.
-self.downtime_thread.join()
self._vm.log.debug('stopped migration monitor thread')
else:
self._vm.log.info('migration monitor thread disabled'
@@ -735,6 +717,7 @@
def stop(self):
self._vm.log.debug('stopping migration monitor thread')
self._stop.set()
+self._thread.join()
def _next_action(self, stalling):
head = self._conv_schedule['stalling'][0]
--
To view, visit https://gerrit.ovirt.org/62587
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib6def55d50a61ac983b45e826c05e09887fe5ee0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
