Milan Zamazal has posted comments on this change.
Change subject: virt: Start all VM related stuff after switching to post-copy migration
......................................................................
Patch Set 8:
(1 comment)
https://gerrit.ovirt.org/#/c/63551/8/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 263: self._lastStatus = vmstatus.WAIT_FOR_LAUNCH
Line 264: self._migrationSourceThread = migration.SourceThread(self)
Line 265: self._kvmEnable = self.conf.get('kvmEnable', 'true')
Line 266: self._incomingMigrationFinished = threading.Event()
Line 267: self._incoming_migration_vm_running = threading.Event()
Why can't we just do incomingMigrationFinished.set() earlier?
Well, what (except for the status and the corresponding event) will change if we
simply finish the creation thread on switching to post-copy? I'd be worried about
unsetting self.recovery, but it seems to be safe. Then perhaps raising exceptions in the
rest of the creation thread, but there should be no exception in that path, right? And
finally there is the incoming_migrations semaphore, it can be handled together with
status. So it might work.
But the primary question is: What do we gain by exiting the creation thread early or what
do we lose by not exiting it early? Is it just to avoid introducing another
synchronization variable (for the price of slightly more complicated handling of the
status and the semaphore) or anything else?
Line 268: self.id = self.conf['vmId']
Line 269: self._volPrepareLock = threading.Lock()
Line 270: self._initTimePauseCode = None
Line 271: self._initTimeRTC = int(self.conf.get('timeOffset', 0))
--
To view, visit
https://gerrit.ovirt.org/63551
To unsubscribe, visit
https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8b17e58f63cbd9db09e4420871a562eaa0b3f3d
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal <mzamazal(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes