Francesco Romani has uploaded a new change for review. Change subject: virt: safer handling of migration parameters ......................................................................
virt: safer handling of migration parameters The Migration Source code path need 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 parameter 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 an 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 Bug-Url: https://bugzilla.redhat.com/1296936 Signed-off-by: Francesco Romani <[email protected]> --- M vdsm/virt/migration.py M vdsm/virt/vm.py 2 files changed, 16 insertions(+), 7 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/56/51656/1 diff --git a/vdsm/virt/migration.py b/vdsm/virt/migration.py index f512b51..948bb02 100644 --- a/vdsm/virt/migration.py +++ b/vdsm/virt/migration.py @@ -286,24 +286,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 d74daaf..9eda9af 100644 --- a/vdsm/virt/vm.py +++ b/vdsm/virt/vm.py @@ -2159,6 +2159,17 @@ 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: + if '_migrationParams' in self.conf: + 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/51656 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I063d74c1fe1725c514f7aeb60463d0558e240baa Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
