vm: avoid starting the same VM multiple times simultaneously

While libvirt handle locking itself, there is also Qubes-specific
startup part. Especially starting qrexec-daemon and waiting until
qrexec-agent connect to it. When someone will attempt to start VM the
second time (or simply assume it's already running) - qrexec will not be
connected yet and the operation will fail. Solve the problem by wrapping
the whole vm.start() function with a lock, including a check if VM is
running and waiting for qrexec.

Also, do not throw exception if VM is already running.

This way, after a call to vm.start(), VM will be started with qrexec
connected - regardless of who really started it.
Note that, it will not solve the situation when someone check if VM is
running manually, like:

    if not vm.is_running():
        yield from vm.start()

Such code should be changed to simply:

    yield from vm.start()

Fixes QubesOS/qubes-issues#2001
Fixes QubesOS/qubes-issues#2666
This commit is contained in:
Marek Marczykowski-Górecki 2017-06-01 03:31:59 +02:00
parent 1f86c9253c
commit 018877a19c
No known key found for this signature in database
GPG Key ID: 063938BA42CFA724

View File

@ -704,6 +704,10 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
# will be initialized after loading all the properties # will be initialized after loading all the properties
#: operations which shouldn't happen simultaneously with qube startup
# (including another startup of the same qube)
self.startup_lock = asyncio.Lock()
# fire hooks # fire hooks
if xml is None: if xml is None:
self.events_enabled = True self.events_enabled = True
@ -841,10 +845,11 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
:param int mem_required: FIXME :param int mem_required: FIXME
''' '''
with (yield from self.startup_lock):
# Intentionally not used is_running(): eliminate also "Paused", # Intentionally not used is_running(): eliminate also "Paused",
# "Crashed", "Halting" # "Crashed", "Halting"
if self.get_power_state() != 'Halted': if self.get_power_state() != 'Halted':
raise qubes.exc.QubesVMNotHaltedError(self) return
self.log.info('Starting {}'.format(self.name)) self.log.info('Starting {}'.format(self.name))
@ -860,16 +865,15 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
yield from self.netvm.start(start_guid=start_guid, yield from self.netvm.start(start_guid=start_guid,
notify_function=notify_function) notify_function=notify_function)
# TODO: lock qmemman_client = yield from asyncio.get_event_loop().\
run_in_executor(None, self.request_memory, mem_required)
qmemman_client = yield from asyncio.get_event_loop().run_in_executor(
None, self.request_memory, mem_required)
try: try:
yield from self.storage.start() yield from self.storage.start()
self._update_libvirt_domain() self._update_libvirt_domain()
self.libvirt_domain.createWithFlags(libvirt.VIR_DOMAIN_START_PAUSED) self.libvirt_domain.createWithFlags(
libvirt.VIR_DOMAIN_START_PAUSED)
finally: finally:
if qmemman_client: if qmemman_client:
qmemman_client.close() qmemman_client.close()
@ -909,8 +913,9 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
except: # pylint: disable=bare-except except: # pylint: disable=bare-except
if self.is_running() or self.is_paused(): if self.is_running() or self.is_paused():
# This avoids losing the exception if an exception is raised in # This avoids losing the exception if an exception is
# self.force_shutdown(), because the vm is not running or paused # raised in self.force_shutdown(), because the vm is not
# running or paused
yield from self.kill() # pylint: disable=not-an-iterable yield from self.kill() # pylint: disable=not-an-iterable
raise raise
finally: finally:
@ -919,10 +924,20 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
return self return self
@asyncio.coroutine
def on_domain_shutdown_coro(self):
'''Coroutine for executing cleanup after domain shutdown.
Do not allow domain to be started again until this finishes.
'''
with (yield from self.startup_lock):
yield from self.storage.stop()
@qubes.events.handler('domain-shutdown') @qubes.events.handler('domain-shutdown')
def on_domain_shutdown(self, _event, **_kwargs): def on_domain_shutdown(self, _event, **_kwargs):
'''Cleanup after domain shutdown''' '''Cleanup after domain shutdown'''
asyncio.ensure_future(self.storage.stop()) # TODO: ensure that domain haven't been started _before_ this
# coroutine got a chance to acquire a lock
asyncio.ensure_future(self.on_domain_shutdown_coro())
@asyncio.coroutine @asyncio.coroutine
def shutdown(self, force=False, wait=False): def shutdown(self, force=False, wait=False):