Hello Dan Kenigsberg, Martin Polednik,
I'd like you to do a code review. Please visit
https://gerrit.ovirt.org/51828
to review the following change.
Change subject: virt: safer handling of migration parameters
......................................................................
virt: safer handling of migration parameters
The Migration Source code path needs to store additional
transient migration parameters in the Vm configuration state,
to be both saved on recovery file and transmitted on the
destination VM.
Those parameters will be dropped once the migration is done.
Up until now, the migration code mangled the Vm state from the outside.
This was ugly and unsafe, because the configuration was mutated without
any locking.
Under high migration load, this can lead to RuntimeErrors, because
the clients of Vm.conf (e.g. status(), getVMList()...) expect the
lock being held and the access serialized.
To fix those issues, we add a helper method into Vm class, to
let the migration code temporarily store the data it needs in a safe
and simple way.
Change-Id: I063d74c1fe1725c514f7aeb60463d0558e240baa
Backport-To: 3.6
Backport-To: 3.5
Bug-Url: https://bugzilla.redhat.com/1296936
Signed-off-by: Francesco Romani <from...@redhat.com>
Reviewed-on: https://gerrit.ovirt.org/51656
Reviewed-by: Martin Polednik <mpoled...@redhat.com>
Continuous-Integration: Jenkins CI
Reviewed-by: Dan Kenigsberg <dan...@redhat.com>
---
M tests/vmMigrationTests.py
M vdsm/virt/migration.py
M vdsm/virt/vm.py
3 files changed, 39 insertions(+), 7 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/28/51828/1
diff --git a/tests/vmMigrationTests.py b/tests/vmMigrationTests.py
index 9f33296..eb5a950 100644
--- a/tests/vmMigrationTests.py
+++ b/tests/vmMigrationTests.py
@@ -128,6 +128,30 @@
return list(_linear_downtime(downtime, steps))
+class MigrationParamsTests(TestCaseBase):
+
+ def setUp(self):
+ # random values, no real meaning
+ self.params = {
+ 'foo': 'bar',
+ 'answer': 42,
+ 'hyperv': ['qemu', 'kvm'],
+ }
+
+ def test_params_stored(self):
+ with fake.VM() as testvm:
+ with testvm.migration_parameters(self.params):
+ self.assertEquals(testvm.conf['_migrationParams'],
+ self.params)
+
+ def test_params_removed(self):
+ with fake.VM() as testvm:
+ with testvm.migration_parameters(self.params):
+ pass
+
+ self.assertNotIn('_migrationParams', testvm.conf)
+
+
# stolen^Wborrowed from itertools recipes
def pairwise(iterable):
"s -> (s0,s1), (s1,s2), (s2, s3), ..."
diff --git a/vdsm/virt/migration.py b/vdsm/virt/migration.py
index 53950bc..63fd1b5 100644
--- a/vdsm/virt/migration.py
+++ b/vdsm/virt/migration.py
@@ -285,24 +285,22 @@
self.log.debug("migration semaphore acquired "
"after %d seconds",
time.time() - startTime)
- self._vm.conf['_migrationParams'] = {
+ params = {
'dst': self._dst,
'mode': self._mode,
'method': METHOD_ONLINE,
'dstparams': self._dstparams,
'dstqemu': self._dstqemu,
}
- self._vm.saveState()
- self._startUnderlyingMigration(time.time())
- self._finishSuccessfully()
+ with self._vm.migration_parameters(params):
+ self._vm.saveState()
+ self._startUnderlyingMigration(time.time())
+ self._finishSuccessfully()
except libvirt.libvirtError as e:
if e.get_error_code() == libvirt.VIR_ERR_OPERATION_ABORTED:
self.status = response.error(
'migCancelErr', message='Migration canceled')
raise
- finally:
- if '_migrationParams' in self._vm.conf:
- del self._vm.conf['_migrationParams']
except MigrationDestinationSetupError as e:
self._recover(str(e))
# we know what happened, no need to dump hollow stack trace
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 9414e23..c2f5f92 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -2145,6 +2145,16 @@
return response.error('updateDevice', e.message)
@contextmanager
+ def migration_parameters(self, params):
+ with self._confLock:
+ self.conf['_migrationParams'] = params
+ try:
+ yield
+ finally:
+ with self._confLock:
+ del self.conf['_migrationParams']
+
+ @contextmanager
def setLinkAndNetwork(self, dev, conf, linkValue, networkValue, custom,
specParams=None):
vnicXML = dev.getXML()
--
To view, visit https://gerrit.ovirt.org/51828
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I063d74c1fe1725c514f7aeb60463d0558e240baa
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Francesco Romani <from...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com>
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
